diff --git a/backend/app.py b/backend/app.py
index 678fabc..2b22d81 100644
--- a/backend/app.py
+++ b/backend/app.py
@@ -7,6 +7,7 @@ Uses real data from UK Government Compare School Performance downloads.
import hashlib
import re
from contextlib import asynccontextmanager
+from datetime import datetime, timezone
from typing import Optional
import numpy as np
@@ -53,8 +54,9 @@ PHASE_GROUPS: dict[str, set[str]] = {
BASE_URL = "https://www.schoolcompare.co.uk"
MAX_SLUG_LENGTH = 60
-# In-memory sitemap cache
-_sitemap_xml: str | None = None
+# In-memory sitemap cache: name -> XML. Populated on startup and by the admin
+# regenerate endpoint after a pipeline run.
+_sitemaps: dict[str, str] | None = None
def _slugify(text: str) -> str:
@@ -135,16 +137,65 @@ def _school_sitemap_rows(df) -> list[str]:
return rows
-def build_sitemap() -> str:
- """Generate sitemap XML from in-memory school data. Returns the XML string."""
+# Sitemaps cap at 50,000 URLs per file. 10,000 keeps a child small enough to
+# scan by eye in Search Console, which is the point of splitting at all:
+# coverage is reported per submitted sitemap, so one file per page family is
+# what makes an indexation problem attributable to a family.
+SITEMAP_CHUNK_SIZE = 10_000
+
+# Children are served under /sitemaps/ because Next.js only treats a whole
+# bracketed path segment as dynamic — a route folder named "sitemap-[...parts]"
+# is read as a literal static segment and never matches.
+SITEMAP_CHILD_PREFIX = "/sitemaps"
+
+
+def _urlset(rows: list[str]) -> str:
+ return "\n".join([
+ '',
+ '',
+ *rows,
+ "",
+ ])
+
+
+def build_sitemaps() -> dict[str, str]:
+ """Build the sitemap index and every child, keyed by name."""
df = load_school_data()
- lines = ['',
- '']
- lines.extend(_url_element(BASE_URL + path) for path in STATIC_SITEMAP_PATHS)
- lines.extend(_school_sitemap_rows(df))
- lines.append("")
- return "\n".join(lines)
+ children: dict[str, str] = {
+ "static.xml": _urlset(
+ [_url_element(BASE_URL + path) for path in STATIC_SITEMAP_PATHS]),
+ }
+
+ school_rows = _school_sitemap_rows(df)
+ # Always emit at least one school child, so the index shape is stable even
+ # on an empty database.
+ chunks = [school_rows[i:i + SITEMAP_CHUNK_SIZE]
+ for i in range(0, len(school_rows), SITEMAP_CHUNK_SIZE)] or [[]]
+ for n, chunk in enumerate(chunks, start=1):
+ children[f"schools-{n}.xml"] = _urlset(chunk)
+
+ # On a sitemap index, lastmod means "when this sitemap file last changed",
+ # so generation time is the correct value here — unlike on a , where
+ # it would be a claim about content we cannot support.
+ generated = datetime.now(timezone.utc).date().isoformat()
+ index_rows = [
+ f" {BASE_URL}{SITEMAP_CHILD_PREFIX}/{name}"
+ f"{generated}"
+ for name in children
+ ]
+ index = "\n".join([
+ '',
+ '',
+ *index_rows,
+ "",
+ ])
+ return {**children, "sitemap.xml": index}
+
+
+def build_sitemap() -> str:
+ """The sitemap index. Kept for `lifespan` and the admin endpoint."""
+ return build_sitemaps()["sitemap.xml"]
def clean_filter_values(series: pd.Series) -> list[str]:
@@ -322,7 +373,7 @@ def validate_postcode(postcode: Optional[str]) -> Optional[str]:
@asynccontextmanager
async def lifespan(app: FastAPI):
"""Application lifespan - startup and shutdown events."""
- global _sitemap_xml
+ global _sitemaps
print("Loading school data from marts...")
df = load_school_data()
if df.empty:
@@ -332,9 +383,9 @@ async def lifespan(app: FastAPI):
# Pre-compute the latest-year snapshot so the first search request is fast
await asyncio.to_thread(load_latest_school_data)
try:
- _sitemap_xml = build_sitemap()
- n = _sitemap_xml.count("")
- print(f"Sitemap built: {n} URLs.")
+ _sitemaps = build_sitemaps()
+ n = sum(x.count("") for x in _sitemaps.values())
+ print(f"Sitemaps built: {len(_sitemaps)} files, {n} URLs.")
except Exception as e:
print(f"Warning: sitemap build failed on startup: {e}")
@@ -1125,16 +1176,28 @@ async def robots_txt():
return FileResponse(settings.frontend_dir / "robots.txt", media_type="text/plain")
-@app.get("/sitemap.xml")
-async def sitemap_xml():
- """Serve sitemap.xml for search engine indexing."""
- global _sitemap_xml
- if _sitemap_xml is None:
+def _serve_sitemap(name: str) -> Response:
+ global _sitemaps
+ if _sitemaps is None:
try:
- _sitemap_xml = build_sitemap()
+ _sitemaps = build_sitemaps()
except Exception as e:
raise HTTPException(status_code=503, detail=f"Sitemap unavailable: {e}")
- return Response(content=_sitemap_xml, media_type="application/xml")
+ if name not in _sitemaps:
+ raise HTTPException(status_code=404, detail="No such sitemap")
+ return Response(content=_sitemaps[name], media_type="application/xml")
+
+
+@app.get("/sitemap.xml")
+async def sitemap_xml():
+ """Serve the sitemap index."""
+ return _serve_sitemap("sitemap.xml")
+
+
+@app.get("/sitemaps/{name}")
+async def sitemap_child(name: str):
+ """Serve a child sitemap (static.xml, or schools-N.xml)."""
+ return _serve_sitemap(name)
@app.post("/api/admin/regenerate-sitemap")
@@ -1144,10 +1207,10 @@ async def regenerate_sitemap(
_: bool = Depends(verify_admin_api_key),
):
"""Rebuild and cache the sitemap from current school data. Called by Airflow after data updates."""
- global _sitemap_xml
- _sitemap_xml = build_sitemap()
- n = _sitemap_xml.count("")
- return {"status": "ok", "urls": n}
+ global _sitemaps
+ _sitemaps = build_sitemaps()
+ n = sum(x.count("") for x in _sitemaps.values())
+ return {"status": "ok", "urls": n, "sitemaps": len(_sitemaps)}
# Mount static files directly (must be after all routes to avoid catching API calls)
diff --git a/backend/tests/test_sitemap.py b/backend/tests/test_sitemap.py
index 7564cb1..eb030b1 100644
--- a/backend/tests/test_sitemap.py
+++ b/backend/tests/test_sitemap.py
@@ -32,32 +32,56 @@ def _schools_df() -> pd.DataFrame:
@pytest.fixture()
def sitemap(monkeypatch) -> str:
+ """The sitemap index."""
from backend import app as app_module
monkeypatch.setattr(app_module, "load_school_data", _schools_df)
return app_module.build_sitemap()
-def test_every_loc_uses_the_www_host(sitemap):
+@pytest.fixture()
+def schools_child(monkeypatch) -> str:
+ """The first school child sitemap, where school URLs actually live."""
+ from backend import app as app_module
+
+ monkeypatch.setattr(app_module, "load_school_data", _schools_df)
+ return app_module.build_sitemaps()["schools-1.xml"]
+
+
+@pytest.fixture()
+def static_child(monkeypatch) -> str:
+ from backend import app as app_module
+
+ monkeypatch.setattr(app_module, "load_school_data", _schools_df)
+ return app_module.build_sitemaps()["static.xml"]
+
+
+def test_every_loc_uses_the_www_host(sitemaps):
# The apex 301s to www. A that redirects burns a crawl per URL.
- assert "https://www.schoolcompare.co.uk" in sitemap
- assert "https://schoolcompare.co.uk" not in sitemap
+ # Checked across every file, index included, not just one.
+ for name, xml in sitemaps.items():
+ assert "https://www.schoolcompare.co.uk" in xml, name
+ assert "https://schoolcompare.co.uk" not in xml, name
-def test_school_with_results_is_listed(sitemap):
- assert "/school/100001-alpha-primary" in sitemap
+def test_school_with_results_is_listed(schools_child):
+ assert "/school/100001-alpha-primary" in schools_child
-def test_school_with_no_results_and_no_ofsted_is_omitted(sitemap):
+def test_school_with_no_results_and_no_ofsted_is_omitted(schools_child):
# Nothing for a search result to say about it. Submitting it spends crawl
# budget and drags the corpus-wide quality signal down.
- assert "/school/100002" not in sitemap
+ #
+ # Asserted against the child, not the index: the index carries no school
+ # URLs at all, so it would pass this trivially and prove nothing.
+ assert "/school/100002" not in schools_child
-def test_no_invented_priority_or_changefreq(sitemap):
+def test_no_invented_priority_or_changefreq(sitemaps):
# Google ignores both. They were noise dressed as signal.
- assert "" not in sitemap
- assert "" not in sitemap
+ for name, xml in sitemaps.items():
+ assert "" not in xml, name
+ assert "" not in xml, name
def test_ofsted_date_becomes_lastmod(monkeypatch):
@@ -70,7 +94,7 @@ def test_ofsted_date_becomes_lastmod(monkeypatch):
return base
monkeypatch.setattr(app_module, "load_school_data", _df)
- xml = app_module.build_sitemap()
+ xml = app_module.build_sitemaps()["schools-1.xml"]
assert "2024-03-14" in xml
@@ -85,10 +109,70 @@ def test_no_lastmod_invented_when_date_unknown(monkeypatch):
return df
monkeypatch.setattr(app_module, "load_school_data", _df)
- xml = app_module.build_sitemap()
+ # The child only. The index legitimately carries a lastmod, because there
+ # it means "when this sitemap file changed", which we do know.
+ xml = app_module.build_sitemaps()["schools-1.xml"]
assert "" not in xml
-def test_static_routes_are_listed(sitemap):
+def test_static_routes_are_listed(static_child):
for path in ("/", "/rankings", "/compare", "/admissions"):
- assert f"https://www.schoolcompare.co.uk{path}" in sitemap
+ assert f"https://www.schoolcompare.co.uk{path}" in static_child
+
+
+@pytest.fixture()
+def sitemaps(monkeypatch) -> dict:
+ from backend import app as app_module
+
+ monkeypatch.setattr(app_module, "load_school_data", _schools_df)
+ return app_module.build_sitemaps()
+
+
+def test_index_lists_each_child(sitemaps):
+ index = sitemaps["sitemap.xml"]
+ assert " entries only; mixing in is invalid.
+ assert "" not in sitemaps["sitemap.xml"]
+
+
+def test_index_does_not_list_itself(sitemaps):
+ assert "https://www.schoolcompare.co.uk/sitemap.xml" not in sitemaps["sitemap.xml"]
+
+
+def test_static_child_holds_the_static_routes(sitemaps):
+ static = sitemaps["static.xml"]
+ for path in ("/", "/rankings", "/compare", "/admissions"):
+ assert f"https://www.schoolcompare.co.uk{path}" in static
+
+
+def test_school_child_holds_the_schools(sitemaps):
+ assert "/school/100001-alpha-primary" in sitemaps["schools-1.xml"]
+
+
+def test_children_are_chunked_under_the_limit(monkeypatch):
+ # Sitemaps cap at 50,000 URLs per file. Chunk at 10,000 so a child stays
+ # small enough to eyeball in Search Console.
+ from backend import app as app_module
+ import pandas as _pd
+
+ rows = [
+ {"urn": 200000 + i, "school_name": f"School {i}", "year": 202425,
+ "rwm_expected_pct": 60.0, "attainment_8_score": None,
+ "ofsted_grade": 2.0, "ofsted_date": None}
+ for i in range(10_001)
+ ]
+ monkeypatch.setattr(app_module, "load_school_data", lambda: _pd.DataFrame(rows))
+
+ maps = app_module.build_sitemaps()
+ assert maps["schools-1.xml"].count("") == 10_000
+ assert maps["schools-2.xml"].count("") == 1
+
+
+def test_build_sitemap_still_returns_the_index(sitemap):
+ # lifespan and the admin endpoint call build_sitemap(); keep it working.
+ assert " {
+async function sitemapChildren(page: Page): Promise {
const res = await page.request.get('/sitemap.xml');
expect(res.ok()).toBeTruthy();
- const xml = await res.text();
+ const index = await res.text();
+ expect(index).toContain('([^<]+)<\/loc>/g)].map((m) => m[1]);
+}
- const urlCount = (xml.match(//g) ?? []).length;
- expect(urlCount, 'sitemap looks empty or truncated').toBeGreaterThan(1000);
+test('the sitemap index names children that all resolve', async ({ page }) => {
+ const index = await (await page.request.get('/sitemap.xml')).text();
+ // An index holds entries only; mixing in is invalid.
+ expect(index).not.toContain('');
- // 401559 (Cardiff) and 402426 (ACT Schools, Cardiff) were both submitted
- // before the England-only filter landed.
- expect(xml).not.toContain('/school/401559');
- expect(xml).not.toContain('/school/402426');
+ const locs = await sitemapChildren(page);
+ expect(locs.length).toBeGreaterThanOrEqual(2);
+
+ for (const loc of locs) {
+ expect(loc.startsWith('https://www.schoolcompare.co.uk/sitemaps/')).toBeTruthy();
+ const child = await page.request.get(new URL(loc).pathname);
+ expect(child.ok(), `${loc} should resolve`).toBeTruthy();
+ expect(await child.text()).toContain(' {
+ const locs = await sitemapChildren(page);
+
+ let total = 0;
+ for (const loc of locs) {
+ const xml = await (await page.request.get(new URL(loc).pathname)).text();
+ total += (xml.match(//g) ?? []).length;
+ // 401559 (Adamsdown, Cardiff) and 402426 (ACT Schools, Cardiff) were both
+ // submitted before the England-only filter landed.
+ expect(xml).not.toContain('/school/401559');
+ expect(xml).not.toContain('/school/402426');
+ }
+ expect(total, 'sitemap looks empty or truncated').toBeGreaterThan(1000);
+});
+
+test('the sitemap invents no priority or changefreq', async ({ page }) => {
+ const [first] = await sitemapChildren(page);
+ expect(first).toBeTruthy();
+
+ const xml = await (await page.request.get(new URL(first).pathname)).text();
+ // Google ignores both. They were noise dressed as signal.
+ expect(xml).not.toContain('');
+ expect(xml).not.toContain('');
});
/*
diff --git a/nextjs-app/app/sitemap.xml/route.ts b/nextjs-app/app/sitemap.xml/route.ts
index acb504d..9cfab89 100644
--- a/nextjs-app/app/sitemap.xml/route.ts
+++ b/nextjs-app/app/sitemap.xml/route.ts
@@ -1,32 +1,8 @@
-/**
- * Runtime proxy for /sitemap.xml → the FastAPI backend's generated sitemap.
- *
- * Like the /api/* proxy, this reads FASTAPI_URL at request time rather than
- * baking the backend host into the build, so one image works in every
- * environment. robots.ts points crawlers here.
- */
-
-import { NextResponse } from 'next/server';
+import { proxySitemap } from '@/lib/sitemapProxy';
export const dynamic = 'force-dynamic';
export const runtime = 'nodejs';
-function backendOrigin(): string {
- const base = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000/api';
- return base.replace(/\/api$/, '');
-}
-
export async function GET() {
- let upstream: Response;
- try {
- upstream = await fetch(`${backendOrigin()}/sitemap.xml`, { cache: 'no-store' });
- } catch {
- return new NextResponse('Sitemap temporarily unavailable', { status: 502 });
- }
-
- const body = await upstream.text();
- return new NextResponse(body, {
- status: upstream.status,
- headers: { 'content-type': upstream.headers.get('content-type') || 'application/xml' },
- });
+ return proxySitemap('/sitemap.xml');
}
diff --git a/nextjs-app/app/sitemaps/[...parts]/route.ts b/nextjs-app/app/sitemaps/[...parts]/route.ts
new file mode 100644
index 0000000..c540b82
--- /dev/null
+++ b/nextjs-app/app/sitemaps/[...parts]/route.ts
@@ -0,0 +1,24 @@
+import { NextResponse } from 'next/server';
+import { proxySitemap } from '@/lib/sitemapProxy';
+
+export const dynamic = 'force-dynamic';
+export const runtime = 'nodejs';
+
+/**
+ * Children are /sitemaps/static.xml and /sitemaps/schools-{n}.xml. The name is
+ * validated here rather than passed through, so this route cannot be used to
+ * reach arbitrary backend paths.
+ */
+const CHILD = /^(static|schools-\d+)\.xml$/;
+
+export async function GET(
+ _request: Request,
+ { params }: { params: Promise<{ parts: string[] }> },
+) {
+ const { parts } = await params;
+ const name = parts.join('/');
+ if (!CHILD.test(name)) {
+ return new NextResponse('Not found', { status: 404 });
+ }
+ return proxySitemap(`/sitemaps/${name}`);
+}
diff --git a/nextjs-app/lib/sitemapProxy.ts b/nextjs-app/lib/sitemapProxy.ts
new file mode 100644
index 0000000..dab6359
--- /dev/null
+++ b/nextjs-app/lib/sitemapProxy.ts
@@ -0,0 +1,29 @@
+/**
+ * Runtime proxy for the sitemap family → the FastAPI backend.
+ *
+ * Like the /api/* proxy, this reads FASTAPI_URL at request time rather than
+ * baking the backend host into the build, so one image works in every
+ * environment. robots.ts points crawlers at /sitemap.xml, which is the index;
+ * the index names children under /sitemaps/, which land on the same proxy.
+ */
+import { NextResponse } from 'next/server';
+
+function backendOrigin(): string {
+ const base = process.env.FASTAPI_URL || process.env.NEXT_PUBLIC_API_URL || 'http://localhost:8000/api';
+ return base.replace(/\/api$/, '');
+}
+
+export async function proxySitemap(path: string): Promise {
+ let upstream: Response;
+ try {
+ upstream = await fetch(`${backendOrigin()}${path}`, { cache: 'no-store' });
+ } catch {
+ return new NextResponse('Sitemap temporarily unavailable', { status: 502 });
+ }
+
+ const body = await upstream.text();
+ return new NextResponse(body, {
+ status: upstream.status,
+ headers: { 'content-type': upstream.headers.get('content-type') || 'application/xml' },
+ });
+}