docs(seo): correct W1 plan's sitemap child routes
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m2s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 44s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 35s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 9s

Next only treats a whole bracketed path segment as dynamic, so the planned
app/sitemap-[...parts]/route.ts would have been read as a literal static
folder and never matched. Children move under /sitemaps/.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
This commit is contained in:
TudorandClaude Opus 5 committed 2026-08-20 22:09:00 +01:00
1 parent e6048c9ca6
commit 69f2201244
1 file changed
+108 -86
@@ -807,25 +807,33 @@ is what makes W2's location pages measurable when they land. 25,193 URLs is
still under the 50,000 per-file limit, so this is for the diagnostics, not the
size.
`/sitemap.xml` becomes the index. Children are `/sitemap-static.xml` and
`/sitemap-schools-{n}.xml`, 10,000 URLs each.
`/sitemap.xml` becomes the index. Children live under `/sitemaps/` —
`/sitemaps/static.xml` and `/sitemaps/schools-{n}.xml`, 10,000 URLs each.
**Why the children sit in their own directory:** Next.js only treats a path
segment as dynamic when the whole segment is bracketed. Verified in Next's
own router source — `UrlNode._insert` only reads a segment as dynamic if it
`startsWith('[') && endsWith(']')`, so a folder named `sitemap-[...parts]`
would be inserted as a *static* segment and never match. Putting the children
under `/sitemaps/` gives a clean `app/sitemaps/[...parts]/route.ts`.
**Files:**
- Modify: `backend/app.py` (`build_sitemap`, the `_sitemap_xml` cache, the
`/sitemap.xml` route, `/api/admin/regenerate-sitemap`)
`/sitemap.xml` route, `/api/admin/regenerate-sitemap`, `lifespan`)
- Create: `nextjs-app/lib/sitemapProxy.ts`
- Create: `nextjs-app/app/sitemap-[...parts]/route.ts`
- Create: `nextjs-app/app/sitemaps/[...parts]/route.ts`
- Modify: `nextjs-app/app/sitemap.xml/route.ts` (body moves to the shared proxy)
- Test: `backend/tests/test_sitemap.py` (extend)
**Interfaces:**
- Consumes: `_school_sitemap_rows`, `_url_element`, `STATIC_SITEMAP_PATHS` (Task 4).
- Produces: `build_sitemaps() -> dict[str, str]` mapping a filename
(`"sitemap.xml"`, `"sitemap-static.xml"`, `"sitemap-schools-1.xml"`) to its
XML. The module-level cache `_sitemap_xml: str | None` is replaced by
- Produces: `build_sitemaps() -> dict[str, str]` mapping a key
(`"sitemap.xml"`, `"static.xml"`, `"schools-1.xml"`) to its XML. The keys of
the children are bare filenames; the index prefixes them with `/sitemaps/`.
The module-level cache `_sitemap_xml: str | None` is replaced by
`_sitemaps: dict[str, str] | None`. `build_sitemap()` is kept as a thin
wrapper returning `build_sitemaps()["sitemap.xml"]` so Task 4's tests and
the startup path in `lifespan` keep working unchanged.
wrapper returning `build_sitemaps()["sitemap.xml"]` so `lifespan` and the
admin endpoint keep working unchanged.
- [ ] **Step 1: Write the failing tests**
@@ -843,8 +851,8 @@ def sitemaps(monkeypatch) -> dict:
def test_index_lists_each_child(sitemaps):
index = sitemaps["sitemap.xml"]
assert "<sitemapindex" in index
assert "https://www.schoolcompare.co.uk/sitemap-static.xml" in index
assert "https://www.schoolcompare.co.uk/sitemap-schools-1.xml" in index
assert "https://www.schoolcompare.co.uk/sitemaps/static.xml" in index
assert "https://www.schoolcompare.co.uk/sitemaps/schools-1.xml" in index
def test_index_carries_no_url_elements(sitemaps):
@@ -852,14 +860,18 @@ def test_index_carries_no_url_elements(sitemaps):
assert "<url>" not in sitemaps["sitemap.xml"]
def test_index_does_not_list_itself(sitemaps):
assert "<loc>https://www.schoolcompare.co.uk/sitemap.xml</loc>" not in sitemaps["sitemap.xml"]
def test_static_child_holds_the_static_routes(sitemaps):
static = sitemaps["sitemap-static.xml"]
static = sitemaps["static.xml"]
for path in ("/", "/rankings", "/compare", "/admissions"):
assert f"<loc>https://www.schoolcompare.co.uk{path}</loc>" in static
def test_school_child_holds_the_schools(sitemaps):
assert "/school/100001-alpha-primary" in sitemaps["sitemap-schools-1.xml"]
assert "/school/100001-alpha-primary" in sitemaps["schools-1.xml"]
def test_children_are_chunked_under_the_limit(monkeypatch):
@@ -877,8 +889,8 @@ def test_children_are_chunked_under_the_limit(monkeypatch):
monkeypatch.setattr(app_module, "load_school_data", lambda: _pd.DataFrame(rows))
maps = app_module.build_sitemaps()
assert maps["sitemap-schools-1.xml"].count("<url>") == 10_000
assert maps["sitemap-schools-2.xml"].count("<url>") == 1
assert maps["schools-1.xml"].count("<url>") == 10_000
assert maps["schools-2.xml"].count("<url>") == 1
def test_build_sitemap_still_returns_the_index(sitemap):
@@ -895,24 +907,21 @@ uv run --quiet --with-requirements requirements.txt --with pytest \
```
Expected: FAIL — `build_sitemaps` is not defined.
Note: `test_no_invented_priority_or_changefreq`, `test_static_routes_are_listed`,
`test_school_with_results_is_listed`, `test_school_with_no_results_and_no_ofsted_is_omitted`,
`test_ofsted_date_becomes_lastmod` and `test_no_lastmod_invented_when_date_unknown`
from Task 4 assert against `build_sitemap()`, which now returns the index and
no longer contains school URLs. Step 3 updates them to read the relevant child
from `build_sitemaps()`.
Note: the Task 4 tests assert against `build_sitemap()`, which now returns the
index and no longer contains school URLs. Step 3 updates them to read the
relevant child from `build_sitemaps()`.
- [ ] **Step 3: Implement the index**
In `backend/app.py`, replace the module-level cache declaration:
```python
# In-memory sitemap cache: filename -> XML. Populated on startup and by the
# admin regenerate endpoint after a pipeline run.
# 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
```
Add below `build_sitemap`'s helpers from Task 4:
Add below Task 4's helpers:
```python
# Sitemaps cap at 50,000 URLs per file. 10,000 keeps a child small enough to
@@ -921,21 +930,27 @@ Add below `build_sitemap`'s helpers from Task 4:
# 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([
'<?xml version="1.0" encoding="UTF-8"?>',
'<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">',
*rows,
"</urlset>",
])
def build_sitemaps() -> dict[str, str]:
"""Build the sitemap index and every child, keyed by filename."""
"""Build the sitemap index and every child, keyed by name."""
df = load_school_data()
def _urlset(rows: list[str]) -> str:
return "\n".join([
'<?xml version="1.0" encoding="UTF-8"?>',
'<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">',
*rows,
"</urlset>",
])
maps: dict[str, str] = {
"sitemap-static.xml": _urlset(
children: dict[str, str] = {
"static.xml": _urlset(
[_url_element(BASE_URL + path) for path in STATIC_SITEMAP_PATHS]),
}
@@ -945,24 +960,24 @@ def build_sitemaps() -> dict[str, str]:
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):
maps[f"sitemap-schools-{n}.xml"] = _urlset(chunk)
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 <url>, where
# it would be a claim about content we cannot support.
generated = datetime.now(timezone.utc).date().isoformat()
index_rows = [
f" <sitemap><loc>{BASE_URL}/{name}</loc>"
f" <sitemap><loc>{BASE_URL}{SITEMAP_CHILD_PREFIX}/{name}</loc>"
f"<lastmod>{generated}</lastmod></sitemap>"
for name in maps
for name in children
]
maps["sitemap.xml"] = "\n".join([
index = "\n".join([
'<?xml version="1.0" encoding="UTF-8"?>',
'<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">',
*index_rows,
"</sitemapindex>",
])
return maps
return {**children, "sitemap.xml": index}
def build_sitemap() -> str:
@@ -970,9 +985,8 @@ def build_sitemap() -> str:
return build_sitemaps()["sitemap.xml"]
```
Delete the old `build_sitemap` body from Task 4 (the one assembling a single
`<urlset>`); `_url_element`, `_has_publishable_data`, `_school_sitemap_rows`
and `STATIC_SITEMAP_PATHS` all stay.
Delete Task 4's single-`<urlset>` `build_sitemap` body; `_url_element`,
`_has_publishable_data`, `_school_sitemap_rows` and `STATIC_SITEMAP_PATHS` stay.
Ensure `from datetime import datetime, timezone` is imported at the top of
`backend/app.py`; add it if absent.
@@ -980,16 +994,16 @@ Ensure `from datetime import datetime, timezone` is imported at the top of
Replace the `/sitemap.xml` route and add the child route:
```python
def _serve_sitemap(filename: str) -> Response:
def _serve_sitemap(name: str) -> Response:
global _sitemaps
if _sitemaps is None:
try:
_sitemaps = build_sitemaps()
except Exception as e:
raise HTTPException(status_code=503, detail=f"Sitemap unavailable: {e}")
if filename not in _sitemaps:
if name not in _sitemaps:
raise HTTPException(status_code=404, detail="No such sitemap")
return Response(content=_sitemaps[filename], media_type="application/xml")
return Response(content=_sitemaps[name], media_type="application/xml")
@app.get("/sitemap.xml")
@@ -998,10 +1012,10 @@ async def sitemap_xml():
return _serve_sitemap("sitemap.xml")
@app.get("/sitemap-{name}.xml")
@app.get("/sitemaps/{name}")
async def sitemap_child(name: str):
"""Serve a child sitemap (static, or schools-N)."""
return _serve_sitemap(f"sitemap-{name}.xml")
"""Serve a child sitemap (static.xml, or schools-N.xml)."""
return _serve_sitemap(name)
```
Update `/api/admin/regenerate-sitemap`:
@@ -1020,7 +1034,8 @@ async def regenerate_sitemap(
return {"status": "ok", "urls": n, "sitemaps": len(_sitemaps)}
```
In `lifespan`, replace the sitemap block:
In `lifespan`, change `global _sitemap_xml` to `global _sitemaps` and replace
the sitemap block:
```python
try:
@@ -1031,14 +1046,12 @@ In `lifespan`, replace the sitemap block:
print(f"Warning: sitemap build failed on startup: {e}")
```
and change its `global _sitemap_xml` declaration to `global _sitemaps`.
Update the six Task 4 tests to read from the right child. In
`backend/tests/test_sitemap.py`, change `test_school_with_results_is_listed`,
Update the Task 4 tests to read the right child: change
`test_school_with_results_is_listed`,
`test_school_with_no_results_and_no_ofsted_is_omitted`,
`test_ofsted_date_becomes_lastmod` and `test_no_lastmod_invented_when_date_unknown`
to assert against `build_sitemaps()["sitemap-schools-1.xml"]`, and
`test_static_routes_are_listed` against `build_sitemaps()["sitemap-static.xml"]`.
to assert against `build_sitemaps()["schools-1.xml"]`, and
`test_static_routes_are_listed` against `build_sitemaps()["static.xml"]`.
`test_no_invented_priority_or_changefreq` and `test_every_loc_uses_the_www_host`
should assert across every value in `build_sitemaps()`.
@@ -1049,12 +1062,12 @@ Run:
uv run --quiet --with-requirements requirements.txt --with pytest \
--with "httpx==0.27.0" python -m pytest backend/tests/test_sitemap.py -q
```
Expected: PASS (13 tests).
Expected: PASS (14 tests).
- [ ] **Step 5: Replace the Next proxy with a catch-all**
- [ ] **Step 5: Share one proxy between the index route and the child route**
The index and the children need the same proxy, and a catch-all cannot match
`/sitemap.xml` itself, so both routes stay and share one handler. Extract it to
The index and the children need the same proxy, and the catch-all cannot match
`/sitemap.xml` itself, so both routes stay and share one handler. Create
`nextjs-app/lib/sitemapProxy.ts`:
```typescript
@@ -1064,7 +1077,7 @@ The index and the children need the same proxy, and a catch-all cannot match
* 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 the children, which land on the same proxy.
* the index names children under /sitemaps/, which land on the same proxy.
*/
import { NextResponse } from 'next/server';
@@ -1073,10 +1086,10 @@ function backendOrigin(): string {
return base.replace(/\/api$/, '');
}
export async function proxySitemap(filename: string): Promise<NextResponse> {
export async function proxySitemap(path: string): Promise<NextResponse> {
let upstream: Response;
try {
upstream = await fetch(`${backendOrigin()}/${filename}`, { cache: 'no-store' });
upstream = await fetch(`${backendOrigin()}${path}`, { cache: 'no-store' });
} catch {
return new NextResponse('Sitemap temporarily unavailable', { status: 502 });
}
@@ -1098,11 +1111,11 @@ export const dynamic = 'force-dynamic';
export const runtime = 'nodejs';
export async function GET() {
return proxySitemap('sitemap.xml');
return proxySitemap('/sitemap.xml');
}
```
Create `nextjs-app/app/sitemap-[...parts]/route.ts`:
Create `nextjs-app/app/sitemaps/[...parts]/route.ts`:
```typescript
import { NextResponse } from 'next/server';
@@ -1112,9 +1125,9 @@ export const dynamic = 'force-dynamic';
export const runtime = 'nodejs';
/**
* Children are /sitemap-static.xml and /sitemap-schools-{n}.xml. The segment
* pattern is validated here rather than passed through, so this route cannot
* be used to reach arbitrary backend paths.
* 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$/;
@@ -1127,31 +1140,35 @@ export async function GET(
if (!CHILD.test(name)) {
return new NextResponse('Not found', { status: 404 });
}
return proxySitemap(`sitemap-${name}`);
return proxySitemap(`/sitemaps/${name}`);
}
```
- [ ] **Step 6: Add the e2e assertions**
- [ ] **Step 6: Update the e2e assertions**
Replace the `the sitemap submits no Welsh or overseas school` test in
`e2e/tests/journeys.spec.ts` with this block, which keeps its assertions and
follows the index to its children:
```typescript
test('the sitemap index names children that all resolve', async ({ page }) => {
async function sitemapChildren(page: Page): Promise<string[]> {
const res = await page.request.get('/sitemap.xml');
expect(res.ok()).toBeTruthy();
const index = await res.text();
expect(index).toContain('<sitemapindex');
return [...index.matchAll(/<loc>([^<]+)<\/loc>/g)].map((m) => m[1]);
}
test('the sitemap index names children that all resolve', async ({ page }) => {
const index = await (await page.request.get('/sitemap.xml')).text();
// An index holds <sitemap> entries only; mixing in <url> is invalid.
expect(index).not.toContain('<url>');
const locs = [...index.matchAll(/<loc>([^<]+)<\/loc>/g)].map((m) => m[1]);
const locs = await sitemapChildren(page);
expect(locs.length).toBeGreaterThanOrEqual(2);
for (const loc of locs) {
expect(loc.startsWith('https://www.schoolcompare.co.uk/')).toBeTruthy();
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('<urlset');
@@ -1159,8 +1176,7 @@ test('the sitemap index names children that all resolve', async ({ page }) => {
});
test('the sitemap submits no Welsh or overseas school', async ({ page }) => {
const index = await (await page.request.get('/sitemap.xml')).text();
const locs = [...index.matchAll(/<loc>([^<]+)<\/loc>/g)].map((m) => m[1]);
const locs = await sitemapChildren(page);
let total = 0;
for (const loc of locs) {
@@ -1174,18 +1190,18 @@ test('the sitemap submits no Welsh or overseas school', async ({ page }) => {
expect(total, 'sitemap looks empty or truncated').toBeGreaterThan(1000);
});
test('the sitemap invents no priority, changefreq, or lastmod it cannot support', async ({ page }) => {
const index = await (await page.request.get('/sitemap.xml')).text();
const firstChild = index.match(/<loc>([^<]+)<\/loc>/)?.[1];
expect(firstChild).toBeTruthy();
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(firstChild!).pathname)).text();
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('<priority>');
expect(xml).not.toContain('<changefreq>');
});
```
- [ ] **Step 7: Verify the e2e file parses and the whole backend suite is green**
- [ ] **Step 7: Verify everything is green**
Run:
```bash
@@ -1194,20 +1210,26 @@ uv run --quiet --with-requirements requirements.txt --with pytest \
--with "httpx==0.27.0" python -m pytest backend/tests -q
cd nextjs-app && npm test && npx next build --no-lint
```
Expected: e2e listing parses; backend suite green; Jest green; Next build
succeeds with both sitemap routes present.
Expected: e2e listing parses; backend suite green; Jest green; the Next build
succeeds and lists both `/sitemap.xml` and `/sitemaps/[...parts]` as routes.
The build output is the check that the dynamic segment resolves — a folder
Next reads as static would simply not appear as a dynamic route.
- [ ] **Step 8: Commit**
```bash
git add backend/app.py backend/tests/test_sitemap.py \
nextjs-app/lib/sitemapProxy.ts nextjs-app/app/sitemap.xml/route.ts \
"nextjs-app/app/sitemap-[...parts]/route.ts" e2e/tests/journeys.spec.ts
"nextjs-app/app/sitemaps/[...parts]/route.ts" e2e/tests/journeys.spec.ts
git commit -m "feat(seo): split the sitemap into a per-family index
Search Console reports coverage per submitted sitemap, so one file per page
family is what will make W2's location pages measurable when they land. The
index's lastmod is generation time, which is the correct semantic there."
index's lastmod is generation time, which is the correct semantic there.
Children sit under /sitemaps/ because Next only treats a whole bracketed path
segment as dynamic; a route folder named sitemap-[...parts] would be read as a
literal static segment and never match."
```
---