diff --git a/backend/app.py b/backend/app.py index 23826c2..e46aff9 100644 --- a/backend/app.py +++ b/backend/app.py @@ -41,7 +41,7 @@ from .data_loader import get_data_info as get_db_info from . import flags from .places import build_place_index, build_place_registry, places_for_urn from .schemas import METRIC_DEFINITIONS, PHASE_GROUPS, RANKING_COLUMNS, SCHOOL_COLUMNS -from .similar_schools import is_secondary_phase, select_similar +from .similar_schools import select_similar from .utils import clean_for_json, convert_to_native # Values to exclude from filter dropdowns (empty strings, non-applicable labels) @@ -266,20 +266,18 @@ def _places_payload(urn: int) -> list[dict]: return payload -def _similar_schools_payload(urn: int, phase: str | None) -> list[dict]: - """Nearby schools this page may offer as alternatives. +def _similar_schools_payload(urn: int) -> list[dict]: + """The nearest eligible schools this page may offer, closest first. + + Phase and reach are read from the school's own row inside select_similar, + so nothing here can hand it a phase that disagrees with the data. Wrapped: a failure in selection must never 500 a page that is otherwise complete, which is the posture get_supplementary_data already takes. The section simply does not render. """ try: - # Decided in similar_schools, beside the PHASE_GROUPS bucket it selects - # from, so the two cannot drift. A substring test for "secondary" here - # would miss "16 plus" and hand a sixth-form college the primary bucket. - return select_similar( - load_latest_school_data(), int(urn), is_secondary_phase(phase) - ) + return select_similar(load_latest_school_data(), int(urn)) except Exception: import logging @@ -999,11 +997,10 @@ async def get_school_details(request: Request, urn: int): # and authority both fall below the publish threshold has nowhere to # point, and the page renders without the module. "places": _places_payload(urn), - # Nearby schools of the same phase and a comparable intake. Always - # present on a build with this code; the frontend treats absent and - # empty identically, which is what lets the two images deploy - # independently. - "similar_schools": _similar_schools_payload(urn, latest.get("phase")), + # The nearest eligible schools, closest first. Always present on a + # build with this code; the frontend treats absent and empty + # identically, which is what lets the two images deploy independently. + "similar_schools": _similar_schools_payload(urn), "yearly_data": clean_for_json(school_data), # Supplementary data (null if not yet populated by Kestra) "ofsted": supplementary.get("ofsted"), diff --git a/backend/similar_schools.py b/backend/similar_schools.py index 0e75467..08a5966 100644 --- a/backend/similar_schools.py +++ b/backend/similar_schools.py @@ -1,17 +1,24 @@ """Which nearby schools a detail page may offer as alternatives. -Two kinds of rule, and they are not interchangeable. +HARD FILTERS decide eligibility, and encode claims the section is not allowed +to make. A selective school is not an alternative to a non-selective one, a +special school is not comparable to a mainstream one, and a Girls school is not +an option for a Boys school's reader. They never relax, at any distance, even +where that means the section does not render at all. -HARD FILTERS encode claims the section is not allowed to make. A selective -school is not an alternative to a non-selective one, a special school is not -comparable to a mainstream one, and a Girls school is not an option for a Boys -school's reader. These never relax, at any distance, even where that means the -section does not render at all. +DISTANCE decides the order, and nothing else does. -SOFT PREFERENCES describe how closely an intake resembles this school's. They -relax in tiers, and every card reports the tier that actually took it so the -page can say what is shared rather than implying more. They relax only far -enough to reach a usable set, never far enough to fill the last of the slots. +An earlier version ranked by intake similarity first and used distance only as +a tiebreak. That put a Catholic school 2.9 miles away above the community +school 0.3 miles down the road, and — because the row filled from the best tier +before widening — filled all six slots with faith matches while omitting every +school a parent could actually walk to. For a primary, a school that far is not +a weaker option; it is not an option. Distance is a constraint and intake is a +preference, and the ranking now says so. + +Similarity survives as `shared`: what a candidate genuinely has in common with +this school, reported on its card, so a reader applies their own weighting +instead of having ours applied for them. Pure functions over a DataFrame: no I/O, no FastAPI, no database. """ @@ -25,19 +32,21 @@ import pandas as pd from .schemas import PHASE_GROUPS -# A cap, not a quota: the section shows everything that qualified at the tiers -# it used, up to this many. Three fit the row; the rest are behind the arrows. +# Three fit the row; the rest are behind the carousel arrows. MAX_SCHOOLS = 6 -# Tiers stop relaxing once this many have been found. Without it, a cap of six -# would reliably drag in tier-3 schools ten miles away to fill a row that three -# good matches had already earned. -ENOUGH = 3 MINIMUM = 2 -# (tier, radius in miles). Faith relaxes before gender: a faith mismatch -# changes the character of a school, while a gender mismatch can mean the -# school is not available to this reader's child at all. -TIERS: tuple[tuple[int, float], ...] = ((1, 3.0), (2, 5.0), (3, 10.0)) +# How far the section will reach, in miles, when nothing closer exists. +# +# A sanity bound rather than a target: ordering by distance already handles +# density, so a school in a dense area fills all six slots inside a mile and +# never sees this. It decides one thing — what happens where the area is +# sparse — and the answer differs by phase because catchments do. Primary +# catchments are routinely under a mile; beyond two, a primary is not a weaker +# option but not an option, and no section is the honest answer. +PRIMARY_RADIUS_MILES = 2.0 +SECONDARY_RADIUS_MILES = 6.0 +POST16_RADIUS_MILES = 10.0 EARTH_RADIUS_MILES = 3958.8 @@ -80,15 +89,6 @@ def genders_compatible(a: str | None, b: str | None) -> bool: return not (left in single and right in single and left != right) -def phase_label(phase: str | None) -> str: - text = (phase or "").strip() - if not text: - return "School" - if text.lower() == "all-through": - return "All-through school" - return f"{text.capitalize()} school" - - def is_secondary_phase(phase: str | None) -> bool: """Whether this phase takes the secondary side: secondary group membership, minus all-through. @@ -107,6 +107,13 @@ def is_secondary_phase(phase: str | None) -> bool: return text != "all-through" and text in PHASE_GROUPS["secondary"] +def radius_miles(phase: str | None) -> float: + """How far this phase's section will reach when nothing closer exists.""" + if (phase or "").strip().lower() == "16 plus": + return POST16_RADIUS_MILES + return SECONDARY_RADIUS_MILES if is_secondary_phase(phase) else PRIMARY_RADIUS_MILES + + def _phase_group(is_secondary: bool) -> set[str]: return PHASE_GROUPS["secondary" if is_secondary else "primary"] @@ -144,26 +151,45 @@ def _mask(series: pd.Series, predicate) -> pd.Series: return pd.Series([predicate(value) for value in series], index=series.index, dtype=bool) -def _chips(subject: pd.Series, candidate: pd.Series, tier: int, is_secondary: bool) -> list[str]: - if tier >= 3: - return [phase_label(candidate.get("phase"))] +def _shared(subject: pd.Series, candidate: pd.Series, is_secondary: bool) -> list[str]: + """What this candidate genuinely has in common with the subject. + + Empty is a real answer, and renders no chips at all. A card claiming a + shared characteristic it does not have would be worse than a bare one — + and since these no longer affect the order, an empty list costs the school + nothing but its place in the row, which distance already decided. + """ + shared: list[str] = [] + + gender = str(subject.get("gender") or "").strip() + if gender and str(candidate.get("gender") or "").strip().lower() == gender.lower(): + shared.append(gender) - chips = [str(subject.get("gender") or "").strip()] if is_secondary: - policy = (candidate.get("admissions_policy") or "").strip() - if policy and policy.lower() not in {"not applicable", "unknown"}: - chips.append(policy) - if tier == 1: - chips.append(faith_label(candidate.get("religious_denomination"))) - return [chip for chip in chips if chip] + policy = str(candidate.get("admissions_policy") or "").strip() + subject_policy = str(subject.get("admissions_policy") or "").strip() + if ( + policy + and policy.lower() == subject_policy.lower() + and policy.lower() not in {"not applicable", "unknown"} + ): + shared.append(policy) + + if faith_key(candidate.get("religious_denomination")) == faith_key( + subject.get("religious_denomination") + ): + shared.append(faith_label(candidate.get("religious_denomination"))) + + return shared -def select_similar(frame: pd.DataFrame, urn: int, is_secondary: bool) -> list[dict]: - """Up to MAX_SCHOOLS nearby schools this page may offer, or [] below MINIMUM. +def select_similar(frame: pd.DataFrame, urn: int) -> list[dict]: + """The nearest eligible schools, closest first — at most MAX_SCHOOLS, and + none at all below MINIMUM. - Selected by tier, displayed by distance: the tier decides which schools - earn a slot, and the render order is then closest-first, because "nearby" - is the promise in the heading. + The phase is read from the subject's own row rather than passed in, so a + caller cannot hand this a phase that disagrees with the data it selects + from. """ subject_rows = frame[frame["urn"] == urn] if subject_rows.empty: @@ -174,6 +200,9 @@ def select_similar(frame: pd.DataFrame, urn: int, is_secondary: bool) -> list[di if lat is None or lon is None: return [] + phase = subject.get("phase") + is_secondary = is_secondary_phase(phase) + reach = radius_miles(phase) metric_key = "attainment_8_score" if is_secondary else "rwm_expected_pct" candidates = frame[frame["urn"] != urn].copy() @@ -208,42 +237,12 @@ def select_similar(frame: pd.DataFrame, urn: int, is_secondary: bool) -> list[di lat, lon, candidates["latitude"].values, candidates["longitude"].values ).round(1) - # ── Soft preferences, in tiers ────────────────────────────────────── - subject_faith = faith_key(subject.get("religious_denomination")) - subject_gender_key = (subject_gender or "").strip().lower() - same_gender = candidates["gender"].fillna("").str.strip().str.lower() == subject_gender_key - same_faith = _mask( - candidates["religious_denomination"], lambda d: faith_key(d) == subject_faith - ) - - tier_masks = { - 1: same_gender & same_faith, - 2: same_gender, - 3: pd.Series(True, index=candidates.index), - } - - # Descend the tiers only until the set reaches ENOUGH. The tier that gets - # there is the last one opened, and the remaining slots up to MAX_SCHOOLS - # are filled from the tiers already used — never by widening again. - picked: dict[int, tuple[int, pd.Series]] = {} - for tier, radius in TIERS: - within = candidates[tier_masks[tier] & (candidates["distance_miles"] <= radius)] - for _, row in within.sort_values("distance_miles").iterrows(): - candidate_urn = int(row["urn"]) - if candidate_urn in picked: - continue - picked[candidate_urn] = (tier, row) - if len(picked) >= MAX_SCHOOLS: - break - if len(picked) >= ENOUGH: - break - - if len(picked) < MINIMUM: + # ── Nearest first, and nothing else has a say ─────────────────────── + within = candidates[candidates["distance_miles"] <= reach] + if len(within) < MINIMUM: return [] - selected = sorted( - picked.values(), key=lambda pair: float(pair[1]["distance_miles"]) - )[:MAX_SCHOOLS] + selected = within.sort_values(["distance_miles", "urn"]).head(MAX_SCHOOLS) return [ { "urn": int(row["urn"]), @@ -251,11 +250,10 @@ def select_similar(frame: pd.DataFrame, urn: int, is_secondary: bool) -> list[di "distance_miles": float(row["distance_miles"]), "school_type": _native(row.get("school_type")), "age_range": _native(row.get("age_range")), - "shared": _chips(subject, row, tier, is_secondary), - "tier": tier, + "shared": _shared(subject, row, is_secondary), "metric_value": _native(row.get(metric_key)), "metric_key": metric_key, "metric_year": _native(row.get("year")), } - for tier, row in selected + for _, row in selected.iterrows() ] diff --git a/backend/tests/test_similar_schools.py b/backend/tests/test_similar_schools.py index ad4aca7..874ed4b 100644 --- a/backend/tests/test_similar_schools.py +++ b/backend/tests/test_similar_schools.py @@ -1,19 +1,26 @@ -"""Selection rules for the "similar schools nearby" section. +"""Selection rules for the nearby-schools section. -The hard filters encode claims the section is not allowed to make — that a +Hard filters encode claims the section is not allowed to make — that a selective school is an alternative to a non-selective one, that a special school is comparable to a mainstream one, or that a Girls school is an option -for a Boys school's reader. They never relax. The soft preferences describe -how close the intake is, and they do — but only far enough to reach a usable -set, never far enough to fill the last of the six slots. +for a Boys school's reader. They decide who is eligible. + +Distance decides the order, and nothing else does. An earlier version ranked by +intake similarity first, which put a Catholic school 2.9 miles away above the +community school 0.3 miles down the road — for a primary, a school that far is +not a weaker option, it is not an option. Similarity is now reported on the +card and never reorders the row. """ import numpy as np import pandas as pd -from backend.similar_schools import is_secondary_phase, select_similar +from backend.similar_schools import ( + is_secondary_phase, + radius_miles, + select_similar, +) -# Roughly 0.7 miles apart in latitude at this longitude. BASE_LAT, BASE_LON = 51.5000, -0.1000 @@ -48,16 +55,68 @@ def _at(miles): return BASE_LAT + miles / 69.0 -def test_returns_nearest_same_phase_schools(): +# --------------------------------------------------------------------------- +# Order: distance, and only distance +# --------------------------------------------------------------------------- + +def test_returns_nearest_first(): frame = _frame( _row(100001, "Subject"), - _row(100002, "Near", latitude=_at(0.5)), - _row(100003, "Mid", latitude=_at(1.0)), - _row(100004, "Far", latitude=_at(2.0)), + _row(100002, "Mid", latitude=_at(1.0)), + _row(100003, "Near", latitude=_at(0.4)), + _row(100004, "Far", latitude=_at(1.8)), ) - result = select_similar(frame, 100001, is_secondary=False) - assert [s["urn"] for s in result] == [100002, 100003, 100004] - assert result[0]["distance_miles"] == 0.5 + result = select_similar(frame, 100001) + assert [s["urn"] for s in result] == [100003, 100002, 100004] + assert result[0]["distance_miles"] == 0.4 + + +def test_a_faith_match_never_outranks_a_closer_school(): + """The reported defect. A Catholic primary surrounded by Catholic primaries + showed six of them and omitted the community school down the road.""" + frame = _frame( + _row(100001, "St Jude's RC Primary", religious_denomination="Roman Catholic"), + _row(100002, "Elm Grove Primary", religious_denomination="None", latitude=_at(0.3)), + _row(100003, "Holy Cross RC", religious_denomination="Roman Catholic", latitude=_at(0.8)), + _row(100004, "Sacred Heart RC", religious_denomination="Roman Catholic", latitude=_at(1.2)), + _row(100005, "St Peter's RC", religious_denomination="Roman Catholic", latitude=_at(1.6)), + ) + result = select_similar(frame, 100001) + assert result[0]["urn"] == 100002, "the nearest school leads, whatever its intake" + assert [s["distance_miles"] for s in result] == sorted(s["distance_miles"] for s in result) + + +def test_the_nearest_eligible_school_is_always_shown(): + """Whatever else changes, a section titled "nearby" cannot omit the nearest + school while listing one four times further away.""" + frame = _frame( + _row(100001, "Subject", gender="Boys", religious_denomination="Roman Catholic"), + _row(100002, "Nearest", gender="Mixed", religious_denomination="None", latitude=_at(0.2)), + *[ + _row(100010 + n, f"Match {n}", gender="Boys", + religious_denomination="Roman Catholic", latitude=_at(0.9 + n * 0.1)) + for n in range(6) + ], + ) + assert select_similar(frame, 100001)[0]["urn"] == 100002 + + +def test_caps_at_six_taking_the_nearest(): + frame = _frame( + _row(100001, "Subject"), + *[_row(100010 + n, f"Peer {n}", latitude=_at(0.1 * (n + 1))) for n in range(7)], + ) + result = select_similar(frame, 100001) + assert len(result) == 6 + assert 100016 not in {s["urn"] for s in result}, "the seventh-nearest is the one dropped" + + +def test_fewer_than_two_matches_returns_empty(): + frame = _frame( + _row(100001, "Subject"), + _row(100002, "Only neighbour", latitude=_at(0.5)), + ) + assert select_similar(frame, 100001) == [] def test_excludes_the_subject_school(): @@ -66,19 +125,66 @@ def test_excludes_the_subject_school(): _row(100002, "A", latitude=_at(0.5)), _row(100003, "B", latitude=_at(0.6)), ) - assert 100001 not in {s["urn"] for s in select_similar(frame, 100001, is_secondary=False)} + assert 100001 not in {s["urn"] for s in select_similar(frame, 100001)} +def test_a_school_is_never_listed_twice(): + frame = _frame( + _row(100001, "Subject"), + _row(100002, "A", latitude=_at(0.5)), + _row(100003, "B", latitude=_at(0.6)), + ) + result = select_similar(frame, 100001) + assert len(result) == len({s["urn"] for s in result}) + + +# --------------------------------------------------------------------------- +# Reach: a sanity bound, not a target +# --------------------------------------------------------------------------- + +def test_primary_does_not_reach_past_two_miles(): + frame = _frame( + _row(100001, "Subject"), + _row(100002, "Just inside", latitude=_at(1.9)), + _row(100003, "Just outside", latitude=_at(2.4)), + _row(100004, "Miles away", latitude=_at(4.0)), + ) + # One inside the cap is below the minimum, so nothing renders at all — + # a primary with nothing within two miles has no nearby schools. + assert select_similar(frame, 100001) == [] + + +def test_secondary_reaches_further_than_primary(): + frame = _frame( + _row(100001, "Subject", phase="Secondary"), + _row(100002, "A", phase="Secondary", latitude=_at(3.0)), + _row(100003, "B", phase="Secondary", latitude=_at(5.5)), + ) + assert {s["urn"] for s in select_similar(frame, 100001)} == {100002, 100003} + + +def test_the_cap_follows_the_phase(): + assert radius_miles("Primary") == 2.0 + assert radius_miles("Middle deemed primary") == 2.0 + assert radius_miles("All-through") == 2.0 + assert radius_miles("Secondary") == 6.0 + assert radius_miles("Middle deemed secondary") == 6.0 + # Post-16 is the phase people travel furthest for. + assert radius_miles("16 plus") == 10.0 + + +# --------------------------------------------------------------------------- +# Hard filters: eligibility, never order +# --------------------------------------------------------------------------- + def test_selective_never_meets_non_selective(): frame = _frame( _row(100001, "Grammar", phase="Secondary", admissions_policy="Selective"), _row(100002, "Comp A", phase="Secondary", admissions_policy="Non-selective", latitude=_at(0.5)), _row(100003, "Comp B", phase="Secondary", admissions_policy="Non-selective", latitude=_at(0.6)), ) - assert select_similar(frame, 100001, is_secondary=True) == [] - - reverse = select_similar(frame, 100002, is_secondary=True) - assert 100001 not in {s["urn"] for s in reverse} + assert select_similar(frame, 100001) == [] + assert 100001 not in {s["urn"] for s in select_similar(frame, 100002)} def test_special_schools_match_only_each_other(): @@ -87,8 +193,8 @@ def test_special_schools_match_only_each_other(): _row(100002, "Mainstream A", latitude=_at(0.5)), _row(100003, "Mainstream B", latitude=_at(0.6)), ) - assert select_similar(frame, 100001, is_secondary=False) == [] - assert select_similar(frame, 100002, is_secondary=False) == [] + assert select_similar(frame, 100001) == [] + assert select_similar(frame, 100002) == [] def test_boys_never_meets_girls(): @@ -98,7 +204,7 @@ def test_boys_never_meets_girls(): _row(100003, "Mixed School", gender="Mixed", latitude=_at(0.6)), _row(100004, "Another Mixed", gender="Mixed", latitude=_at(0.7)), ) - urns = {s["urn"] for s in select_similar(frame, 100001, is_secondary=False)} + urns = {s["urn"] for s in select_similar(frame, 100001)} assert 100002 not in urns assert urns == {100003, 100004} @@ -111,80 +217,7 @@ def test_closed_schools_and_missing_coordinates_are_dropped(): _row(100004, "Good A", latitude=_at(0.6)), _row(100005, "Good B", latitude=_at(0.7)), ) - assert {s["urn"] for s in select_similar(frame, 100001, is_secondary=False)} == {100004, 100005} - - -def test_tiers_relax_faith_before_gender(): - frame = _frame( - _row(100001, "Subject", gender="Boys", religious_denomination="Roman Catholic"), - # Tier 1: same gender and same faith. - _row(100002, "Tier one", gender="Boys", religious_denomination="Roman Catholic", latitude=_at(2.0)), - # Tier 2: same gender, different faith — closer, but a weaker match. - _row(100003, "Tier two", gender="Boys", religious_denomination="None", latitude=_at(0.5)), - # Tier 3: mixed gender, different faith. - _row(100004, "Tier three", gender="Mixed", religious_denomination="None", latitude=_at(0.6)), - ) - result = select_similar(frame, 100001, is_secondary=False) - tier_by_urn = {s["urn"]: s["tier"] for s in result} - assert tier_by_urn == {100002: 1, 100003: 2, 100004: 3} - # Selected by tier, displayed by distance. - assert [s["urn"] for s in result] == [100003, 100004, 100002] - - -def test_caps_at_six_taking_the_nearest(): - frame = _frame( - _row(100001, "Subject"), - *[_row(100010 + n, f"Peer {n}", latitude=_at(0.1 * (n + 1))) for n in range(7)], - ) - result = select_similar(frame, 100001, is_secondary=False) - assert len(result) == 6 - # The seventh-nearest is the one dropped, not an arbitrary one. - assert 100016 not in {s["urn"] for s in result} - - -def test_tiers_stop_once_enough_are_found(): - """Four tier-1 matches are a usable set, so tier 2 is never opened — even - though it holds a school that is closer than any of them.""" - frame = _frame( - _row(100001, "Subject", religious_denomination="Roman Catholic"), - _row(100002, "RC one", religious_denomination="Roman Catholic", latitude=_at(0.5)), - _row(100003, "RC two", religious_denomination="Roman Catholic", latitude=_at(0.6)), - _row(100004, "RC three", religious_denomination="Roman Catholic", latitude=_at(0.7)), - _row(100005, "RC four", religious_denomination="Roman Catholic", latitude=_at(0.8)), - # Closer than every one of them, but only a tier-2 match. - _row(100006, "Secular and nearer", religious_denomination="None", latitude=_at(0.2)), - ) - result = select_similar(frame, 100001, is_secondary=False) - assert 100006 not in {s["urn"] for s in result} - assert len(result) == 4 - assert all(s["tier"] == 1 for s in result) - - -def test_a_school_is_never_taken_twice(): - frame = _frame( - _row(100001, "Subject"), - _row(100002, "A", latitude=_at(0.5)), - _row(100003, "B", latitude=_at(0.6)), - ) - result = select_similar(frame, 100001, is_secondary=False) - assert len(result) == len({s["urn"] for s in result}) - - -def test_fewer_than_two_matches_returns_empty(): - frame = _frame( - _row(100001, "Subject"), - _row(100002, "Only neighbour", latitude=_at(0.5)), - ) - assert select_similar(frame, 100001, is_secondary=False) == [] - - -def test_beyond_the_widest_radius_is_not_offered(): - frame = _frame( - _row(100001, "Subject"), - _row(100002, "A", latitude=_at(11.0)), - _row(100003, "B", latitude=_at(12.0)), - ) - assert select_similar(frame, 100001, is_secondary=False) == [] + assert {s["urn"] for s in select_similar(frame, 100001)} == {100004, 100005} def test_all_through_is_offered_on_both_phase_sides(): @@ -193,14 +226,14 @@ def test_all_through_is_offered_on_both_phase_sides(): _row(100002, "All through", phase="All-through", latitude=_at(0.5)), _row(100003, "Primary peer", phase="Primary", latitude=_at(0.6)), ) - assert 100002 in {s["urn"] for s in select_similar(frame, 100001, is_secondary=False)} + assert 100002 in {s["urn"] for s in select_similar(frame, 100001)} secondary = _frame( _row(100010, "Secondary subject", phase="Secondary"), _row(100002, "All through", phase="All-through", latitude=_at(0.5)), _row(100011, "Secondary peer", phase="Secondary", latitude=_at(0.6)), ) - assert 100002 in {s["urn"] for s in select_similar(secondary, 100010, is_secondary=True)} + assert 100002 in {s["urn"] for s in select_similar(secondary, 100010)} def test_sixteen_plus_is_matched_against_secondary_not_primary(): @@ -212,11 +245,10 @@ def test_sixteen_plus_is_matched_against_secondary_not_primary(): _row(100001, "Sixth Form College", phase="16 plus", age_range="16-19"), _row(100002, "Nearby Secondary", phase="Secondary", latitude=_at(0.5), attainment_8_score=52.0), - _row(100003, "Nearby College", phase="16 plus", latitude=_at(0.6), - attainment_8_score=np.nan), + _row(100003, "Nearby College", phase="16 plus", latitude=_at(0.6)), _row(100004, "Nearby Primary", phase="Primary", latitude=_at(0.1)), ) - result = select_similar(frame, 100001, is_secondary=is_secondary_phase("16 plus")) + result = select_similar(frame, 100001) urns = {s["urn"] for s in result} assert 100004 not in urns, "a primary school is not a peer for a sixth form" assert urns == {100002, 100003} @@ -224,18 +256,20 @@ def test_sixteen_plus_is_matched_against_secondary_not_primary(): def test_is_secondary_phase_agrees_with_the_phase_groups_it_selects_from(): - """The two must not drift: whatever this calls secondary decides which - PHASE_GROUPS bucket the candidates come from.""" for phase in ("Secondary", "Middle deemed secondary", "16 plus"): assert is_secondary_phase(phase) is True, phase for phase in ("Primary", "Middle deemed primary", "Nursery", "", None): assert is_secondary_phase(phase) is False, phase # In PHASE_GROUPS an all-through school is on both sides, but it renders - # with the primary template, and the metric follows the template. + # with the primary template, and the metric follows the phase side. assert is_secondary_phase("All-through") is False -def test_chips_state_only_what_the_tier_earned(): +# --------------------------------------------------------------------------- +# What the card reports +# --------------------------------------------------------------------------- + +def test_shared_lists_only_what_is_actually_shared(): frame = _frame( _row(100001, "Subject", phase="Secondary", gender="Mixed", religious_denomination="None", admissions_policy="Non-selective"), @@ -244,28 +278,47 @@ def test_chips_state_only_what_the_tier_earned(): _row(100003, "Faith differs", phase="Secondary", gender="Mixed", religious_denomination="Church of England", admissions_policy="Non-selective", latitude=_at(0.6)), ) - by_urn = {s["urn"]: s for s in select_similar(frame, 100001, is_secondary=True)} + by_urn = {s["urn"]: s for s in select_similar(frame, 100001)} assert by_urn[100002]["shared"] == ["Mixed", "Non-selective", "No religious character"] assert by_urn[100003]["shared"] == ["Mixed", "Non-selective"] -def test_tier_three_chip_is_the_plain_phase(): +def test_a_shared_faith_is_named(): frame = _frame( - _row(100001, "Subject", gender="Boys"), - _row(100002, "A", gender="Mixed", latitude=_at(0.5)), - _row(100003, "B", gender="Mixed", latitude=_at(0.6)), + _row(100001, "Subject", religious_denomination="Roman Catholic"), + _row(100002, "Also RC", religious_denomination="Roman Catholic", latitude=_at(0.4)), + _row(100003, "Secular", religious_denomination="None", latitude=_at(0.5)), ) - result = select_similar(frame, 100001, is_secondary=False) - assert all(s["shared"] == ["Primary school"] for s in result) + by_urn = {s["urn"]: s for s in select_similar(frame, 100001)} + assert "Roman Catholic" in by_urn[100002]["shared"] + assert by_urn[100003]["shared"] == ["Mixed"] -def test_metric_follows_the_template_not_the_neighbour(): +def test_shared_is_empty_when_nothing_is_shared(): + frame = _frame( + _row(100001, "Subject", gender="Boys", religious_denomination="Roman Catholic"), + _row(100002, "A", gender="Mixed", religious_denomination="None", latitude=_at(0.4)), + _row(100003, "B", gender="Mixed", religious_denomination="Church of England", latitude=_at(0.5)), + ) + assert all(s["shared"] == [] for s in select_similar(frame, 100001)) + + +def test_no_tier_is_reported_because_there_are_no_tiers(): + frame = _frame( + _row(100001, "Subject"), + _row(100002, "A", latitude=_at(0.4)), + _row(100003, "B", latitude=_at(0.5)), + ) + assert all("tier" not in s for s in select_similar(frame, 100001)) + + +def test_metric_follows_the_phase_side_not_the_neighbour(): frame = _frame( _row(100001, "Subject", phase="Secondary", attainment_8_score=50.0), _row(100002, "A", phase="Secondary", attainment_8_score=52.8, latitude=_at(0.5)), _row(100003, "B", phase="Secondary", attainment_8_score=np.nan, latitude=_at(0.6)), ) - by_urn = {s["urn"]: s for s in select_similar(frame, 100001, is_secondary=True)} + by_urn = {s["urn"]: s for s in select_similar(frame, 100001)} assert by_urn[100002]["metric_key"] == "attainment_8_score" assert by_urn[100002]["metric_value"] == 52.8 assert by_urn[100002]["metric_year"] == 202425 @@ -278,7 +331,7 @@ def test_values_are_json_safe_native_types(): _row(100002, "A", latitude=_at(0.5)), _row(100003, "B", latitude=_at(0.6)), ) - for school in select_similar(frame, 100001, is_secondary=False): + for school in select_similar(frame, 100001): assert isinstance(school["urn"], int) assert isinstance(school["distance_miles"], float) assert not isinstance(school["metric_value"], np.generic) @@ -310,7 +363,7 @@ def client(monkeypatch): return TestClient(app_module.app, raise_server_exceptions=False) -def test_detail_payload_carries_similar_schools(client): +def test_detail_payload_carries_nearby_schools(client): resp = client.get("/api/schools/100001") assert resp.status_code == 200, resp.text similar = resp.json()["similar_schools"] diff --git a/e2e/tests/journeys.spec.ts b/e2e/tests/journeys.spec.ts index 23ac1bb..4de2c6f 100644 --- a/e2e/tests/journeys.spec.ts +++ b/e2e/tests/journeys.spec.ts @@ -2736,7 +2736,7 @@ test('the content sitemap lists the about page and is advertised in robots', asy }); /** - * Similar schools nearby. + * Other schools nearby. * * The section is absent by design where fewer than two schools qualify, and the * arrows are absent where three cards fit, so this asserts each part of the @@ -2746,12 +2746,12 @@ test('the content sitemap lists the about page and is advertised in robots', asy * which jsdom cannot measure because it has no layout, and the scroll position * surviving a selection, which is DOM state rather than React state. */ -test('similar schools link on to other schools and into compare', async ({ page }) => { +test('nearby schools link on to other schools and into compare', async ({ page }) => { await searchByName(page, 'Primary'); await schoolLinks(page).first().click(); await page.waitForURL(/\/school\//); - const section = page.locator('#similar'); + const section = page.locator('#nearby'); if ((await section.count()) === 0) { test.skip(true, 'No qualifying similar schools for this school'); } @@ -2793,7 +2793,7 @@ test('similar schools link on to other schools and into compare', async ({ page }); /** - * The section at MOBILE.md's three reference widths. + * The nearby-schools section at MOBILE.md's three reference widths. * * MOBILE.md asks for exactly this check and records that it was not written * because "Playwright isn't currently in the project dependency set". That is @@ -2801,13 +2801,13 @@ test('similar schools link on to other schools and into compare', async ({ page * to the page this feature touches. */ for (const width of [360, 390, 430]) { - test(`similar schools survives a ${width}px viewport`, async ({ page }) => { + test(`nearby schools survives a ${width}px viewport`, async ({ page }) => { await page.setViewportSize({ width, height: 800 }); await searchByName(page, 'Primary'); await schoolLinks(page).first().click(); await page.waitForURL(/\/school\//); - const section = page.locator('#similar'); + const section = page.locator('#nearby'); if ((await section.count()) === 0) { test.skip(true, 'No qualifying similar schools for this school'); } diff --git a/nextjs-app/__tests__/components/SimilarSchoolsSection.test.tsx b/nextjs-app/__tests__/components/SimilarSchoolsSection.test.tsx index 646ad89..3e381b3 100644 --- a/nextjs-app/__tests__/components/SimilarSchoolsSection.test.tsx +++ b/nextjs-app/__tests__/components/SimilarSchoolsSection.test.tsx @@ -1,8 +1,8 @@ /** - * The section's job is to be honest about what it matched. These tests pin the - * ways it could lie: rendering below the minimum, claiming a similar intake at - * tier 3, showing a missing figure as a number, or hiding a card behind an - * arrow where a crawler cannot reach it. + * The section's job is to be honest about what it is showing. These tests pin + * the ways it could mislead: rendering below the minimum, claiming a likeness + * it does not rank on, showing a missing figure as a number, or hiding a card + * behind an arrow where a crawler cannot reach it. */ import { render, screen } from '@testing-library/react'; @@ -34,7 +34,6 @@ function school(overrides: Partial = {}): SimilarSchool { school_type: 'Community school', age_range: '4-11', shared: ['Mixed', 'No religious character'], - tier: 1, metric_value: 74, metric_key: 'rwm_expected_pct', metric_year: 202425, @@ -74,15 +73,36 @@ describe('render gates', () => { }); }); -describe('the claim the lede makes', () => { - it('claims a similar intake when every card is tier 1 or 2', () => { - renderSection([school({ tier: 1 }), school({ urn: 100003, tier: 2 })]); - expect(screen.getByText(/with a similar intake/i)).toBeInTheDocument(); +describe('what the section claims', () => { + it('never claims a similar intake, because it does not rank on one', () => { + renderSection([school(), school({ urn: 100003, shared: [] })]); + expect(screen.queryByText(/similar intake/i)).not.toBeInTheDocument(); }); - it('drops the claim when any card is tier 3', () => { - renderSection([school({ tier: 1 }), school({ urn: 100003, tier: 3, shared: ['Primary school'] })]); - expect(screen.queryByText(/with a similar intake/i)).not.toBeInTheDocument(); + it('is headed "Other schools nearby", not "similar"', () => { + renderSection([school(), school({ urn: 100003 })]); + expect(screen.getByRole('heading', { name: 'Other schools nearby' })).toBeInTheDocument(); + }); + + it('shows chips for what is shared', () => { + renderSection([school({ shared: ['Mixed', 'Roman Catholic'] }), school({ urn: 100003 })]); + expect(screen.getAllByText('Roman Catholic').length).toBe(1); + }); + + it('shows no chips at all when nothing is shared, rather than inventing one', () => { + const { container } = render( + , + ); + // The card still carries its distance, name, type and figure — just no + // claim of likeness. + expect(container.querySelectorAll('li ul').length).toBe(0); + expect(screen.getAllByText(/miles away/).length).toBe(2); }); }); diff --git a/nextjs-app/__tests__/lib/schoolSections.test.ts b/nextjs-app/__tests__/lib/schoolSections.test.ts index fdd807a..6bff0d3 100644 --- a/nextjs-app/__tests__/lib/schoolSections.test.ts +++ b/nextjs-app/__tests__/lib/schoolSections.test.ts @@ -173,7 +173,7 @@ describe('buildSecondaryNavItems', () => { }); }); -describe('the similar-schools nav item', () => { +describe('the nearby-schools nav item', () => { const navInput = { ofsted: null, admissions: null, admissionDistance: null, hasLocation: true, yearlyDataLength: 1, @@ -182,29 +182,29 @@ describe('the similar-schools nav item', () => { it('appears on both templates when the section renders', () => { const primary = computeSchoolFlags(primaryFixture); const secondary = computeSecondaryFlags(secondaryFixture); - const input = { ...navInput, hasSimilarSchools: true }; + const input = { ...navInput, hasNearbySchools: true }; - expect(buildNavItems(primary, input).map((i) => i.id)).toContain('similar'); - expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).toContain('similar'); + expect(buildNavItems(primary, input).map((i) => i.id)).toContain('nearby'); + expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).toContain('nearby'); }); it('is absent when the section does not render', () => { const primary = computeSchoolFlags(primaryFixture); const secondary = computeSecondaryFlags(secondaryFixture); - const input = { ...navInput, hasSimilarSchools: false }; + const input = { ...navInput, hasNearbySchools: false }; - expect(buildNavItems(primary, input).map((i) => i.id)).not.toContain('similar'); - expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).not.toContain('similar'); + expect(buildNavItems(primary, input).map((i) => i.id)).not.toContain('nearby'); + expect(buildSecondaryNavItems(secondary, input).map((i) => i.id)).not.toContain('nearby'); }); it('is absent when nothing says either way', () => { const primary = computeSchoolFlags(primaryFixture); - expect(buildNavItems(primary, navInput).map((i) => i.id)).not.toContain('similar'); + expect(buildNavItems(primary, navInput).map((i) => i.id)).not.toContain('nearby'); }); it('comes last, because the section renders last', () => { const primary = computeSchoolFlags(primaryFixture); - const ids = buildNavItems(primary, { ...navInput, hasSimilarSchools: true }).map((i) => i.id); - expect(ids[ids.length - 1]).toBe('similar'); + const ids = buildNavItems(primary, { ...navInput, hasNearbySchools: true }).map((i) => i.id); + expect(ids[ids.length - 1]).toBe('nearby'); }); }); diff --git a/nextjs-app/app/(frontend)/school/[slug]/page.tsx b/nextjs-app/app/(frontend)/school/[slug]/page.tsx index 23b98e8..1a2f040 100644 --- a/nextjs-app/app/(frontend)/school/[slug]/page.tsx +++ b/nextjs-app/app/(frontend)/school/[slug]/page.tsx @@ -189,7 +189,7 @@ export default async function SchoolPage({ params }: SchoolPageProps) { admissions: admissions ?? null, admissionDistance: admission_distance ?? null, hasLocation: school_info.latitude != null && school_info.longitude != null, - hasSimilarSchools: shouldRenderSimilar(similarSchools), + hasNearbySchools: shouldRenderSimilar(similarSchools), yearlyDataLength: yearly_data.length, }; const primaryNavItems = buildNavItems(primaryFlags, navInput); diff --git a/nextjs-app/components/school/SimilarSchools.module.css b/nextjs-app/components/school/SimilarSchools.module.css index 2427b7a..82bad54 100644 --- a/nextjs-app/components/school/SimilarSchools.module.css +++ b/nextjs-app/components/school/SimilarSchools.module.css @@ -39,7 +39,6 @@ .shared { display: flex; flex-wrap: wrap; gap: 0.35rem; list-style: none; margin: 0 0 0.85rem; padding: 0; } .chip { font-size: 0.72rem; line-height: 1.4; padding: 0.25rem 0.5rem; border-radius: 999px; background: var(--brand-bg); color: var(--brand); border: 1px solid transparent; } -.chipLoose { font-size: 0.72rem; line-height: 1.4; padding: 0.25rem 0.5rem; border-radius: 999px; background: transparent; color: var(--text-muted); border: 1px solid var(--border); } .metric { margin-top: auto; padding-top: 0.8rem; border-top: 1px solid var(--border); } /* No valence colour here, deliberately: green and terracotta mean "against the diff --git a/nextjs-app/components/school/SimilarSchoolsSection.tsx b/nextjs-app/components/school/SimilarSchoolsSection.tsx index 76db7e1..ec0334e 100644 --- a/nextjs-app/components/school/SimilarSchoolsSection.tsx +++ b/nextjs-app/components/school/SimilarSchoolsSection.tsx @@ -1,12 +1,16 @@ /** - * SimilarSchoolsSection — nearby schools of the same phase and a comparable - * intake. Server component; only the carousel, the compare bar and the - * add-to-compare button are client-side. + * NearbySchoolsSection — the nearest eligible schools, closest first. Server + * component; only the carousel, the compare bar and the add-to-compare button + * are client-side. * - * The section is allowed to say exactly what the backend matched and no more. - * The lede only claims a similar intake when no card came from tier 3, and a - * card's chips list what that school actually shares rather than a match it - * did not earn. + * "Other schools nearby", not "similar" ones: the order is distance and only + * distance. The hard filters upstream still guarantee the set is comparable — + * same phase, same selectivity, mainstream never beside special — but nothing + * here ranks by how alike two schools are, so the heading does not say it does. + * + * The chips report what a school shares, and may be absent entirely. That is + * information for the reader to weigh, not a verdict this section has already + * reached on their behalf. * * There is deliberately no "how these are chosen" panel: the method is already * visible in the lede, the chips and the distances. The single caption line is @@ -78,25 +82,20 @@ export function SimilarSchoolsSection({ // One card matched on phase alone, so the section may not claim the set // shares an intake with this school. - const loosest = Math.max(...schools.map((s) => s.tier)); const metricKey = schools[0].metric_key; const noun = nearbyNoun(phase); return ( -
+
-

- Similar schools nearby +

+ Other schools nearby

-

- {loosest >= 3 - ? `Other ${noun} near ${schoolName}.` - : `Other ${noun} near ${schoolName}, with a similar intake.`} -

+

{`Other ${noun} near ${schoolName}.`}

} > @@ -113,13 +112,13 @@ export function SimilarSchoolsSection({ .filter(Boolean) .join(' · ')}

-
    - {school.shared.map((label) => ( -
  • = 3 ? styles.chipLoose : styles.chip}> - {label} -
    • - ))} -
+ {school.shared.length > 0 && ( +
    + {school.shared.map((label) => ( +
  • {label}
    • + ))} +
+ )}