# W1: Crawl Hygiene and Sitemap — Implementation Plan > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. **Goal:** Make every page declare one correct canonical URL, stop the homepage competing with itself across eleven search params, and turn the sitemap from a flat 27k-URL dump into a per-family index carrying honest `lastmod`. **Architecture:** Two independent halves. The frontend half is Next.js `Metadata` — a single shared canonical-host constant, `alternates.canonical` on every route, and `generateMetadata` on `/compare` so a parameterised comparison goes `noindex`. The backend half is `build_sitemap()` in `backend/app.py`, which grows a dataless-school filter and splits into a sitemap index with child sitemaps, proxied through Next by a catch-all route. **Tech Stack:** Next.js 15 App Router (`Metadata` / `generateMetadata`), Jest + jsdom for frontend unit tests, FastAPI + pandas for the sitemap, pytest with `monkeypatch`-injected DataFrames for backend tests, Playwright for the e2e gate. **Spec:** `docs/superpowers/specs/2026-08-20-seo-programme-design.md` (workstream W1) ## Global Constraints - **Canonical host is `https://www.schoolcompare.co.uk`.** The apex 301s to `www` at Cloudflare (verified 2026-08-20). Every canonical, `metadataBase`, sitemap `` and `robots.txt` `Sitemap:` line must use `www` so no canonical points at a redirect. - **Prerequisite:** branch `feat/england-only-corpus` must be merged first. This plan's sitemap counts assume the England-only corpus of 25,193 schools. - **Never push to `main`.** Feature branch and a PR, per `CLAUDE.md`. - **User-facing behaviour changes extend `e2e/tests/journeys.spec.ts` in the same PR** — the journeys gate staging and promotion. - **Backend test command** (there is no local pytest; this builds an isolated env): ```bash uv run --quiet --with-requirements requirements.txt --with pytest \ --with "httpx==0.27.0" python -m pytest backend/tests -q ``` - **Frontend test command:** `cd nextjs-app && npm test` - **Do not start a local server to test** (`CLAUDE.md`). Backend behaviour is tested through `TestClient`, frontend through Jest, integration through the staging e2e run. --- ### Task 1: One canonical host, used everywhere Today `metadataBase`, the school-page canonical, the backend `BASE_URL` and `robots.ts` all say `https://schoolcompare.co.uk`, which 301s to `www`. A canonical that points at a redirect is a wasted signal. This task introduces one constant and routes every producer of an absolute URL through it. **Files:** - Create: `nextjs-app/lib/site.ts` - Create: `nextjs-app/__tests__/lib/site.test.ts` - Modify: `nextjs-app/app/layout.tsx` (the `metadataBase` and `openGraph.url` keys) - Modify: `nextjs-app/app/robots.ts` (the `sitemap` key) - Modify: `nextjs-app/app/school/[slug]/page.tsx` (the two hardcoded `https://schoolcompare.co.uk` template strings in `generateMetadata`) - Modify: `backend/app.py:51` (`BASE_URL`) - Test: `backend/tests/test_sitemap.py` (created here, extended in Tasks 4–5) **Interfaces:** - Consumes: nothing from earlier tasks. - Produces: `SITE_URL: string` and `absoluteUrl(path: string): string` from `@/lib/site`. Tasks 2 and 3 import both. `backend.app.BASE_URL` keeps its name and type (`str`) so nothing else in `app.py` changes. - [ ] **Step 1: Write the failing test for the site-URL helper** Create `nextjs-app/__tests__/lib/site.test.ts`: ```typescript import { SITE_URL, absoluteUrl } from '@/lib/site'; describe('SITE_URL', () => { it('is the www host, which is the one that serves a 200', () => { // The apex 301s to www at Cloudflare. A canonical pointing at a redirect // is a wasted signal, so every absolute URL we emit must already be www. expect(SITE_URL).toBe('https://www.schoolcompare.co.uk'); }); it('has no trailing slash, so joins never double up', () => { expect(SITE_URL.endsWith('/')).toBe(false); }); }); describe('absoluteUrl', () => { it('joins a rooted path', () => { expect(absoluteUrl('/rankings')).toBe('https://www.schoolcompare.co.uk/rankings'); }); it('joins a path missing its leading slash', () => { expect(absoluteUrl('rankings')).toBe('https://www.schoolcompare.co.uk/rankings'); }); it('maps the site root to a bare trailing slash', () => { expect(absoluteUrl('/')).toBe('https://www.schoolcompare.co.uk/'); }); }); ``` - [ ] **Step 2: Run the test to verify it fails** Run: `cd nextjs-app && npm test -- __tests__/lib/site.test.ts` Expected: FAIL — `Cannot find module '@/lib/site'`. - [ ] **Step 3: Write the helper** Create `nextjs-app/lib/site.ts`: ```typescript /** * The one place the site's absolute origin is written down. * * The apex domain 301s to www at Cloudflare, so www is the host that actually * serves a 200. Canonicals, og:url, sitemap entries and the robots.txt * Sitemap: line must all agree with it — a canonical pointing at a redirect * makes Google resolve the hop before it can consolidate the signal. * * backend/app.py holds the same value as BASE_URL for the sitemap. The two are * asserted against each other by the e2e journeys rather than shared at build * time, because the backend and frontend ship as separate images. */ export const SITE_URL = 'https://www.schoolcompare.co.uk'; /** Absolute URL for a site-relative path. Tolerates a missing leading slash. */ export function absoluteUrl(path: string): string { const rooted = path.startsWith('/') ? path : `/${path}`; return `${SITE_URL}${rooted}`; } ``` - [ ] **Step 4: Run the test to verify it passes** Run: `cd nextjs-app && npm test -- __tests__/lib/site.test.ts` Expected: PASS (5 tests). - [ ] **Step 5: Point the frontend's absolute URLs at the helper** In `nextjs-app/app/layout.tsx`, add the import and replace the two hardcoded hosts: ```typescript import { SITE_URL } from '@/lib/site'; ``` ```typescript metadataBase: new URL(SITE_URL), ``` ```typescript openGraph: { type: 'website', title: 'schoolcompare | Compare School Performance', description: 'Compare primary and secondary school SATs and GCSE performance across England', url: SITE_URL, siteName: 'schoolcompare', }, ``` In `nextjs-app/app/robots.ts`: ```typescript import { absoluteUrl } from '@/lib/site'; ``` ```typescript sitemap: absoluteUrl('/sitemap.xml'), ``` In `nextjs-app/app/school/[slug]/page.tsx`, add the import and replace both occurrences inside `generateMetadata`: ```typescript import { absoluteUrl } from '@/lib/site'; ``` ```typescript url: absoluteUrl(canonicalPath), ``` ```typescript alternates: { canonical: absoluteUrl(canonicalPath), }, ``` - [ ] **Step 6: Point the backend's BASE_URL at the same host** In `backend/app.py`, replace line 51: ```python # Must match SITE_URL in nextjs-app/lib/site.ts. The apex 301s to www, and a # sitemap that redirects wastes a crawl on every URL it lists. BASE_URL = "https://www.schoolcompare.co.uk" ``` - [ ] **Step 7: Write the backend test pinning the host** Create `backend/tests/test_sitemap.py`: ```python """Tests for sitemap generation (spec 2026-08-20, workstream W1). The sitemap is built from the in-memory school DataFrame, so these inject a small frame via monkeypatch rather than touching a database. """ import numpy as np import pandas as pd import pytest def _schools_df() -> pd.DataFrame: """Two schools: one with results, one with neither results nor Ofsted.""" base = { "local_authority": "Testshire", "school_type": "Academy", "phase": "Primary", "year": 202425, "ofsted_date": None, } return pd.DataFrame( [ {**base, "urn": 100001, "school_name": "Alpha Primary", "rwm_expected_pct": 62.0, "attainment_8_score": np.nan, "ofsted_grade": 2.0}, {**base, "urn": 100002, "school_name": "Ghost Primary", "rwm_expected_pct": np.nan, "attainment_8_score": np.nan, "ofsted_grade": np.nan}, ] ) @pytest.fixture() def sitemap(monkeypatch) -> str: 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): # 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 ``` - [ ] **Step 8: Run both suites to verify they pass** Run: ```bash cd nextjs-app && npm test -- __tests__/lib/site.test.ts && cd .. uv run --quiet --with-requirements requirements.txt --with pytest \ --with "httpx==0.27.0" python -m pytest backend/tests/test_sitemap.py -q ``` Expected: frontend 5 passed; backend 1 passed. Note: `test_every_loc_uses_the_www_host` asserts absence of the apex string. Because `https://www.schoolcompare.co.uk` contains `schoolcompare.co.uk` but not `https://schoolcompare.co.uk`, the second assertion is precise as written. - [ ] **Step 9: Commit** ```bash git add nextjs-app/lib/site.ts nextjs-app/__tests__/lib/site.test.ts \ nextjs-app/app/layout.tsx nextjs-app/app/robots.ts \ "nextjs-app/app/school/[slug]/page.tsx" backend/app.py \ backend/tests/test_sitemap.py git commit -m "fix(seo): canonicalise on the www host, which is the one that serves 200 The apex 301s to www at Cloudflare, but metadataBase, the school-page canonical, robots.txt's Sitemap: line and the sitemap's own entries all named the apex. Every one of those pointed Google at a redirect." ``` --- ### Task 2: A canonical on every route, and one homepage instead of eleven params `app/page.tsx` accepts eleven search params and sets no canonical, so every filter combination is a crawlable near-duplicate of the page we most want to rank. `/rankings` and `/admissions` set no canonical either. **Files:** - Modify: `nextjs-app/app/page.tsx` (the `metadata` export) - Modify: `nextjs-app/app/rankings/page.tsx` (the `metadata` export) - Modify: `nextjs-app/app/admissions/page.tsx` (the `metadata` export) - Test: `nextjs-app/__tests__/app/metadata.test.ts` (created here, extended in Task 3) **Interfaces:** - Consumes: `absoluteUrl` from `@/lib/site` (Task 1). - Produces: nothing later tasks import. Task 3 adds cases to the same test file. - [ ] **Step 1: Write the failing tests** Create `nextjs-app/__tests__/app/metadata.test.ts`: ```typescript import { metadata as homeMetadata } from '@/app/page'; import { metadata as rankingsMetadata } from '@/app/rankings/page'; import { metadata as admissionsMetadata } from '@/app/admissions/page'; describe('canonical URLs', () => { it('the homepage canonicalises to the bare root', () => { // page.tsx reads eleven search params. Without this, every filter // combination is a crawlable near-duplicate of the one page we want to // rank for "compare schools". expect(homeMetadata.alternates?.canonical) .toBe('https://www.schoolcompare.co.uk/'); }); it('rankings canonicalises to the bare path', () => { expect(rankingsMetadata.alternates?.canonical) .toBe('https://www.schoolcompare.co.uk/rankings'); }); it('admissions canonicalises to the bare path', () => { expect(admissionsMetadata.alternates?.canonical) .toBe('https://www.schoolcompare.co.uk/admissions'); }); }); ``` - [ ] **Step 2: Run the tests to verify they fail** Run: `cd nextjs-app && npm test -- __tests__/app/metadata.test.ts` Expected: FAIL — all three receive `undefined`. - [ ] **Step 3: Add the canonicals** In `nextjs-app/app/page.tsx`, add the import and extend the metadata export: ```typescript import { absoluteUrl } from '@/lib/site'; ``` ```typescript export const metadata: Metadata = { title: { absolute: 'schoolcompare | Compare every school in England' }, description: 'Search and compare school performance across England', // This page reads eleven search params. They filter a result set; they do // not make a new document. Collapsing every combination onto "/" stops the // homepage competing with itself for its own head terms. alternates: { canonical: absoluteUrl('/') }, }; ``` In `nextjs-app/app/rankings/page.tsx`: ```typescript import { absoluteUrl } from '@/lib/site'; ``` ```typescript export const metadata: Metadata = { title: 'School Rankings', description: 'Top-ranked schools by SATs and GCSE performance across England', keywords: 'school rankings, top schools, best schools, KS2 rankings, KS4 rankings, school league tables', // Param forms (?metric=&local_authority=&year=&phase=) collapse here for // now. W3 replaces them with real indexable paths. alternates: { canonical: absoluteUrl('/rankings') }, }; ``` In `nextjs-app/app/admissions/page.tsx`: ```typescript import { absoluteUrl } from '@/lib/site'; ``` ```typescript export const metadata: Metadata = { title: 'School Admissions Guide', description: 'Understand the Primary and Secondary school admissions process in England, with live countdowns to every key deadline and National Offer Day.', alternates: { canonical: absoluteUrl('/admissions') }, }; ``` - [ ] **Step 4: Run the tests to verify they pass** Run: `cd nextjs-app && npm test -- __tests__/app/metadata.test.ts` Expected: PASS (3 tests). - [ ] **Step 5: Add the e2e assertion** Append to `e2e/tests/journeys.spec.ts`: ```typescript /* * Canonical URLs (spec 2026-08-20, W1). * * Every indexable route declares exactly one canonical, on the www host, with * no query string. The homepage's eleven search params filter a result set * rather than making a new document, so they all collapse onto "/". */ const CANONICAL_ROUTES: Array<[string, string]> = [ ['/', 'https://www.schoolcompare.co.uk/'], ['/rankings', 'https://www.schoolcompare.co.uk/rankings'], ['/admissions', 'https://www.schoolcompare.co.uk/admissions'], ]; for (const [path, expected] of CANONICAL_ROUTES) { test(`${path} declares exactly one canonical, on the www host`, async ({ page }) => { await page.goto(path); const hrefs = await page.locator('link[rel="canonical"]').evaluateAll( (els) => els.map((e) => e.getAttribute('href'))); expect(hrefs, `${path} should declare one canonical`).toHaveLength(1); expect(hrefs[0]).toBe(expected); }); } test('a filtered homepage still canonicalises to the bare root', async ({ page }) => { await page.goto('/?search=primary&phase=primary&sort=name&page=2'); const href = await page.locator('link[rel="canonical"]').first() .getAttribute('href'); expect(href).toBe('https://www.schoolcompare.co.uk/'); }); test('a school page canonicalises to its own slug on the www host', async ({ page }) => { const res = await page.request.get('/api/schools?search=primary&per_page=1'); expect(res.ok()).toBeTruthy(); const [first] = (await res.json()).schools ?? []; expect(first, 'no school available').toBeTruthy(); await page.goto(`/school/${first.urn}-x`); const href = await page.locator('link[rel="canonical"]').first() .getAttribute('href'); expect(href).toMatch(/^https:\/\/www\.schoolcompare\.co\.uk\/school\/\d+-/); }); ``` - [ ] **Step 6: Verify the e2e file still parses** Run: `cd e2e && npx playwright test --list` Expected: the five new tests appear in the listing; total rises by 5. - [ ] **Step 7: Commit** ```bash git add nextjs-app/app/page.tsx nextjs-app/app/rankings/page.tsx \ nextjs-app/app/admissions/page.tsx \ nextjs-app/__tests__/app/metadata.test.ts e2e/tests/journeys.spec.ts git commit -m "fix(seo): declare a canonical on every route The homepage read eleven search params and declared no canonical, so every filter combination was a crawlable near-duplicate of the page we most want to rank. Rankings and admissions declared none either." ``` --- ### Task 3: A parameterised comparison is not a document `/compare?urns=…` is an unbounded parameter space — 25,193 schools make ~317 million pairs, before triples. Bare `/compare` stays indexable: it is the landing page for "compare schools", the head term in cluster C1. **Files:** - Modify: `nextjs-app/app/compare/page.tsx` (replace the static `metadata` export with `generateMetadata`) - Test: `nextjs-app/__tests__/app/metadata.test.ts` (extend) **Interfaces:** - Consumes: `absoluteUrl` from `@/lib/site` (Task 1). - Produces: `generateMetadata({ searchParams }): Promise` exported from `app/compare/page.tsx`, replacing the `metadata` const. `searchParams` has the same `Promise<{ urns?: string; metric?: string }>` shape the default export already declares as `ComparePageProps`. - [ ] **Step 1: Write the failing tests** Append to `nextjs-app/__tests__/app/metadata.test.ts`: ```typescript import { generateMetadata as compareMetadata } from '@/app/compare/page'; describe('/compare indexability', () => { it('the bare compare page is indexable and canonical to itself', async () => { // This is the landing page for the "compare schools" head term. const meta = await compareMetadata({ searchParams: Promise.resolve({}) }); expect(meta.alternates?.canonical) .toBe('https://www.schoolcompare.co.uk/compare'); expect(meta.robots).toBeUndefined(); }); it('a comparison of specific schools is noindex, follow', async () => { // ~317 million pairs before triples. Indexing the parameter space would // swamp everything else in the corpus. const meta = await compareMetadata({ searchParams: Promise.resolve({ urns: '100001,100002' }), }); expect(meta.robots).toEqual({ index: false, follow: true }); }); it('a parameterised comparison still canonicalises to the bare path', async () => { // follow:true plus a canonical means the outbound links to each school // page still pass value even though this URL is not indexed. const meta = await compareMetadata({ searchParams: Promise.resolve({ urns: '100001,100002' }), }); expect(meta.alternates?.canonical) .toBe('https://www.schoolcompare.co.uk/compare'); }); }); ``` - [ ] **Step 2: Run the tests to verify they fail** Run: `cd nextjs-app && npm test -- __tests__/app/metadata.test.ts` Expected: FAIL — `compareMetadata is not a function`. - [ ] **Step 3: Replace the static metadata with generateMetadata** In `nextjs-app/app/compare/page.tsx`, add the import: ```typescript import { absoluteUrl } from '@/lib/site'; ``` Delete the whole `export const metadata: Metadata = { … };` block and put this in its place: ```typescript /** * Indexability depends on the query string, so this cannot be a static export. * * Bare /compare is the landing page for the "compare schools" head term and * stays indexable. /compare?urns=… is an unbounded parameter space — 25,193 * schools make ~317 million pairs — so it goes noindex. It stays `follow` and * keeps a canonical to the bare path, so the links out to each school page * still count. */ export async function generateMetadata( { searchParams }: ComparePageProps, ): Promise { const { urns } = await searchParams; const base: Metadata = { title: 'Compare Schools', description: 'Compare schools in England side by side — Ofsted inspections, KS2 and GCSE results against the England average, admissions odds and school community.', keywords: 'school comparison, compare schools, Ofsted comparison, school admissions, KS2 comparison, primary school performance', alternates: { canonical: absoluteUrl('/compare') }, }; if (!urns) return base; return { ...base, robots: { index: false, follow: true } }; } ``` - [ ] **Step 4: Run the tests to verify they pass** Run: `cd nextjs-app && npm test -- __tests__/app/metadata.test.ts` Expected: PASS (6 tests — 3 from Task 2, 3 from this task). - [ ] **Step 5: Add the e2e assertion** Append to `e2e/tests/journeys.spec.ts`: ```typescript test('a bare /compare is indexable, a parameterised one is not', async ({ page }) => { await page.goto('/compare'); await expect(page.locator('meta[name="robots"]')).toHaveCount(0); const [a, b] = await twoPrimaryUrns(page); await page.goto(`/compare?urns=${a},${b}`); const robots = await page.locator('meta[name="robots"]').first() .getAttribute('content'); expect(robots).toContain('noindex'); expect(robots).toContain('follow'); // noindex but follow: the links out to each school page still count, so the // canonical must still be present and point at the bare path. const canonical = await page.locator('link[rel="canonical"]').first() .getAttribute('href'); expect(canonical).toBe('https://www.schoolcompare.co.uk/compare'); }); ``` - [ ] **Step 6: Verify the e2e file parses** Run: `cd e2e && npx playwright test --list` Expected: the new test appears; total rises by 1. - [ ] **Step 7: Commit** ```bash git add nextjs-app/app/compare/page.tsx \ nextjs-app/__tests__/app/metadata.test.ts e2e/tests/journeys.spec.ts git commit -m "fix(seo): noindex parameterised comparisons, keep bare /compare 25,193 schools make ~317 million pairs. The bare page stays indexable as the landing page for the head term; the parameter space goes noindex, follow so its outbound links still count." ``` --- ### Task 4: Drop dataless schools and invented priorities from the sitemap `build_sitemap()` lists every URN and attaches `priority` and `changefreq`, both of which Google ignores, while omitting `lastmod`, which it reads. After the England-only change, 3,927 of 25,193 schools still have no results and no Ofsted — nothing for a search result to say. `lastmod` is set from each school's `ofsted_date` where one exists, and omitted otherwise. An always-`now` `lastmod` is a claim Google learns to distrust; an absent one honestly means "unknown". **Files:** - Modify: `backend/app.py:73-108` (`build_sitemap`) - Test: `backend/tests/test_sitemap.py` (extend) **Interfaces:** - Consumes: `BASE_URL` (Task 1). - Produces: `build_sitemap()` keeps its `() -> str` signature. Task 5 changes what it returns; the helper `_school_sitemap_rows(df) -> list[str]` added here is what Task 5 reuses. - [ ] **Step 1: Write the failing tests** Append to `backend/tests/test_sitemap.py`: ```python def test_school_with_results_is_listed(sitemap): assert "/school/100001-alpha-primary" in sitemap def test_school_with_no_results_and_no_ofsted_is_omitted(sitemap): # 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 def test_no_invented_priority_or_changefreq(sitemap): # Google ignores both. They were noise dressed as signal. assert "" not in sitemap assert "" not in sitemap def test_ofsted_date_becomes_lastmod(monkeypatch): from backend import app as app_module import datetime def _df(): base = _schools_df() base.loc[base["urn"] == 100001, "ofsted_date"] = datetime.date(2024, 3, 14) return base monkeypatch.setattr(app_module, "load_school_data", _df) xml = app_module.build_sitemap() assert "2024-03-14" in xml def test_no_lastmod_invented_when_date_unknown(monkeypatch): # An always-now lastmod is a claim Google learns to distrust. Absent # honestly means unknown. from backend import app as app_module def _df(): df = _schools_df() df["ofsted_date"] = None return df monkeypatch.setattr(app_module, "load_school_data", _df) xml = app_module.build_sitemap() assert "" not in xml def test_static_routes_are_listed(sitemap): for path in ("/", "/rankings", "/compare", "/admissions"): assert f"https://www.schoolcompare.co.uk{path}" in sitemap ``` - [ ] **Step 2: Run the tests to verify they fail** Run: ```bash uv run --quiet --with-requirements requirements.txt --with pytest \ --with "httpx==0.27.0" python -m pytest backend/tests/test_sitemap.py -q ``` Expected: FAIL — the dataless school is still listed, `` is present, `/admissions` is missing, and no `` is emitted. - [ ] **Step 3: Rewrite build_sitemap** Replace `build_sitemap` in `backend/app.py` (currently lines 73-108) with: ```python # Routes worth submitting that are not a school page. /admissions was missing # from the sitemap entirely despite being a static, indexable guide. STATIC_SITEMAP_PATHS = ("/", "/rankings", "/compare", "/admissions") def _has_publishable_data(row) -> bool: """True when a school page has something a search result could state. A school with no results in any year and no Ofsted grade renders an empty page. Submitting it spends crawl budget and drags the corpus-wide quality signal down, so it stays out of the sitemap. The page itself still resolves for anyone who has the URL. """ for field in ("rwm_expected_pct", "attainment_8_score", "ofsted_grade"): value = row.get(field) if value is not None and not pd.isna(value): return True return False def _url_element(loc: str, lastmod: str | None = None) -> str: """One entry. No priority or changefreq — Google ignores both.""" body = f"{loc}" if lastmod: body += f"{lastmod}" return f" {body}" def _school_sitemap_rows(df) -> list[str]: """A element per school that has something to show, URN order. lastmod comes from the school's Ofsted date where there is one and is omitted otherwise. An always-now lastmod is a claim Google learns to distrust; an absent one honestly means "unknown". """ if df.empty or "urn" not in df.columns or "school_name" not in df.columns: return [] rows: list[str] = [] seen: set[int] = set() # Latest row per URN first, so a school's most recent Ofsted date wins. ordered = df.sort_values("year", ascending=False) if "year" in df.columns else df for _, row in ordered.iterrows(): urn = int(row["urn"]) if urn in seen: continue seen.add(urn) if not _has_publishable_data(row): continue lastmod = None ofsted_date = row.get("ofsted_date") if ofsted_date is not None and not pd.isna(ofsted_date): lastmod = pd.Timestamp(ofsted_date).date().isoformat() rows.append(_url_element( BASE_URL + _school_url(urn, str(row["school_name"])), lastmod)) return rows def build_sitemap() -> str: """Generate sitemap XML from in-memory school data. Returns the XML string.""" 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) ``` Confirm `pandas` is imported in `backend/app.py` as `pd`. If it is not, add `import pandas as pd` alongside the other imports at the top of the file. - [ ] **Step 4: Run the tests to verify they pass** Run: ```bash 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 (7 tests). - [ ] **Step 5: Run the whole backend suite for regressions** Run: ```bash uv run --quiet --with-requirements requirements.txt --with pytest \ --with "httpx==0.27.0" python -m pytest backend/tests -q ``` Expected: PASS. Baseline before this plan was 54 passed; expect 61. - [ ] **Step 6: Commit** ```bash git add backend/app.py backend/tests/test_sitemap.py git commit -m "fix(seo): submit only school pages that have something to show Drops the 3,927 schools with neither results nor an Ofsted grade, adds /admissions which was never listed, replaces the invented priority and changefreq with a lastmod taken from each school's Ofsted date." ``` --- ### Task 5: Split the sitemap into a per-family index One flat file cannot tell you which page family Google is failing to index. Search Console reports coverage per submitted sitemap, so splitting by family 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 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`, `lifespan`) - Create: `nextjs-app/lib/sitemapProxy.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 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 `lifespan` and the admin endpoint keep working unchanged. - [ ] **Step 1: Write the failing tests** Append to `backend/tests/test_sitemap.py`: ```python @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 " XML. Populated on startup and by the admin # regenerate endpoint after a pipeline run. _sitemaps: dict[str, str] | None = None ``` Add below Task 4's helpers: ```python # 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() 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"] ``` Delete Task 4's single-`` `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. Replace the `/sitemap.xml` route and add the child route: ```python 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 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) ``` Update `/api/admin/regenerate-sitemap`: ```python @app.post("/api/admin/regenerate-sitemap") @limiter.limit("10/minute") async def regenerate_sitemap( request: Request, _: bool = Depends(verify_admin_api_key), ): """Rebuild and cache the sitemaps from current school data. Called by Airflow after data updates.""" global _sitemaps _sitemaps = build_sitemaps() n = sum(x.count("") for x in _sitemaps.values()) return {"status": "ok", "urls": n, "sitemaps": len(_sitemaps)} ``` In `lifespan`, change `global _sitemap_xml` to `global _sitemaps` and replace the sitemap block: ```python try: _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}") ``` 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()["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()`. - [ ] **Step 4: Run the tests to verify they pass** Run: ```bash 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 (14 tests). - [ ] **Step 5: Share one proxy between the index route and the child route** 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 /** * 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' }, }); } ``` Replace the body of `nextjs-app/app/sitemap.xml/route.ts` with: ```typescript import { proxySitemap } from '@/lib/sitemapProxy'; export const dynamic = 'force-dynamic'; export const runtime = 'nodejs'; export async function GET() { return proxySitemap('/sitemap.xml'); } ``` Create `nextjs-app/app/sitemaps/[...parts]/route.ts`: ```typescript 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}`); } ``` - [ ] **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 async function sitemapChildren(page: Page): Promise { const res = await page.request.get('/sitemap.xml'); expect(res.ok()).toBeTruthy(); const index = await res.text(); expect(index).toContain('([^<]+)<\/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 entries only; mixing in is invalid. expect(index).not.toContain(''); 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(''); }); ``` - [ ] **Step 7: Verify everything is green** Run: ```bash cd e2e && npx playwright test --list && cd .. 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; 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/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. 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." ``` --- ## After the plan 1. Open a PR from the feature branch. Do not merge until the England-only PR is in `main`, or the sitemap counts in the tests will not hold. 2. **Staging needs an Airflow run between the deploy and the e2e gate.** The England-only filter is a dbt change and staging's marts must be rebuilt before the journeys run, or the England-only tests fail and block promotion. 3. After staging is verified, resubmit `/sitemap.xml` in Search Console. The old flat sitemap should be removed from the property so its coverage report does not compete with the index's. 4. W0's Search Console baseline export should be captured **before** this lands, so the before/after comparison has a clean cut. ## Not in this plan W1 item 5 in the spec — a better template for the 3,927 English schools with no data — is deliberately excluded. Task 4 stops submitting them, which is the crawl-hygiene half. Giving them something worth showing is a product change, not a crawl fix, and belongs with its own design.