fix(compare): expert should-fixes S1-S4, S6 (S5/S7 closed by verification) #55

Merged
tudor merged 2 commits from fix/expert-signoff-shouldfixes into main 2026-07-17 17:53:16 +00:00
Owner

Implements the expert's should-fix findings S1–S4 and S6; S5 and S7 were closed by verification (see below).

Code changes

  • S1 — banded "missed out" chip: < 50% offered → "More than half of first choices missed out"; 50–66% → "About 1 in 3…"; 67–74% → "Over 1 in 4…" (previously one blanket chip that understated the worst cases by half).
  • S2 — Progress 8 absence explained: for 2024/25+ cohorts the row says "Not published — this GCSE year group sat no KS2 tests (COVID), so DfE has no baseline to measure progress from" instead of a bare "No data".
  • S3 — KS4 trend gap caption + axis label: secondary charts now carry their own honest caption (2019/20–2020/21 school-level GCSE unpublished due to COVID grading; later missing years are a dataset gap, not a DfE one) — deliberately different from the KS2 caption, which stays as-is. Y-axis "Value" → "Score". buildCompareChart now exposes englandOnlyYears.
  • S4 — all-through framing: chip meta gains an "All-through" label; the rail caption says "N schools · primary view" when the basket isn't purely one phase; an all-through school's whole-school roll shows "Whole-school roll (all-through, all ages)" instead of being judged against the single-phase median; "Who goes there" carries an all-ages caveat when an all-through school is present.
  • S6 — disadvantaged cohort sizes (spec §8.5, finally): the row now shows "of ~N disadvantaged pupils" computed from eligible pupils × disadvantaged share.

Closed by verification (no code)

  • S5 — Watford Grammar "Church of England" / "Non-selective": matches the official Ofsted MI record exactly (the Watford Grammars are officially partially-selective comprehensives). Not a join bug; our data is faithful to source.
  • S7 — Barclay/Elmhurst same report-card date: confirmed against reports.ofsted.gov.uk — both genuinely inspected 3 February 2026 (Barclay's report published 20 March 2026). Coincidence, not a join bug.

Tests: 100 frontend green (new coverage for the chip bands and englandOnlyYears); these are copy/framing refinements within journeys already covered by e2e, so no new journey assertions were added.

🤖 Generated with Claude Code

https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB

Implements the expert's should-fix findings S1–S4 and S6; S5 and S7 were closed by verification (see below). ## Code changes - **S1 — banded "missed out" chip**: `< 50%` offered → "More than half of first choices missed out"; `50–66%` → "About 1 in 3…"; `67–74%` → "Over 1 in 4…" (previously one blanket chip that understated the worst cases by half). - **S2 — Progress 8 absence explained**: for 2024/25+ cohorts the row says "Not published — this GCSE year group sat no KS2 tests (COVID), so DfE has no baseline to measure progress from" instead of a bare "No data". - **S3 — KS4 trend gap caption + axis label**: secondary charts now carry their own honest caption (2019/20–2020/21 school-level GCSE unpublished due to COVID grading; later missing years are a dataset gap, not a DfE one) — deliberately different from the KS2 caption, which stays as-is. Y-axis "Value" → "Score". `buildCompareChart` now exposes `englandOnlyYears`. - **S4 — all-through framing**: chip meta gains an "All-through" label; the rail caption says "N schools · primary view" when the basket isn't purely one phase; an all-through school's whole-school roll shows "Whole-school roll (all-through, all ages)" instead of being judged against the single-phase median; "Who goes there" carries an all-ages caveat when an all-through school is present. - **S6 — disadvantaged cohort sizes (spec §8.5, finally)**: the row now shows "of ~N disadvantaged pupils" computed from eligible pupils × disadvantaged share. ## Closed by verification (no code) - **S5 — Watford Grammar "Church of England" / "Non-selective"**: matches the official Ofsted MI record exactly (the Watford Grammars are officially partially-selective comprehensives). Not a join bug; our data is faithful to source. - **S7 — Barclay/Elmhurst same report-card date**: confirmed against reports.ofsted.gov.uk — both genuinely inspected 3 February 2026 (Barclay's report published 20 March 2026). Coincidence, not a join bug. Tests: 100 frontend green (new coverage for the chip bands and `englandOnlyYears`); these are copy/framing refinements within journeys already covered by e2e, so no new journey assertions were added. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
tudor added 1 commit 2026-07-17 16:41:35 +00:00
fix(compare): expert should-fixes S1-S4, S6 — banded chips, P8 reason, KS4 gap caption, all-through framing, cohort sizes
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m2s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 49s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 3m8s
f3fa12806b
S1: first-choice chip banded (More than half / About 1 in 3 / Over 1 in 4
missed out) so a 44%-offered grammar isn't understated by half.
S2: Progress 8 explains its absence for 2024/25+ cohorts (no KS2 baseline,
COVID) instead of a bare 'No data'.
S3: KS4 trend charts get their own honest gap caption (2019/20-2020/21
unpublished; later years not in our dataset yet); y-axis 'Value'→'Score';
buildCompareChart exposes englandOnlyYears.
S4: all-through schools labelled in chips, rail caption says 'N schools ·
<phase> view' for mixed baskets, whole-school roll no longer judged
against the single-phase median, community section carries an all-ages
caveat.
S6 (spec §8.5): disadvantaged attainment shows the cohort behind it
('of ~50 disadvantaged pupils').

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB

🤖 AI Code Review (Claude Code)

This PR adds honest, phase-aware copy and context to the compare UI: banded admissions-miss chips, KS4-specific chart gap captions, all-through school framing (roll size, community, phase labels), a Progress-8-not-published explanation, and disadvantaged-pupil cohort sizing. The changes are UI/copy-only, well-tested with new unit tests, and don't touch data pipelines, auth, or deploy config.

🟡 Minor

  • nextjs-app/components/compare/CompareAcademics.tsx: p8NotPublished is derived from the max latest data-year across all compared schools, then applied uniformly to every school's blank Progress 8 cell. A school whose data is missing for an unrelated reason (not the 2024/25 COVID-cohort gap) sitting next to a school with 2024/25 data would incorrectly show the 'no KS2 baseline (COVID)' explanation.
  • nextjs-app/components/compare/CompareAcademics.tsx: The new '~N disadvantaged pupils' figure multiplies eligible_pupils and disadvantaged_pct, each independently resolved via latestValues (which picks the latest year with a non-null value per metric). If these fields are populated for different years for a given school, the cohort count shown can be computed from a mismatched year versus the displayed percentage.
## 🤖 AI Code Review (Claude Code) This PR adds honest, phase-aware copy and context to the compare UI: banded admissions-miss chips, KS4-specific chart gap captions, all-through school framing (roll size, community, phase labels), a Progress-8-not-published explanation, and disadvantaged-pupil cohort sizing. The changes are UI/copy-only, well-tested with new unit tests, and don't touch data pipelines, auth, or deploy config. ### 🟡 Minor - **nextjs-app/components/compare/CompareAcademics.tsx**: `p8NotPublished` is derived from the max latest data-year across all compared schools, then applied uniformly to every school's blank Progress 8 cell. A school whose data is missing for an unrelated reason (not the 2024/25 COVID-cohort gap) sitting next to a school with 2024/25 data would incorrectly show the 'no KS2 baseline (COVID)' explanation. - **nextjs-app/components/compare/CompareAcademics.tsx**: The new '~N disadvantaged pupils' figure multiplies `eligible_pupils` and `disadvantaged_pct`, each independently resolved via `latestValues` (which picks the latest year with a non-null value per metric). If these fields are populated for different years for a given school, the cohort count shown can be computed from a mismatched year versus the displayed percentage.
tudor added 1 commit 2026-07-17 16:53:10 +00:00
fix(compare): per-school P8 explanation; disadvantaged cohort from the same yearly row
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 10s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 10s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 2m24s
200a97d0b9
Review findings: (1) the 'no KS2 baseline (COVID)' explanation was derived
from the basket-wide max year, so a school with an unrelated data gap
borrowed it from a neighbour with 2024/25 data — now judged per school on
its own latest year. (2) The '~N disadvantaged pupils' cohort multiplied
eligible_pupils and disadvantaged_pct resolved independently across years
— now both come from the same yearly row that supplies the displayed
percentage.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB

🤖 AI Code Review (Claude Code)

This PR is a frontend-only UX/copy refinement to the school comparison views: it adds honest, per-school explanations for missing Progress 8 data and KS4 chart gaps, banded admissions chips, all-through school framing (labels/roll size/community stats), and disadvantaged-cohort sizing, backed by new unit tests. I verified the cohort-size math against the actual eligible_pupils/disadvantaged_pct field semantics in the backend, the year-sort assumption backing the P8-not-published heuristic, the phase string values ("All-through") against the new regex checks, and the chart note branching logic — all are correct and consistent with existing conventions.

No issues found.

## 🤖 AI Code Review (Claude Code) This PR is a frontend-only UX/copy refinement to the school comparison views: it adds honest, per-school explanations for missing Progress 8 data and KS4 chart gaps, banded admissions chips, all-through school framing (labels/roll size/community stats), and disadvantaged-cohort sizing, backed by new unit tests. I verified the cohort-size math against the actual `eligible_pupils`/`disadvantaged_pct` field semantics in the backend, the year-sort assumption backing the P8-not-published heuristic, the `phase` string values (`"All-through"`) against the new regex checks, and the chart note branching logic — all are correct and consistent with existing conventions. ✅ No issues found.
tudor merged commit 284215fbce into main 2026-07-17 17:53:16 +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#55