diff --git a/backend/localities.py b/backend/localities.py index ecd3077..0d6369e 100644 --- a/backend/localities.py +++ b/backend/localities.py @@ -20,6 +20,18 @@ A locality whose outcodes hold fewer than MIN_SCHOOLS schools is not published, so a typo produces no page rather than an empty one. Places that fail that check are logged at startup, because a locality you meant to publish quietly not appearing is the failure worth hearing about. + +Two rules for anything added here. + +**Sub-borough districts only.** A London borough is a local authority and +already has a page at /schools/authority/[la] covering all of its schools; a +locality defined by two or three outcodes would be a partial, near-duplicate +subset of it. Hackney, Islington, Greenwich and Ealing were all in the first +draft for that reason and have been removed. + +**The slug must not match a GIAS town.** "Richmond" did — GIAS has a Richmond +in North Yorkshire with 37 schools — so the London one could never publish. +The registry skips any locality that collides and logs it. """ # slug -> (display name, outcodes) @@ -30,16 +42,11 @@ LOCALITY_OUTCODES: dict[str, tuple[str, tuple[str, ...]]] = { "shoreditch": ("Shoreditch", ("EC2A", "E1")), "peckham": ("Peckham", ("SE15",)), "brixton": ("Brixton", ("SW2", "SW9")), - "hackney": ("Hackney", ("E5", "E8", "E9")), - "islington": ("Islington", ("N1", "N5", "N7")), "camden-town": ("Camden Town", ("NW1",)), - "greenwich": ("Greenwich", ("SE10",)), "wimbledon": ("Wimbledon", ("SW19",)), "putney": ("Putney", ("SW15",)), "fulham": ("Fulham", ("SW6",)), "chiswick": ("Chiswick", ("W4",)), - "ealing": ("Ealing", ("W5", "W13")), - "richmond": ("Richmond", ("TW9", "TW10")), "stratford": ("Stratford", ("E15",)), "walthamstow": ("Walthamstow", ("E17",)), "tooting": ("Tooting", ("SW17",)), diff --git a/backend/places.py b/backend/places.py index 12eb84a..5cab181 100644 --- a/backend/places.py +++ b/backend/places.py @@ -128,10 +128,19 @@ def _locality_places(df, publishable: set[int], out: dict[str, Place] = {} for slug, (name, outcodes) in LOCALITY_OUTCODES.items(): if slug in town_slugs: - raise ValueError( - f"locality {slug!r} collides with a published town of the same " - "slug; publishing both would shadow the town silently" - ) + # Skip, do not raise. The guard exists so a locality never + # silently shadows a town — skipping achieves that, and the error + # log makes it loud. + # + # Raising here took down sitemap generation for all 25,000 school + # pages when "richmond" met the GIAS town Richmond in North + # Yorkshire. Worse, GIAS town names change without any code change, + # so a raise means curated data can break the site spontaneously. + # A curation mistake must cost one page, not the sitemap. + logger.error( + "locality %r collides with the published town of the same " + "slug and has been skipped; rename it or remove it", slug) + continue group = working[working["_oc"].isin(outcodes)] urns = tuple(sorted({int(u) for u in group["urn"]} & publishable)) if len(urns) < MIN_SCHOOLS: diff --git a/backend/tests/test_places.py b/backend/tests/test_places.py index bcd7019..af14671 100644 --- a/backend/tests/test_places.py +++ b/backend/tests/test_places.py @@ -108,16 +108,42 @@ def test_locality_below_the_threshold_is_not_published(monkeypatch): assert "locality:nowhere" not in reg -def test_a_locality_may_not_shadow_a_viable_town(monkeypatch): - # Silently shadowing a town would lose a page carrying real demand. +def test_a_locality_may_not_shadow_a_viable_town(monkeypatch, caplog): + """A colliding locality is skipped loudly, and the town survives. + + This used to raise, which took down sitemap generation for all 25,000 + school pages the first time a curated slug met a real GIAS town. Curated + data must not be able to break the site — and GIAS town names change with + no code change at all, so the raise could fire spontaneously. + """ + import logging + from backend import localities monkeypatch.setattr(localities, "LOCALITY_OUTCODES", {"brentwood": ("Brentwood", ("CM13",))}) rows = _town(MIN_SCHOOLS, "Brentwood", "Essex") for r in rows: r["postcode"] = "CM13 1AA" - with pytest.raises(ValueError, match="brentwood"): - build_place_registry(_df(rows)) + + with caplog.at_level(logging.ERROR): + reg = build_place_registry(_df(rows)) + + assert "locality:brentwood" not in reg # skipped + assert "town:brentwood" in reg # the town is untouched + assert "brentwood" in caplog.text # and it was loud about it + + +def test_a_locality_collision_does_not_break_the_rest_of_the_registry(monkeypatch): + # The whole point of skipping rather than raising. + from backend import localities + monkeypatch.setattr(localities, "LOCALITY_OUTCODES", + {"brentwood": ("Brentwood", ("CM13",))}) + rows = _town(MIN_SCHOOLS, "Brentwood", "Essex") + for r in rows: + r["postcode"] = "CM13 1AA" + reg = build_place_registry(_df(rows)) + assert "authority:essex" in reg + assert "outcode:cm13" in reg def test_outcode_places_are_built_from_postcodes(): @@ -174,3 +200,32 @@ def test_the_pipeline_seed_mirrors_the_canonical_module(): for row in csv.DictReader(seed_path.open()) } assert seed == LOCALITY_OUTCODES + + +def test_no_curated_locality_names_a_london_borough(): + """Boroughs are authorities and already have a page. + + A locality defined by two or three outcodes inside a borough would be a + partial, near-duplicate subset of that authority page — the exact + thin-content failure the two-namespace design exists to avoid. Hackney, + Islington, Greenwich and Ealing were all in the first draft. + + Hardcoded rather than read from the corpus because this must fail in CI, + where there is no database. + """ + from backend.localities import LOCALITY_OUTCODES + + boroughs = { + "barking-and-dagenham", "barnet", "bexley", "brent", "bromley", + "camden", "croydon", "ealing", "enfield", "greenwich", "hackney", + "hammersmith-and-fulham", "haringey", "harrow", "havering", + "hillingdon", "hounslow", "islington", "kensington-and-chelsea", + "kingston-upon-thames", "lambeth", "lewisham", "merton", "newham", + "redbridge", "richmond-upon-thames", "southwark", "sutton", + "tower-hamlets", "waltham-forest", "wandsworth", "westminster", + } + named = boroughs & set(LOCALITY_OUTCODES) + assert not named, ( + f"these are boroughs, not districts: {sorted(named)} - they already " + "have an authority page covering every school" + ) diff --git a/pipeline/transform/seeds/locality_outcodes.csv b/pipeline/transform/seeds/locality_outcodes.csv index 968f6ce..11dfc8d 100644 --- a/pipeline/transform/seeds/locality_outcodes.csv +++ b/pipeline/transform/seeds/locality_outcodes.csv @@ -5,16 +5,11 @@ clapham,Clapham,SW4,London shoreditch,Shoreditch,EC2A|E1,London peckham,Peckham,SE15,London brixton,Brixton,SW2|SW9,London -hackney,Hackney,E5|E8|E9,London -islington,Islington,N1|N5|N7,London camden-town,Camden Town,NW1,London -greenwich,Greenwich,SE10,London wimbledon,Wimbledon,SW19,London putney,Putney,SW15,London fulham,Fulham,SW6,London chiswick,Chiswick,W4,London -ealing,Ealing,W5|W13,London -richmond,Richmond,TW9|TW10,London stratford,Stratford,E15,London walthamstow,Walthamstow,E17,London tooting,Tooting,SW17,London