diff --git a/docs/superpowers/plans/2026-08-20-w1-crawl-hygiene.md b/docs/superpowers/plans/2026-08-20-w1-crawl-hygiene.md new file mode 100644 index 0000000..a6b09e8 --- /dev/null +++ b/docs/superpowers/plans/2026-08-20-w1-crawl-hygiene.md @@ -0,0 +1,1255 @@ +# 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. diff --git a/docs/superpowers/specs/2026-08-20-seo-programme-design.md b/docs/superpowers/specs/2026-08-20-seo-programme-design.md new file mode 100644 index 0000000..1986897 --- /dev/null +++ b/docs/superpowers/specs/2026-08-20-seo-programme-design.md @@ -0,0 +1,309 @@ +# SEO Programme — Design + +Date: 2026-08-20 +Status: awaiting review + +## Problem + +schoolcompare ranks second for "school compare" — an exact match for the +brand and the domain. It ranks poorly for "compare schools", "school +comparison" and "schools near me". The first is a naming artefact and +transfers to nothing; the rest are the queries that actually carry parent +demand. + +The cause is structural, not editorial. The site publishes five route +families: + + / /compare /rankings /admissions /school/[slug] + +Location intent has no landing page at all. Every competitor outranking us +on those queries wins with programmatic location pages: + +| Competitor | URL pattern | +|---------------|--------------------------------------------| +| School Guide | `/best-schools-in/manchester` | +| Locrating | `/the-best-primary-schools-in-Manchester_…`| +| FindMySchool | `/best-primary-schools/manchester` | +| Snobe | `/best-primary-schools/manchester` | +| School Atlas | `/guides/best-primary-schools-manchester` | + +"Schools near me" is a local-intent query. Google resolves it against the +user's coordinates and serves pages that are *about a place*. A national +homepage cannot win it. No title or description change fixes this; only +pages Google can localise will. + +## Baseline (measured 2026-08-20, production API) + +| Measure | Value | +|--------------------------------------------|---------| +| Unique schools | 27,229 | +| URLs in sitemap.xml | 27,232 | +| Schools with 2024/25 performance data | 21,266 | +| Schools with **no** current performance data| ~5,963 (22%) | +| Welsh establishments (all metrics null) | 1,569 | +| Overseas / offshore establishments | 467 | +| Static URLs in sitemap | 3 | +| Routes setting a canonical | 1 of 5 | + +Three findings from that table drive the plan. + +**We submit ~6,000 thin pages to Google.** `build_sitemap()` +(`backend/app.py:73`) enumerates every URN regardless of whether the school +has any data. Welsh establishments return `school_type: "Welsh +establishment"` with every performance metric, Ofsted grade and phase field +null. Overseas and offshore establishments ("BFPO Overseas Establishments", +"Gibraltar Overseas Establishments", "Jersey Offshore Establishments") are +in the local-authority list too. At 22% of the submitted corpus this is a +site-wide quality signal problem and a crawl-budget waste, not a rounding +error. + +W1 item 4 removes 2,036 of those — every non-England establishment — taking +the corpus to 25,193. The 3,927 that remain are English schools with no +current data: mostly newly opened, special, nursery or alternative provision. +Those are a template problem, not a corpus problem, and item 5 handles them +separately. + +**The homepage is its own competitor.** `app/page.tsx` accepts eleven search +params (`search`, `local_authority`, `school_type`, `phase`, `page`, +`postcode`, `radius`, `sort`, `gender`, `admissions_policy`, +`has_sixth_form`) and sets no canonical. Every filter combination is a +crawlable near-duplicate of the single page we are asking to rank for +"compare schools". + +**School pages are near-orphans.** Reachable from the sitemap and from site +search, but almost nothing links to them contextually, so they accrue no +internal authority. + +Also noted: the sitemap emits invented `priority` values and no `lastmod`. +Google ignores `priority` and `changefreq` entirely; `lastmod` is the field +it does read, and we omit it. + +## Keyword clusters + +Ranked by judgement of UK parent search behaviour and by the competitive +SERP evidence above. Google Search Console is connected, so cluster +priorities are to be re-derived from measured impressions before build +starts (see Workstream 0). + +**C1 — Head "compare" terms.** compare schools · school comparison · school +comparison tool · compare school performance · compare primary schools · +compare secondary schools · compare two schools + +**C2 — League tables and rankings.** primary school league tables · +secondary school league tables · school league tables 2026 · SATs results by +school · GCSE results by school · KS2 league tables · Progress 8 rankings · +best primary schools in [town] · top 10 primary schools in [LA] + +**C3 — Local / near me.** schools near me · primary schools near me · +secondary schools near me · best schools near me · good schools near me · +schools in [town] · primary schools in [LA] · schools near [postcode] · +[postcode] school catchment + +**C4 — Individual school long tail.** [school] ofsted · [school] SATs +results · [school] catchment area · [school] reviews · [school] URN + +**C5 — Admissions.** primary school admissions 2027 · national offer day +2027 · school application deadline · school admissions appeal · +oversubscription criteria · distance criteria school admissions · didn't get +first choice school · school admissions [LA] + +**C6 — Metric explainers.** what is a good SATs score · what is Progress 8 · +what is Attainment 8 · expected standard KS2 meaning · scaled score +explained · Ofsted grades explained · Ofsted report cards · pupil premium +explained + +**C7 — Head to head.** [school A] vs [school B] · academy vs community +school · grammar school vs comprehensive · faith school vs community school + +## Workstreams + +### W0 — Measure before touching anything + +Export a Google Search Console baseline: impressions, clicks, average +position and CTR by query and by page, for the trailing 16 months. Bucket +queries into C1–C7. This sets the counterfactual — without it, no later +claim about lift is defensible, because school-search traffic is strongly +seasonal (results day in December, offer day in March/April). + +Re-rank C1–C7 against measured impressions and adjust the sequence below if +the data disagrees with the judgement calls. + +### W1 — Crawl hygiene and index sanity + +Cheap, and it unblocks everything after it. Adding 5,000 pages on top of a +corpus that is 22% thin would compound the existing problem. + +1. Canonical on every route. `/`, `/rankings`, `/compare` and `/admissions` + currently set none. +2. The homepage canonicalises to `/` regardless of search params. +3. `/compare?urns=…` gets `noindex, follow` — it is an unbounded parameter + space with no standalone value. +4. **England only — DONE.** Wales, the Crown Dependencies, Gibraltar and the + service/overseas schools are removed from the corpus at the mart boundary, + not hidden at the view layer. `dim_school` and `dim_location` both exclude + `TypeOfEstablishment` in {25, 26, 30, 37} — Offshore schools, Service + children's education, Welsh establishment, British schools overseas — + listed once as `vars.non_england_school_type_codes` in `dbt_project.yml`. + That removes 2,036 establishments and 29 local authorities, and because + `build_sitemap()` reads the same marts, it drops them from the sitemap in + the same stroke. `assert_england_only_schools` fails the pipeline if a GIAS + refresh reintroduces them or if the two models drift apart. +5. Prune the remaining thin pages: exclude any school with no performance data + **and** no Ofsted record. Distinct from item 4 — these are English schools + with nothing yet to show, so the fix may be a better template rather than + removal. +6. Rebuild the sitemap as a sitemap **index**: one child per page family, + real `lastmod` from the data-load timestamp, `priority` and `changefreq` + dropped. + +### W2 — The location layer + +The dominant lever. `dim_location` already carries `town`, `county`, +`local_authority_name`, `parliamentary_constituency`, `latitude`, +`longitude` and `postcode`, so no new ingestion is required. + +Routes: + + /schools/[la] e.g. /schools/manchester + /schools/[la]/primary + /schools/[la]/secondary + /best-primary-schools/[town] + /best-secondary-schools/[town] + /schools/near/[outcode] e.g. /schools/near/m20 + /schools/near-me geolocating hub + +**Thin-page threshold: generate a town or outcode page only where at least +five schools have current performance data.** Below that, 301 to the parent +LA page. This is the single most important constraint in the workstream — +it is what separates a location layer from index bloat. + +Each page must earn its place with content a parent would actually use, not +a template shell: + +- H1 matching the query intent ("Best primary schools in Manchester") +- Counts framed usefully: "137 primary schools, 9 rated Outstanding" +- A ranked table of the top 20 on the headline metric +- Local average against the England average +- Ofsted grade distribution +- Map +- Links to neighbouring towns and to the parent LA +- An FAQ block (feeds `FAQPage` in W4) +- Links to every school page in scope — this is what de-orphans W1's corpus + +Sizing estimate: ~150 usable LAs × 3 ≈ 450; towns clearing the threshold +≈ 1,200 × 2 ≈ 2,400; outcodes ≈ 2,300. Roughly **5,000 new pages**, +comfortably inside a sitemap index and well under the per-file 50,000 limit. + +### W3 — Make rankings indexable + +`/rankings` is driven entirely by query params, so Google indexes +approximately one page where there should be hundreds. + + /rankings/[phase]/[metric] + /rankings/[phase]/[metric]/[la] + +The interactive filter UI stays; its state moves into real paths. Param +forms canonicalise to the clean path. This is the direct play for C2. + +### W4 — Structured data and internal linking + +- Replace the bare `EducationalOrganization` on school pages with `School`, + and populate it properly. +- `BreadcrumbList` site-wide. +- `ItemList` on every rankings and location page. +- `FAQPage` on admissions and on location pages. +- New school-page modules: "Other schools in [town]", "Nearby schools", + "Compare with similar schools". Each links out to W2 and W3 pages, which + is what circulates authority instead of stranding it. + +Explicitly **not** doing `Dataset` or `AggregateRating` — no review corpus +exists, and fabricating one would be both useless and a policy violation. + +### W5 — Admissions expansion + +One static page currently carries an entire cluster. + + /admissions/[la] per-authority deadlines and offer day + /admissions/appeals + /admissions/national-offer-day + +`school admissions [LA]` is high-intent and highly seasonal; per-authority +pages are the natural unit. + +### W6 — Explainer content + + /guides/progress-8 + /guides/attainment-8 + /guides/sats-scaled-scores + /guides/ofsted-grades + /guides/expected-standard + +Each links into the corresponding W3 rankings page. Cheap to build, and it +is what gives the metric vocabulary enough topical weight to support C1–C4. + +### W7 — Head-to-head pages + + /compare/[school-a]-vs-[school-b] + +C7 is uncontested and native to the product. It is also the easiest way to +destroy everything W1 fixes: 27,229 schools generate 370 million pairs. +**Curated pairs only** — same town, both with current data, both with real +search demand — capped in the low thousands. Gated behind evidence that W2 +is indexing cleanly. + +### W8 — Metadata rewrite for C1 + +Current homepage title is `schoolcompare | Compare every school in England`, +which spends the most valuable position on the brand. Rewrite the homepage, +rankings and compare titles and descriptions around C1 phrasing. Small +change, and the cheapest item in the programme. + +## Sequencing + + W0 → W1 → W2 → W3 → W4 → W5 → W6 → (W7 if W2 indexes cleanly) + +W1 before W2 is not negotiable: adding pages to a corpus that is 22% thin +compounds the problem rather than diluting it. + +This spec is a programme, not a single implementation plan. Each workstream +gets its own plan and its own PR; W2 will likely need several. Only W0 and W1 +are ready to plan against today — the rest should be re-read after W0's +Search Console baseline lands, because that data may reorder them. + +## Testing + +Per CLAUDE.md, user-facing behaviour changes extend the `e2e/` journeys in +the same PR. Each workstream adds: + +- W1: canonical present and correct on every route; `/compare?urns=` carries + `noindex`; sitemap excludes a known dataless URN. +- W2: a known LA, town and outcode page renders with the expected school + count; a below-threshold town redirects to its LA. +- W3: a clean rankings path renders; the param form canonicalises to it. +- W4: JSON-LD parses and validates against the declared types. + +## Risks + +**Index bloat.** The failure mode of every programmatic SEO programme. The +five-school threshold, the W1 prune and the W7 gate are the three controls. + +**Helpful-content exposure.** Google's stance on templated location pages +has hardened. The mitigation is that each page carries genuinely local +computed data — real counts, real distributions, real local-vs-national +comparison — rather than a name substituted into boilerplate. + +**Build cost.** School pages already use ISR with a 7-day revalidate and +`PRERENDER_SCHOOLS` gating full prerender. 5,000 more routes need the same +treatment; full static generation of 32,000 pages is likely impractical in +CI. + +**Seasonality.** Results day and offer day dominate the traffic curve. +Judging the programme on a mid-summer window would misread it in either +direction. W0's baseline must be year-on-year, not month-on-month. + +## Open questions + +1. Catchment areas are Locrating's moat and a strong C3 driver + (`[postcode] school catchment`). `fact_admissions` carries admission + distances. Is deriving approximate catchment a later workstream, or out + of scope? diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index b3fdbc0..f649d57 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -1473,3 +1473,104 @@ test('no single section dominates the height of a school page', async ({ page }) + sections.map((s) => `${s.id}=${s.h}`).join(', '), ).toBeLessThan(2.5); }); + +/* + * England-only corpus. + * + * GIAS ships the whole UK plus overseas and offshore establishments, none of + * which carry comparable DfE performance data. dim_school and dim_location + * exclude them (vars.non_england_school_type_codes in dbt_project.yml), which + * keeps them out of the site, the filter lists and the sitemap together. + * + * These assert the published surface, not the warehouse: the dbt test + * assert_england_only_schools guards the marts, and these guard what the + * environment actually serves once the marts have been rebuilt. + */ + +const WELSH_AUTHORITIES = [ + 'Blaenau Gwent', 'Bridgend', 'Caerphilly', 'Cardiff', 'Carmarthenshire', + 'Ceredigion', 'Conwy', 'Denbighshire', 'Flintshire', 'Gwynedd', + 'Isle of Anglesey', 'Merthyr Tydfil', 'Monmouthshire', 'Neath Port Talbot', + 'Newport', 'Pembrokeshire', 'Powys', 'Rhondda Cynon Taf', 'Swansea', + 'Torfaen', 'Vale of Glamorgan', 'Wrexham', +]; + +const NON_ENGLAND_AUTHORITIES = [ + 'BFPO Overseas Establishments', 'Fieldwork Overseas Establishments', + 'Gibraltar Overseas Establishments', 'Guernsey Offshore Establishments', + 'Isle of Man Offshore Establishments', 'Jersey Offshore Establishments', + 'Scotland Offshore Establishments', +]; + +const NON_ENGLAND_TYPES = [ + 'Welsh establishment', 'Offshore schools', + "Service children's education", 'British schools overseas', +]; + +test('the local authority filter offers no Welsh or overseas authority', async ({ page }) => { + const res = await page.request.get('/api/filters'); + expect(res.ok()).toBeTruthy(); + const { local_authorities: las } = await res.json(); + + expect(Array.isArray(las)).toBeTruthy(); + // Guards against the list being empty, which would pass the check below + // for the wrong reason. + expect(las.length).toBeGreaterThan(100); + + const leaked = [...WELSH_AUTHORITIES, ...NON_ENGLAND_AUTHORITIES] + .filter((la) => las.includes(la)); + expect(leaked, `non-England authorities still offered: ${leaked.join(', ')}`) + .toEqual([]); +}); + +test('the school type filter offers no non-England establishment type', async ({ page }) => { + const res = await page.request.get('/api/filters'); + expect(res.ok()).toBeTruthy(); + const { school_types: types } = await res.json(); + + expect(Array.isArray(types)).toBeTruthy(); + expect(types.length).toBeGreaterThan(10); + + const leaked = NON_ENGLAND_TYPES.filter((t) => types.includes(t)); + expect(leaked, `non-England types still offered: ${leaked.join(', ')}`) + .toEqual([]); +}); + +test('searching a Welsh authority by name returns no schools', async ({ page }) => { + // Cardiff held 144 Welsh establishments and nothing else, so the authority + // should now be absent from the corpus entirely rather than merely thinned. + const res = await page.request.get('/api/schools?local_authority=Cardiff&page_size=1'); + expect(res.ok()).toBeTruthy(); + const body = await res.json(); + expect(body.total ?? (body.schools ?? []).length).toBe(0); +}); + +test('a Welsh school URL 404s while an English one still resolves', async ({ page }) => { + // Paired on purpose: the Welsh assertion alone would also pass if the whole + // site were down, which is the failure this test most needs to distinguish. + const english = await page.request.get('/api/schools?search=primary&per_page=1'); + expect(english.ok()).toBeTruthy(); + const [first] = (await english.json()).schools ?? []; + expect(first, 'no English school available to compare against').toBeTruthy(); + + const good = await page.goto(`/school/${first.urn}-x`); + expect(good?.status(), 'an English school should still resolve').toBeLessThan(400); + + // Adamsdown Primary School, Cardiff — a Welsh establishment (URN 401559). + const welsh = await page.goto('/school/401559-adamsdown-primary-school'); + expect(welsh?.status(), 'a Welsh school should no longer resolve').toBe(404); +}); + +test('the sitemap submits no Welsh or overseas school', async ({ page }) => { + const res = await page.request.get('/sitemap.xml'); + expect(res.ok()).toBeTruthy(); + const xml = await res.text(); + + const urlCount = (xml.match(//g) ?? []).length; + expect(urlCount, 'sitemap looks empty or truncated').toBeGreaterThan(1000); + + // 401559 (Cardiff) and 402426 (ACT Schools, Cardiff) were both submitted + // before the England-only filter landed. + expect(xml).not.toContain('/school/401559'); + expect(xml).not.toContain('/school/402426'); +}); diff --git a/pipeline/transform/dbt_project.yml b/pipeline/transform/dbt_project.yml index ba23641..ff62b6f 100644 --- a/pipeline/transform/dbt_project.yml +++ b/pipeline/transform/dbt_project.yml @@ -11,6 +11,19 @@ seed-paths: ["seeds"] target-path: "target" clean-targets: ["target", "dbt_packages"] +# schoolcompare publishes England only. GIAS ships the whole UK plus overseas +# and offshore establishments, none of which have comparable DfE performance +# data — Wales does not publish on the English measures at all. Excluding them +# at the mart boundary keeps them out of the site, the filter lists and the +# sitemap together. Codes are TypeOfEstablishment, see seeds/gias_code_names.csv. +# 25 = Offshore schools (Jersey, Guernsey, Isle of Man, Gibraltar) +# 26 = Service children's education (BFPO, Fieldwork Overseas) +# 30 = Welsh establishment +# 37 = British schools overseas +# dim_school, dim_location and assert_england_only_schools all read this list. +vars: + non_england_school_type_codes: [25, 26, 30, 37] + models: school_compare: staging: diff --git a/pipeline/transform/models/marts/dim_location.sql b/pipeline/transform/models/marts/dim_location.sql index b05e8a7..3495b14 100644 --- a/pipeline/transform/models/marts/dim_location.sql +++ b/pipeline/transform/models/marts/dim_location.sql @@ -31,5 +31,9 @@ select else null end as longitude from {{ ref('stg_gias_establishments') }} s --- Must match dim_school's status filter exactly (the API inner-joins the two). +-- Must match dim_school's status and England filters exactly (the API +-- inner-joins the two). where s.status_code in (1, 3) +-- coalesce, not a bare NOT IN: a null type code would make the predicate +-- null and drop the row silently. Unknown type is not grounds for exclusion. +and coalesce(s.school_type_code, -1) not in ({{ var('non_england_school_type_codes') | join(', ') }}) diff --git a/pipeline/transform/models/marts/dim_school.sql b/pipeline/transform/models/marts/dim_school.sql index 86dfa2e..bcb246c 100644 --- a/pipeline/transform/models/marts/dim_school.sql +++ b/pipeline/transform/models/marts/dim_school.sql @@ -91,3 +91,8 @@ left join {{ ref('int_ofsted_latest') }} o on s.urn = o.urn -- 1 = Open; 3 = Open, but proposed to close (still operating; drops out when -- GIAS flips to Closed — marts fully rebuild each run). where s.status_code in (1, 3) +-- England only. dim_location must apply this filter identically (the API +-- inner-joins the two). See vars in dbt_project.yml for what the codes are. +-- coalesce, not a bare NOT IN: a null type code would make the predicate +-- null and drop the row silently. Unknown type is not grounds for exclusion. +and coalesce(s.school_type_code, -1) not in ({{ var('non_england_school_type_codes') | join(', ') }}) diff --git a/pipeline/transform/tests/assert_england_only_schools.sql b/pipeline/transform/tests/assert_england_only_schools.sql new file mode 100644 index 0000000..0356e29 --- /dev/null +++ b/pipeline/transform/tests/assert_england_only_schools.sql @@ -0,0 +1,28 @@ +-- Custom test: the published corpus is England only. +-- +-- GIAS ships the whole UK plus overseas and offshore establishments. None of +-- them carry comparable DfE performance data, so dim_school and dim_location +-- filter them out (see vars.non_england_school_type_codes in dbt_project.yml). +-- This test is the guard: a GIAS refresh that reintroduces them, or an edit +-- that drops the filter from one of the two models, fails the pipeline rather +-- than quietly republishing ~2,000 dataless pages to the site and the sitemap. + +select + urn, + school_name, + school_type_code +from {{ ref('dim_school') }} +where school_type_code in ({{ var('non_england_school_type_codes') | join(', ') }}) + +union all + +-- dim_location is inner-joined to dim_school by the API, so it must filter +-- identically. Anything here that dim_school does not have means the two +-- models have drifted apart. +select + l.urn, + null::text as school_name, + null::integer as school_type_code +from {{ ref('dim_location') }} l +left join {{ ref('dim_school') }} s on l.urn = s.urn +where s.urn is null