school_compare/docs/superpowers/plans/2026-08-20-w1-crawl-hygiene.md

# 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 `<loc>` 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 <loc> 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 <loc> 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 <loc> 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 <loc> 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<Metadata>` 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<Metadata> {
  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 "<priority>" not in sitemap
    assert "<changefreq>" 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 "<lastmod>2024-03-14</lastmod>" 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 "<lastmod>" not in xml


def test_static_routes_are_listed(sitemap):
    for path in ("/", "/rankings", "/compare", "/admissions"):
        assert f"<loc>https://www.schoolcompare.co.uk{path}</loc>" 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, `<priority>` is present,
`/admissions` is missing, and no `<lastmod>` 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 <url> entry. No priority or changefreq — Google ignores both."""
    body = f"<loc>{loc}</loc>"
    if lastmod:
        body += f"<lastmod>{lastmod}</lastmod>"
    return f"  <url>{body}</url>"


def _school_sitemap_rows(df) -> list[str]:
    """A <url> 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 = ['<?xml version="1.0" encoding="UTF-8"?>',
             '<urlset xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">']
    lines.extend(_url_element(BASE_URL + path) for path in STATIC_SITEMAP_PATHS)
    lines.extend(_school_sitemap_rows(df))
    lines.append("</urlset>")
    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 "<sitemapindex" 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):
    # A sitemap index holds <sitemap> entries only; mixing in <url> is invalid.
    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["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["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("<url>") == 10_000
    assert maps["schools-2.xml"].count("<url>") == 1


def test_build_sitemap_still_returns_the_index(sitemap):
    # lifespan and the admin endpoint call build_sitemap(); keep it working.
    assert "<sitemapindex" 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 — `build_sitemaps` is not defined.

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: name -> 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([
        '<?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 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 <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}{SITEMAP_CHILD_PREFIX}/{name}</loc>"
        f"<lastmod>{generated}</lastmod></sitemap>"
        for name in children
    ]
    index = "\n".join([
        '<?xml version="1.0" encoding="UTF-8"?>',
        '<sitemapindex xmlns="http://www.sitemaps.org/schemas/sitemap/0.9">',
        *index_rows,
        "</sitemapindex>",
    ])
    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-`<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.

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("<url>") 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("<url>") 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<NextResponse> {
  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<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 = 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('<urlset');
  }
});

test('the sitemap submits no Welsh or overseas school', async ({ page }) => {
  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(/<url>/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('<priority>');
  expect(xml).not.toContain('<changefreq>');
});
```

- [ ] **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.