feat: similar schools nearby on the detail page #150

Merged
tudor merged 12 commits from feat/similar-schools-nearby into main 2026-09-22 05:53:07 +00:00
Owner

A school page links outward to its place pages and nowhere else — it has never
linked to another school. This adds that edge: up to six nearby schools of the
same phase and a comparable intake, three at a time in a carousel, each a
crawlable link and each addable to the comparison basket.

Spec: docs/superpowers/specs/2026-09-21-similar-schools-nearby-design.md
Plan: docs/superpowers/plans/2026-09-21-similar-schools-nearby.md
Mockup: https://claude.ai/artifact/168KdUMcfkUeGWW2FGjuec

The rule everything hangs off

Two kinds of filter, and they are not interchangeable.

Hard filters encode claims the section may not make, so they never relax at
any distance — even where that means no section renders:

  • a selective school is not an alternative to a non-selective one
  • a special school, PRU or AP is not comparable to a mainstream one (the same
    reasoning as #70, which dropped the England benchmark for them)
  • a Girls school is not an option for a Boys school's reader

Soft filters describe closeness of fit, so they relax across three tiers at
3/5/10 miles — faith first, then gender exactness, because a faith mismatch
changes a school's character while a gender mismatch can mean the school is not
available at all.

Six, and when to stop widening

Six is a cap, not a quota. Tiers descend only until three schools are found;
the remaining slots fill from the tiers already opened, never by widening
again. Four tier-1 matches therefore never open tier 2.

Without that stopping rule a cap of six would reliably drag in tier-3 schools
ten miles away to fill a row three good matches had already earned. The
previous cap of three hid the problem; six exposes it.

Under two qualifying schools renders nothing at all — no padding, no empty
state, no nav item.

A carousel that scrolls, not one that paginates

Every card is in the server-rendered HTML; the arrows only move the viewport
across them. A widget that mounted cards on click would put four of the six
links beyond a crawler and beyond a reader with no JavaScript, which would
defeat the point of the section. With JS off it degrades to a scrollable row.

Mobile

Checked against MOBILE.md at 360 / 390 / 430, which turned up two real
failures rather than confirming a guess:

  • the arrows sat in the heading's flex row, taking 96px from a 328px card and
    crushing the lede into four lines. Below 640px they are gone, replaced by the
    right-edge scroll-fade MOBILE.md already documents for horizontal scrollers.
  • the arrow and add-to-compare buttons were 40px against a 44px floor.

All three widths now report zero horizontal overflow, no failing tap targets and
no text under 11px.

MOBILE.md asks for a Playwright width check and records that it was not written
because Playwright was not in the project. It is — so the journey carries one now,
scoped to this page.

Honesty rules

  • the lede claims "with a similar intake" only when no card came from tier 3
  • chips list what a school actually shares; a weaker match carries fewer, not a
    chip it has not earned
  • a missing figure reads "Not published", never 0 or blank
  • the neighbour's metric carries no valence colour: green and terracotta mean
    "against the England average" everywhere else, and using them here would read
    as ranking the neighbours against each other

Notable during the build

Series.apply on an emptied DataFrame returns a DataFrame, and using that as a
mask silently drops every column — so the next lookup raises KeyError rather
than yielding no rows. This fires whenever the provision filter empties the
frame, which is the ordinary case for a special school with no special school
near it. Fixed with a _mask() helper; the test that caught it is in the suite.

PHASE_GROUPS moves from app.py to schemas.py so the new module can share
it without an import cycle. All three call sites are unchanged.

Verification

  • backend + pipeline + CI suites: 247 passed (17 new for selection, 2 for the endpoint)
  • frontend: 464 passed across 55 suites, tsc --noEmit clean
  • the four new E2E journeys compile and register, but this project runs the E2E
    gate against staging after merge, so they are not proven by these checks

🤖 Generated with Claude Code

A school page links outward to its place pages and nowhere else — it has never linked to another school. This adds that edge: up to six nearby schools of the same phase and a comparable intake, three at a time in a carousel, each a crawlable link and each addable to the comparison basket. Spec: `docs/superpowers/specs/2026-09-21-similar-schools-nearby-design.md` Plan: `docs/superpowers/plans/2026-09-21-similar-schools-nearby.md` Mockup: https://claude.ai/artifact/168KdUMcfkUeGWW2FGjuec ## The rule everything hangs off Two kinds of filter, and they are not interchangeable. **Hard filters encode claims the section may not make**, so they never relax at any distance — even where that means no section renders: - a selective school is not an alternative to a non-selective one - a special school, PRU or AP is not comparable to a mainstream one (the same reasoning as #70, which dropped the England benchmark for them) - a Girls school is not an option for a Boys school's reader **Soft filters describe closeness of fit**, so they relax across three tiers at 3/5/10 miles — faith first, then gender exactness, because a faith mismatch changes a school's character while a gender mismatch can mean the school is not available at all. ## Six, and when to stop widening Six is a cap, not a quota. Tiers descend only until three schools are found; the remaining slots fill from the tiers already opened, never by widening again. Four tier-1 matches therefore never open tier 2. Without that stopping rule a cap of six would reliably drag in tier-3 schools ten miles away to fill a row three good matches had already earned. The previous cap of three hid the problem; six exposes it. Under two qualifying schools renders nothing at all — no padding, no empty state, no nav item. ## A carousel that scrolls, not one that paginates Every card is in the server-rendered HTML; the arrows only move the viewport across them. A widget that mounted cards on click would put four of the six links beyond a crawler and beyond a reader with no JavaScript, which would defeat the point of the section. With JS off it degrades to a scrollable row. ## Mobile Checked against `MOBILE.md` at 360 / 390 / 430, which turned up two real failures rather than confirming a guess: - the arrows sat in the heading's flex row, taking 96px from a 328px card and crushing the lede into four lines. Below 640px they are gone, replaced by the right-edge scroll-fade `MOBILE.md` already documents for horizontal scrollers. - the arrow and add-to-compare buttons were 40px against a 44px floor. All three widths now report zero horizontal overflow, no failing tap targets and no text under 11px. `MOBILE.md` asks for a Playwright width check and records that it was not written because Playwright was not in the project. It is — so the journey carries one now, scoped to this page. ## Honesty rules - the lede claims "with a similar intake" only when no card came from tier 3 - chips list what a school actually shares; a weaker match carries fewer, not a chip it has not earned - a missing figure reads "Not published", never 0 or blank - the neighbour's metric carries no valence colour: green and terracotta mean "against the England average" everywhere else, and using them here would read as ranking the neighbours against each other ## Notable during the build `Series.apply` on an emptied DataFrame returns a DataFrame, and using that as a mask silently drops every column — so the next lookup raises `KeyError` rather than yielding no rows. This fires whenever the provision filter empties the frame, which is the ordinary case for a special school with no special school near it. Fixed with a `_mask()` helper; the test that caught it is in the suite. `PHASE_GROUPS` moves from `app.py` to `schemas.py` so the new module can share it without an import cycle. All three call sites are unchanged. ## Verification - backend + pipeline + CI suites: 247 passed (17 new for selection, 2 for the endpoint) - frontend: 464 passed across 55 suites, `tsc --noEmit` clean - the four new E2E journeys compile and register, but this project runs the E2E gate against staging after merge, so they are not proven by these checks 🤖 Generated with [Claude Code](https://claude.com/claude-code)
tudor added 10 commits 2026-09-22 05:20:04 +00:00
A school page links outward to its places and never to another school.
This section adds that edge: three nearby schools of the same phase and a
comparable intake, each a crawlable link and each addable to the basket.

The design separates hard filters from soft preferences and never confuses
them. Selectivity, provision and opposite-sex intake are claims the section
cannot make, so they never relax, even where that means no section renders.
Gender and religious character describe closeness of fit, so they relax in
tiers — and the card states what actually survived rather than padding with
a match it did not earn.

Includes the mockup the design is drawn against, with all three tier states
live in both themes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Five tasks, each ending in a green test run and a commit: the pure
selection module, the endpoint key, the section and its two client
islands, the wiring into both templates, and the journey.

The selection logic gets its own module rather than another 200 lines in
app.py, which means the tier rules are testable against a synthetic frame
with no TestClient, no database and no monkeypatch. Moving PHASE_GROUPS
to schemas.py is what keeps that import acyclic.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The "how these schools are chosen" panel restated what the section already
shows — the phase in the lede, the shared characteristics on each card, the
distance above each name — so it cost space to say nothing new.

One line survives, and it is not a method note. A reader who sees "0.6 miles
away" and takes it for the walk has been misled by us, and no other element
on the card corrects that. The rest were claims the selection rules keep
true without narrating them.

Also records what happens past the third school: surplus matches are dropped
silently, because NearbyPlaces below already leads to the full lists.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Three schools is not a neighbourhood in inner London, so the cap is six with
three visible and arrows for the rest.

Raising the cap exposes something the old cap hid. Tiers exist to reach a
usable set, and with six slots a naive loop would keep widening to fill them
— dragging in tier-3 schools ten miles away to sit beside three good matches
that had already earned the row. So tiers now stop relaxing once three are
found, and the remaining slots are filled only from the tiers already used.
Four tier-1 matches never open tier 2.

The carousel scrolls a list rather than swapping a view: all six cards are in
the initial HTML, so every link stays crawlable and the row still scrolls with
JavaScript off. The arrows' edge test carries an 8px tolerance because the
scroller's focus-ring padding is the first snap position — a row at rest
reports scrollLeft 2, and an exact test for 0 left the back arrow live and
pointing nowhere. Caught in the mockup.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Checked the section against MOBILE.md at its three reference widths instead
of assuming the breakpoints were enough. Two real failures at 360px.

The arrows sat in the heading's flex row, taking 96px from a 328px card and
crushing the lede into a four-line column — for a control that swiping
already provides. Below 640px they are now gone, the header is a single
column, one card shows at 86% so the next one peeks, and the affordance is
the right-edge scroll-fade MOBILE.md already documents for horizontal
scrollers. The fade lifts at the end of the travel, so the at-end state is
computed whether or not an arrow exists to consume it.

The arrow and add-to-compare buttons were 40px against a 44px floor. Both
are 44 now. A card title's own box is shorter, but its hit area is the whole
card through the ::after overlay, so it passes on the target that actually
receives the tap.

360, 390 and 430 now all report zero overflow, no failing tap targets and no
text under 11px. MOBILE.md wanted a Playwright width check and recorded that
Playwright was not in the project; it is, so the journey now carries one for
this page.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Hard filters encode claims the section may not make — a selective school is
not an alternative to a non-selective one, a special school is not comparable
to a mainstream one, a Girls school is not an option for a Boys school's
reader — so they never relax. Soft preferences describe closeness of fit, so
they relax across three tiers, and only far enough to reach three; the
remaining slots up to six fill from the tiers already opened.

PHASE_GROUPS moves to schemas.py so this module can share it without
importing app, which would be a cycle.

_mask() exists because Series.apply on an empty Series returns a DataFrame,
and using that as a mask drops every column — so the next lookup raises
KeyError instead of yielding no rows. A special school with no special school
near it empties the frame at the provision filter, which is the ordinary case
for most special schools, so this was a crash on a common path rather than an
edge case.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Rides in the existing payload rather than a new endpoint: the page already
makes one server fetch for its data, and /school/[slug] regenerates weekly,
so the per-request cost is paid once per school per week.

Wrapped so a failure in selection never 500s a page that is otherwise
complete — the posture get_supplementary_data already takes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Server-rendered cards inside a client carousel that scrolls rather than
paginates, so all six links stay in the initial HTML and the row still works
with JavaScript off. Three client islands, split by what each needs: a school,
the whole selection, a DOM ref.

The lede claims a similar intake only when no card came from tier 3, chips
list what a school actually shares, a missing figure reads "Not published",
and the neighbour's number carries no valence colour — green and terracotta
mean "against England" everywhere else, and colouring it here would read as
ranking the neighbours.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Inside SchoolDetailShell rather than after it, because the sticky nav's
scroll-spy finds sections with getElementById and can only reach one that
lives in the shell. Last in the order, and last in the nav, because the two
must agree or the nav links to an anchor that was never rendered.

hasSimilarSchools is optional on NavItemsInput, matching hasLocation beside
it: absent has to mean "no section", and making it required would have
churned ten unrelated call sites for no added safety.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
test(e2e): cover the similar-schools section, compare hand-off and mobile widths
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m13s
PR Checks / Backend Smoke (pull_request) Successful in 9s
PR Checks / Build Backend (no push) (pull_request) Successful in 37s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m19s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m15s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 3m1s
bd2a6c385b
Two things here cannot be covered anywhere else. jsdom has no layout, so
scrollWidth and clientWidth are both 0 and the arrows' disabled state can only
be measured by a real engine. And the scroll position surviving a selection is
DOM state rather than React state, so only a real browser can prove the row
does not jump back when the footer re-renders.

MOBILE.md asks for a Playwright width check and records that it was not written
because Playwright was not in the project. It is — this suite — so the check
exists now, scoped to the page this feature touches.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

🤖 AI Code Review (Claude Code)

This PR adds a backend-selected 'similar schools nearby' feature: a pure, well-tested pandas module (backend/similar_schools.py) applying hard filters (phase, provision, selectivity, gender) and tiered soft preferences (faith, gender exactness) to pick up to 6 nearby schools, served on the existing /api/schools/{urn} payload behind a try/except that degrades to an empty list on failure. The backend logic is thorough, defensively written (e.g. the _mask helper avoids a real pandas empty-Series/apply pitfall), and covered by extensive unit tests; no security, data-loss, or CI/deploy issues were found. The frontend component diffs (SimilarSchoolsSection.tsx, carousel, compare bar, etc.) were not present in the supplied diff, so they could not be reviewed here.

🟡 Minor

  • backend/app.py: _similar_schools_payload's is_secondary check ("secondary" in phase_text) does not treat the GIAS phase value "16 plus" as secondary, unlike PHASE_GROUPS which explicitly includes it in the secondary group. A school with raw phase "16 plus" would be treated as primary here (wrong metric key and wrong candidate phase group), silently producing incorrect similar-schools data rather than a crash.
## 🤖 AI Code Review (Claude Code) This PR adds a backend-selected 'similar schools nearby' feature: a pure, well-tested pandas module (backend/similar_schools.py) applying hard filters (phase, provision, selectivity, gender) and tiered soft preferences (faith, gender exactness) to pick up to 6 nearby schools, served on the existing /api/schools/{urn} payload behind a try/except that degrades to an empty list on failure. The backend logic is thorough, defensively written (e.g. the _mask helper avoids a real pandas empty-Series/apply pitfall), and covered by extensive unit tests; no security, data-loss, or CI/deploy issues were found. The frontend component diffs (SimilarSchoolsSection.tsx, carousel, compare bar, etc.) were not present in the supplied diff, so they could not be reviewed here. ### 🟡 Minor - **backend/app.py**: _similar_schools_payload's is_secondary check (`"secondary" in phase_text`) does not treat the GIAS phase value "16 plus" as secondary, unlike PHASE_GROUPS which explicitly includes it in the secondary group. A school with raw phase "16 plus" would be treated as primary here (wrong metric key and wrong candidate phase group), silently producing incorrect similar-schools data rather than a crash.
tudor added 1 commit 2026-09-22 05:41:51 +00:00
fix(api): treat "16 plus" as secondary, the way PHASE_GROUPS already does
PR Checks / Frontend Typecheck + Tests (pull_request) Canceled after 9s
PR Checks / Backend Smoke (pull_request) Canceled after 0s
PR Checks / Build Backend (no push) (pull_request) Canceled after 0s
PR Checks / Build Frontend (no push) (pull_request) Canceled after 0s
PR Checks / Build Pipeline (no push) (pull_request) Canceled after 0s
PR Checks / AI Code Review (Claude) (pull_request) Canceled after 0s
2175dccb7c
GIAS phase 6 is "16 plus", and PHASE_GROUPS deliberately files it in the
secondary group. The payload helper decided the same question with
`"secondary" in phase_text`, which that value does not satisfy — so a
sixth-form college was handed the primary bucket and offered infant schools
as its peers, with the KS2 metric key to label them. No crash; just a page
confidently showing the wrong schools.

The decision now lives in similar_schools.is_secondary_phase, beside the
PHASE_GROUPS bucket it selects from, so the two cannot drift again. A test
pins them together.

The same binary assumption had a second output. computeSchoolFlags tests for
the substring too, so a 16-plus school renders the primary template, and the
composer was labelling the section from the template: "Other primary schools
near <sixth form college>" above a row of secondaries. The section now
derives its noun from the school's own phase, which also removes the
duplicated wording from both composers. A 16-plus school's candidates span
the whole secondary group, so no single noun fits and it gets the honest
general one.

Reported in review on #150.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
tudor added 1 commit 2026-09-22 05:42:00 +00:00
docs: correct the metric rule for "16 plus"
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m11s
PR Checks / Backend Smoke (pull_request) Successful in 10s
PR Checks / Build Backend (no push) (pull_request) Successful in 31s
PR Checks / Build Frontend (no push) (pull_request) Successful in 1m18s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 1m15s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m11s
83dc5ae5dc
The spec said the metric follows the template the page renders. That is no
longer true for GIAS phase 6: a sixth-form college renders the primary
template but is matched, correctly, against secondaries. The rule is phase
group membership, decided once in is_secondary_phase — and the section's lede
noun comes from the school's phase rather than its template for the same
reason.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

🤖 AI Code Review (Claude Code)

This PR adds a backend-driven 'similar schools nearby' feature: a pure selection module (backend/similar_schools.py) with hard filters (phase, provision, selectivity, gender) and tiered soft preferences (faith, then gender exactness) over distance, wired into the /api/schools/{urn} endpoint behind a broad try/except so a selection failure can't break the page. It also relocates PHASE_GROUPS from app.py into schemas.py to avoid a circular import with the new module, and adds extensive backend unit tests plus e2e coverage. The backend logic is thorough, well-tested, and consistent with the previously-landed '16 plus' phase-grouping fix; no correctness, security, or data-loss issues were found.

🟡 Minor

  • backend/app.py: _similar_schools_payload does import logging inside the except block and calls logging.getLogger(__name__) on every failure instead of using a module-level logger (as the rest of app.py likely does, e.g. via a logger already used for get_supplementary_data's error handling). Harmless but inconsistent style.
## 🤖 AI Code Review (Claude Code) This PR adds a backend-driven 'similar schools nearby' feature: a pure selection module (backend/similar_schools.py) with hard filters (phase, provision, selectivity, gender) and tiered soft preferences (faith, then gender exactness) over distance, wired into the /api/schools/{urn} endpoint behind a broad try/except so a selection failure can't break the page. It also relocates PHASE_GROUPS from app.py into schemas.py to avoid a circular import with the new module, and adds extensive backend unit tests plus e2e coverage. The backend logic is thorough, well-tested, and consistent with the previously-landed '16 plus' phase-grouping fix; no correctness, security, or data-loss issues were found. ### 🟡 Minor - **backend/app.py**: _similar_schools_payload does `import logging` inside the except block and calls `logging.getLogger(__name__)` on every failure instead of using a module-level logger (as the rest of app.py likely does, e.g. via a `logger` already used for get_supplementary_data's error handling). Harmless but inconsistent style.
tudor merged commit 151cf4bc80 into main 2026-09-22 05:53:07 +00:00
Sign in to join this conversation.
No Reviewers
No labels
2 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: tudor/school_compare#150