fix(compare): five must-fix findings from the final expert review #50

Merged
tudor merged 8 commits from fix/compare-final-review-mustfix into main 2026-07-16 19:16:54 +00:00
Owner

Fixes the five must-fix findings from the Ofsted expert's final staging review of the compare screen (see docs/superpowers/plans/2026-07-16-compare-mustfix-final-review.md for full root-cause evidence).

Fixes

  1. Blank all-secondary compare view — the auto-phase effect bailed while the basket was still hydrating and never re-fired (deps were only [comparisonData]), leaving the page stuck on an empty "primary" tab. Fixed with a Jest regression test and a new e2e secondary-comparison journey.
  2. Report cards dated with 2021 legacy inspection dates — the MI file's report-card grades belong to the latest full inspection (its own "Inspection start date" column), but the tap dated everything with the legacy OEIF/ungraded dates. New rc_inspection_date carried tap → staging → mart → API → UI; report cards are dated only by their own inspection (em dash until the data lands, never the legacy date). Also fixed discover_csv_url(), which returned the oldest (2017) file on the GOV.UK page.
  3. FSM chip benchmarked against the wrong measurefsm_pct ?? disadvantaged_pct fallback removed; new fact_census_benchmarks mart supplies pupil-weighted FSM/EAL means per phase. The junk "secondary" medians computed from KS2-only columns (the impossible 50% disadvantaged anchor) are gone.
  4. KS4 "national averages" were dataset means, footnoted as official — new ees_ks4_national stream ingests the official EES series (2024/25 A8 = 46.0; P8 legitimately unpublished that year). The API's computed fallback is removed: an unbuilt mart yields an empty series, never a mislabelled stand-in.
  5. 2021/22 footnote — expert finding refuted on re-verification: the GOV.UK announcement "Primary school performance tables: 2022" is CANCELLED ("will not be published in key stage 2 performance tables in academic year 2021/22"). The existing copy is correct; spec §8.1 now records the re-verification.

After merge

Findings 2–4 need the daily DAG to run (fresh Ofsted extract + new marts) before staging shows corrected data. Then verify on staging:

  • Barclay (138690): ofsted.rc_inspection_date == "2026-02-03"; report card no longer dated Oct 2021.
  • Watford Grammar (136276): report_card == {} (stale rc grades cleared by the fresh extract).
  • /api/national-averages secondary: attainment_8_score == 46.0 for 202425, no progress_8_score.
  • /api/compare benchmarks: fsm_pct ≈ 24–26 both phases; secondary.disadvantaged_pct == null.
  • An all-secondary /compare URL renders all sections (also covered by the new e2e journey).

Production promotion remains the human's manual call.

🤖 Generated with Claude Code

https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB

Fixes the five must-fix findings from the Ofsted expert's final staging review of the compare screen (see `docs/superpowers/plans/2026-07-16-compare-mustfix-final-review.md` for full root-cause evidence). ## Fixes 1. **Blank all-secondary compare view** — the auto-phase effect bailed while the basket was still hydrating and never re-fired (deps were only `[comparisonData]`), leaving the page stuck on an empty "primary" tab. Fixed with a Jest regression test and a new e2e secondary-comparison journey. 2. **Report cards dated with 2021 legacy inspection dates** — the MI file's report-card grades belong to the *latest full inspection* (its own "Inspection start date" column), but the tap dated everything with the legacy OEIF/ungraded dates. New `rc_inspection_date` carried tap → staging → mart → API → UI; report cards are dated only by their own inspection (em dash until the data lands, never the legacy date). Also fixed `discover_csv_url()`, which returned the oldest (2017) file on the GOV.UK page. 3. **FSM chip benchmarked against the wrong measure** — `fsm_pct ?? disadvantaged_pct` fallback removed; new `fact_census_benchmarks` mart supplies pupil-weighted FSM/EAL means per phase. The junk "secondary" medians computed from KS2-only columns (the impossible 50% disadvantaged anchor) are gone. 4. **KS4 "national averages" were dataset means, footnoted as official** — new `ees_ks4_national` stream ingests the official EES series (2024/25 A8 = 46.0; P8 legitimately unpublished that year). The API's computed fallback is removed: an unbuilt mart yields an empty series, never a mislabelled stand-in. 5. **2021/22 footnote** — expert finding **refuted on re-verification**: the GOV.UK announcement "Primary school performance tables: 2022" is CANCELLED ("will not be published in key stage 2 performance tables in academic year 2021/22"). The existing copy is correct; spec §8.1 now records the re-verification. ## After merge Findings 2–4 need the **daily DAG to run** (fresh Ofsted extract + new marts) before staging shows corrected data. Then verify on staging: - Barclay (138690): `ofsted.rc_inspection_date == "2026-02-03"`; report card no longer dated Oct 2021. - Watford Grammar (136276): `report_card == {}` (stale rc grades cleared by the fresh extract). - `/api/national-averages` secondary: `attainment_8_score == 46.0` for 202425, no `progress_8_score`. - `/api/compare` benchmarks: `fsm_pct` ≈ 24–26 both phases; `secondary.disadvantaged_pct == null`. - An all-secondary `/compare` URL renders all sections (also covered by the new e2e journey). Production promotion remains the human's manual call. 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
tudor added 7 commits 2026-07-16 18:13:02 +00:00
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
The MI file's report-card grade columns belong to the latest FULL
inspection (col 'Inspection start date'), but inspection_date maps to the
legacy OEIF graded/ungraded dates — so report cards were being dated with
pre-Nov-2025 inspections. Also discover_csv_url() returned matches[0],
the oldest (2017) link on the GOV.UK page.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
The FSM chip anchored against disadvantaged_pct (a different measure,
FSM6+CLA) whenever fsm_pct was null — which it always was, since the
performance df has no fsm_pct. New fact_census_benchmarks mart supplies
pupil-weighted FSM/EAL means per phase; the KS2-column medians that
produced a bogus 50% 'secondary disadvantaged' anchor are gone.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
New ees_ks4_national stream ingests the EES 'National characteristics
summary data' series (England, state-funded, all pupils). The old mart's
unweighted school means were 7-15 points off every headline measure and
produced an impossible national Progress 8 (-0.27). The API's computed
fallback is gone too: the footnote calls these figures official, so an
unbuilt mart now yields an empty series, never a stand-in.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
docs(spec): re-verify the 2021/22 KS2 gap — expert finding refuted, footnote copy stands
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m5s
PR Checks / Backend Smoke (pull_request) Successful in 7s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 47s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 47s
PR Checks / AI Code Review (Claude) (pull_request) Successful in 4m1s
77d7052662
The final review claimed DfE published school-level 2021/22 KS2 in Dec
2022. Re-verified: the GOV.UK announcement 'Primary school performance
tables: 2022' is CANCELLED ('will not be published in key stage 2
performance tables in academic year 2021/22'), and the EES 2021/22
release carries the same statement. No code change.

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

🤖 AI Code Review (Claude Code)

This PR fixes five staging-review must-fix issues in the school compare feature: it renders all-secondary comparisons, dates report cards with their own inspection date instead of a legacy one, sources FSM/EAL benchmarks from a new census mart instead of medianing incompatible columns, replaces computed KS4 'national averages' with official DfE figures from a new EES stream, and (per the final commit) reconfirms — rather than changes — the 2021/22 footnote copy after re-verifying the underlying DfE claim. The code changes are internally consistent, well-tested, and the tap/dbt/backend/frontend layers line up correctly (column names, model fields, and API payload shapes all match); the one issue is that generated dbt build artifacts were accidentally committed alongside the source changes.

🟡 Minor

  • pipeline/transform/target/manifest.json: Generated dbt build artifact committed to the repo (along with pipeline/transform/logs/dbt.log and pipeline/transform/.user.yml). These embed the developer's local absolute filesystem paths (/Users/tudor/projects/school_compare/...) and a personal anonymous-usage-tracking UUID, bloat the diff with non-source content, and a stale manifest can cause confusing dbt partial-parse/version-mismatch warnings for other contributors or CI. Should be excluded via .gitignore rather than committed.
## 🤖 AI Code Review (Claude Code) This PR fixes five staging-review must-fix issues in the school compare feature: it renders all-secondary comparisons, dates report cards with their own inspection date instead of a legacy one, sources FSM/EAL benchmarks from a new census mart instead of medianing incompatible columns, replaces computed KS4 'national averages' with official DfE figures from a new EES stream, and (per the final commit) reconfirms — rather than changes — the 2021/22 footnote copy after re-verifying the underlying DfE claim. The code changes are internally consistent, well-tested, and the tap/dbt/backend/frontend layers line up correctly (column names, model fields, and API payload shapes all match); the one issue is that generated dbt build artifacts were accidentally committed alongside the source changes. ### 🟡 Minor - **pipeline/transform/target/manifest.json**: Generated dbt build artifact committed to the repo (along with pipeline/transform/logs/dbt.log and pipeline/transform/.user.yml). These embed the developer's local absolute filesystem paths (/Users/tudor/projects/school_compare/...) and a personal anonymous-usage-tracking UUID, bloat the diff with non-source content, and a stale manifest can cause confusing dbt partial-parse/version-mismatch warnings for other contributors or CI. Should be excluded via .gitignore rather than committed.
tudor added 1 commit 2026-07-16 19:09:13 +00:00
chore: untrack dbt build artifacts (target/, logs/, .user.yml) and gitignore them
PR Checks / Frontend Typecheck + Tests (pull_request) Successful in 1m3s
PR Checks / Backend Smoke (pull_request) Successful in 6s
PR Checks / Build Backend (no push) (pull_request) Successful in 17s
PR Checks / Build Frontend (no push) (pull_request) Successful in 46s
PR Checks / Build Pipeline (no push) (pull_request) Successful in 46s
PR Checks / AI Code Review (Claude) (pull_request) Failing after 3m25s
852ed11e4d
Swept in accidentally by a broad 'git add pipeline'. They embed local
absolute paths and a personal usage-tracking UUID, and a stale committed
manifest causes partial-parse/version-mismatch noise for others.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0146VHeLAWjDVE2B5uU67jCB
tudor merged commit 20a27f3958 into main 2026-07-16 19:16:54 +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#50