/schools/[place]/[phase] shipped as working, canonical routes — and reached nothing. The sitemap emitted one URL per registry entry and the registry had no phase dimension, so ~950 pages were in no sitemap. PlaceView did not link them either, so they had no internal path in at all.
That is the exact query shape the W0 baseline showed — primary schools in beccles, secondary schools in brentwood, colleges in solihull. W2 was built for that demand and then hid from it.
Fix
Place now carries phase_urns, so the per-phase threshold is applied without re-querying:
Sitemap emits a variant wherever a phase clears the threshold on its own — a town with 30 primaries and 2 secondaries gets a primary page and no secondary one.
API exposes the qualifying phases, so the page links only variants that exist rather than 404s.
PlaceView links them from the bare place page, and does not link sideways from a variant to itself.
Outcodes excluded — nobody searches primary schools in SW11, and those routes do not exist.
Verification
Backend 100 passed · frontend 247 · tsc --noEmit clean · next build green · 83 e2e journeys, including one that clicks through from a place page to its variant and one asserting variants reach the sitemap.
Expect the count to rise
Staging currently reports 25,718 URLs across 7 sitemaps. After this, expect roughly 26,700 as the phase variants appear in places-1.xml.
**Includes #115** (the locality-collision fix) — merge this instead of, or after, that one; the branch carries both.
## The defect
Verifying the staging sitemap after W2 deployed showed **zero phase variants submitted**:
```
static 4 urls
schools-1 10000
schools-2 10000
schools-3 3069
places-1 925 <- towns + localities + authorities only
outcodes-1 1720
```
`/schools/[place]/[phase]` shipped as working, canonical routes — and reached nothing. The sitemap emitted one URL per registry entry and the registry had no phase dimension, so ~950 pages were in no sitemap. `PlaceView` did not link them either, so they had **no internal path in at all**.
That is the exact query shape the W0 baseline showed — `primary schools in beccles`, `secondary schools in brentwood`, `colleges in solihull`. W2 was built for that demand and then hid from it.
## Fix
`Place` now carries `phase_urns`, so the per-phase threshold is applied without re-querying:
- **Sitemap** emits a variant wherever a phase clears the threshold on its own — a town with 30 primaries and 2 secondaries gets a primary page and no secondary one.
- **API** exposes the qualifying phases, so the page links only variants that exist rather than 404s.
- **PlaceView** links them from the bare place page, and does not link sideways from a variant to itself.
- **Outcodes excluded** — nobody searches `primary schools in SW11`, and those routes do not exist.
## Verification
Backend 100 passed · frontend 247 · `tsc --noEmit` clean · `next build` green · 83 e2e journeys, including one that clicks through from a place page to its variant and one asserting variants reach the sitemap.
## Expect the count to rise
Staging currently reports **25,718** URLs across 7 sitemaps. After this, expect roughly **26,700** as the phase variants appear in `places-1.xml`.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
/schools/[place]/[phase] shipped as routes but reached nothing. The sitemap
emitted one URL per registry entry and the registry had no phase dimension, so
~950 pages were absent from every sitemap — and PlaceView did not link them
either, leaving them reachable by nothing at all.
That is the query shape the baseline actually showed: 'primary schools in
beccles', 'secondary schools in brentwood'. Publishing the routes without a
path in meant building for the demand and then hiding from it.
Place now carries phase_urns so the per-phase threshold can be applied without
re-querying, the sitemap emits a variant wherever a phase clears the threshold
on its own, and the API exposes the qualifying phases so the place page links
only variants that exist. Outcodes are excluded: nobody searches 'primary
schools in SW11' and those routes do not exist.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
tudor
merged commit 6c5db0c266 into main2026-08-21 19:46:55 +00:00
This PR adds phase (primary/secondary) variant pages for place listings — sitemap rows, an API phases field, and frontend nav links — driven by a per-phase URN threshold on the Place dataclass. The core primary/secondary URN-splitting logic is sound and well-tested for the town kind, but the phase-variant feature was only wired up correctly for towns/localities; it silently leaks into the authority and outcode kinds, which have no corresponding [phase] route, producing broken or wrong-place links.
🔴 Severe (blocks merge)
backend/app.py: _place_sitemap_rows (line ~211) only special-cases kind == "outcode" to skip phase variants, but the "places" sitemap group also includes authority. There is no /schools/authority/[la]/[phase] route in the Next.js app (only /schools/authority/[la]), so every authority that clears the per-phase threshold gets /schools/authority/{slug}/primary and/or /secondary submitted to the sitemap — URLs that are guaranteed 404s when Googlebot crawls them, defeating the PR's own indexation goal.
nextjs-app/components/places/PlaceView.tsx: The new phase nav (line ~95) builds hrefs as /schools/${place.slug}/${ph} regardless of place.kind, instead of using the existing placeUrl(kind, slug, phase) helper in lib/places.ts which already handles the per-kind prefixes. For outcode places this links to a route that doesn't exist (404). For authority places it links into the town/locality namespace (/schools/[place]/[phase], whose resolver only tries fetchPlace('town', ...) / fetchPlace('locality', ...)); per this file's own comment, ~67 town names collide with an authority name, so an authority's phase link can silently render a different place's (the colliding town's) schools instead of 404ing. This is reachable in practice because backend/places.py computes phase_urns (and thus Place.publishes_phase) for authority and outcode places too, so get_place's phases field is non-empty for those kinds and the nav renders on every authority/outcode page that clears the threshold — nothing scopes phase support to town/locality the way the outcode sitemap exclusion and the outcode page's own comment ("No phase variants... the variants would be pages without demand") say it should.
🟡 Minor
backend/tests/test_sitemap.py: New tests only exercise the town and outcode sitemap cases (brentwood); nothing covers authority, which is the kind actually broken (see above) since it's neither given phase variants intentionally nor excluded like outcode.
e2e/tests/journeys.spec.ts: The new phase-link e2e test only ever picks places.find(p => p.kind === 'town'), so it can't catch the broken/mismatched links produced for authority or outcode place kinds.
## 🤖 AI Code Review (Claude Code)
This PR adds phase (primary/secondary) variant pages for place listings — sitemap rows, an API `phases` field, and frontend nav links — driven by a per-phase URN threshold on the `Place` dataclass. The core primary/secondary URN-splitting logic is sound and well-tested for the `town` kind, but the phase-variant feature was only wired up correctly for towns/localities; it silently leaks into the `authority` and `outcode` kinds, which have no corresponding `[phase]` route, producing broken or wrong-place links.
### 🔴 Severe (blocks merge)
- **backend/app.py**: `_place_sitemap_rows` (line ~211) only special-cases `kind == "outcode"` to skip phase variants, but the "places" sitemap group also includes `authority`. There is no `/schools/authority/[la]/[phase]` route in the Next.js app (only `/schools/authority/[la]`), so every authority that clears the per-phase threshold gets `/schools/authority/{slug}/primary` and/or `/secondary` submitted to the sitemap — URLs that are guaranteed 404s when Googlebot crawls them, defeating the PR's own indexation goal.
- **nextjs-app/components/places/PlaceView.tsx**: The new phase nav (line ~95) builds hrefs as `/schools/${place.slug}/${ph}` regardless of `place.kind`, instead of using the existing `placeUrl(kind, slug, phase)` helper in lib/places.ts which already handles the per-kind prefixes. For `outcode` places this links to a route that doesn't exist (404). For `authority` places it links into the town/locality namespace (`/schools/[place]/[phase]`, whose resolver only tries `fetchPlace('town', ...)` / `fetchPlace('locality', ...)`); per this file's own comment, ~67 town names collide with an authority name, so an authority's phase link can silently render a different place's (the colliding town's) schools instead of 404ing. This is reachable in practice because `backend/places.py` computes `phase_urns` (and thus `Place.publishes_phase`) for authority and outcode places too, so `get_place`'s `phases` field is non-empty for those kinds and the nav renders on every authority/outcode page that clears the threshold — nothing scopes phase support to town/locality the way the outcode sitemap exclusion and the outcode page's own comment ("No phase variants... the variants would be pages without demand") say it should.
### 🟡 Minor
- **backend/tests/test_sitemap.py**: New tests only exercise the `town` and `outcode` sitemap cases (brentwood); nothing covers `authority`, which is the kind actually broken (see above) since it's neither given phase variants intentionally nor excluded like outcode.
- **e2e/tests/journeys.spec.ts**: The new phase-link e2e test only ever picks `places.find(p => p.kind === 'town')`, so it can't catch the broken/mismatched links produced for `authority` or `outcode` place kinds.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Includes #115 (the locality-collision fix) — merge this instead of, or after, that one; the branch carries both.
The defect
Verifying the staging sitemap after W2 deployed showed zero phase variants submitted:
/schools/[place]/[phase]shipped as working, canonical routes — and reached nothing. The sitemap emitted one URL per registry entry and the registry had no phase dimension, so ~950 pages were in no sitemap.PlaceViewdid not link them either, so they had no internal path in at all.That is the exact query shape the W0 baseline showed —
primary schools in beccles,secondary schools in brentwood,colleges in solihull. W2 was built for that demand and then hid from it.Fix
Placenow carriesphase_urns, so the per-phase threshold is applied without re-querying:primary schools in SW11, and those routes do not exist.Verification
Backend 100 passed · frontend 247 ·
tsc --noEmitclean ·next buildgreen · 83 e2e journeys, including one that clicks through from a place page to its variant and one asserting variants reach the sitemap.Expect the count to rise
Staging currently reports 25,718 URLs across 7 sitemaps. After this, expect roughly 26,700 as the phase variants appear in
places-1.xml.🤖 Generated with Claude Code
https://claude.ai/code/session_015mWQnpye9F299NVRCCSRvj
🤖 AI Code Review (Claude Code)
This PR adds phase (primary/secondary) variant pages for place listings — sitemap rows, an API
phasesfield, and frontend nav links — driven by a per-phase URN threshold on thePlacedataclass. The core primary/secondary URN-splitting logic is sound and well-tested for thetownkind, but the phase-variant feature was only wired up correctly for towns/localities; it silently leaks into theauthorityandoutcodekinds, which have no corresponding[phase]route, producing broken or wrong-place links.🔴 Severe (blocks merge)
_place_sitemap_rows(line ~211) only special-caseskind == "outcode"to skip phase variants, but the "places" sitemap group also includesauthority. There is no/schools/authority/[la]/[phase]route in the Next.js app (only/schools/authority/[la]), so every authority that clears the per-phase threshold gets/schools/authority/{slug}/primaryand/or/secondarysubmitted to the sitemap — URLs that are guaranteed 404s when Googlebot crawls them, defeating the PR's own indexation goal./schools/${place.slug}/${ph}regardless ofplace.kind, instead of using the existingplaceUrl(kind, slug, phase)helper in lib/places.ts which already handles the per-kind prefixes. Foroutcodeplaces this links to a route that doesn't exist (404). Forauthorityplaces it links into the town/locality namespace (/schools/[place]/[phase], whose resolver only triesfetchPlace('town', ...)/fetchPlace('locality', ...)); per this file's own comment, ~67 town names collide with an authority name, so an authority's phase link can silently render a different place's (the colliding town's) schools instead of 404ing. This is reachable in practice becausebackend/places.pycomputesphase_urns(and thusPlace.publishes_phase) for authority and outcode places too, soget_place'sphasesfield is non-empty for those kinds and the nav renders on every authority/outcode page that clears the threshold — nothing scopes phase support to town/locality the way the outcode sitemap exclusion and the outcode page's own comment ("No phase variants... the variants would be pages without demand") say it should.🟡 Minor
townandoutcodesitemap cases (brentwood); nothing coversauthority, which is the kind actually broken (see above) since it's neither given phase variants intentionally nor excluded like outcode.places.find(p => p.kind === 'town'), so it can't catch the broken/mismatched links produced forauthorityoroutcodeplace kinds.