Added — GIAS identity/contact details on the school detail page
Seven school-identity fields now render in the detail header (both the primary/all-through view and the secondary view):
Chips: age range (Ages 4–11), religious character (hidden for "Does not apply"/"None"), nursery, sixth form
Header details: telephone (as a tel: link), county, parliamentary constituency
religious_denomination, age_range and has_sixth_form were already served by the API. telephone, nursery_provision, county, parliamentary_constituency are newly wired through:
data_loader.py — added to the main marts query, with a NULL-fallback query variant for un-rebuilt marts (mirrors the existing has_sixth_form fallback so an old mart degrades instead of failing the whole load)
app.py — added to the school_info response
Removed — three unwired UI sections
The backend never populated these (always null), so the components were dead:
Year 1 Phonics section (DfE only publishes phonics at national/LA level)
SEN "types of additional needs" breakdown
Average class size card
Along with their dead props, route plumbing, and the SenDetail/Phonics types + class_size_avg field.
## What & why
### Added — GIAS identity/contact details on the school detail page
Seven school-identity fields now render in the detail header (both the primary/all-through view and the secondary view):
- **Chips:** age range (`Ages 4–11`), religious character (hidden for "Does not apply"/"None"), nursery, sixth form
- **Header details:** telephone (as a `tel:` link), county, parliamentary constituency
`religious_denomination`, `age_range` and `has_sixth_form` were already served by the API. **telephone, nursery_provision, county, parliamentary_constituency** are newly wired through:
- `data_loader.py` — added to the main marts query, with a NULL-fallback query variant for un-rebuilt marts (mirrors the existing `has_sixth_form` fallback so an old mart degrades instead of failing the whole load)
- `app.py` — added to the `school_info` response
### Removed — three unwired UI sections
The backend never populated these (always `null`), so the components were dead:
- **Year 1 Phonics** section (DfE only publishes phonics at national/LA level)
- **SEN "types of additional needs"** breakdown
- **Average class size** card
Along with their dead props, route plumbing, and the `SenDetail`/`Phonics` types + `class_size_avg` field.
## Verification
- Frontend `tsc --noEmit`: clean
- Frontend jest: 124/124 pass
- Backend: module imports clean (query-variant asserts hold); `test_school_details`, `test_compare_enrichment`, `test_sixth_form_flag`: 13/13 pass
- E2E: extended the detail journey to assert the Phonics section is gone and the new header fields render when present (runs against staging in CI)
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Add seven school-identity fields to the detail header (both primary and
secondary views): age range, religious character, nursery and sixth-form
indicators as chips; telephone (tel: link), county and parliamentary
constituency as header details. religious_denomination, age_range and
has_sixth_form were already served; telephone, nursery_provision, county
and parliamentary_constituency are newly wired through the marts query
(with a NULL fallback for un-rebuilt marts, mirroring has_sixth_form) and
the school_info API response.
Remove three UI sections the backend never populated (always null): Year 1
Phonics, the SEN "types of additional needs" breakdown, and the average
class-size card — along with their now-dead props, route plumbing, and the
SenDetail/Phonics types + class_size_avg field.
Extend the e2e detail journey to assert the Phonics section is gone and the
new header fields render when the record carries them.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Previously the chip was hidden for "Does not apply"/"None". Show it in all
cases, rendering "None" for those two sentinel values.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This PR surfaces new GIAS/location fields (telephone, nursery provision, county, parliamentary constituency) end-to-end from SQL through the API to the detail pages, and removes the unwired Phonics/SEN-breakdown/class-size sections along with their now-dead types. The frontend and test changes are clean and consistent, but the backend's graceful-degradation fallback for missing mart columns has a regex gap that only accounts for the s. table alias.
🔴 Severe (blocks merge)
backend/data_loader.py: _MISSING_COLUMN_RE (line 342) is column "?(?:s\.)?(\w+)"? does not exist, which only strips an optional s. alias prefix. The two new dim_location columns (county, parliamentary_constituency) are selected via the l. alias, and Postgres reports undefined qualified columns unquoted as e.g. column l.county does not exist. Against that message the regex fails to match at all (since \w+ can't consume the l. prefix), so _missing_column_name returns None. If a mart rebuild state ever has dim_school already migrated (telephone/nursery_provision present) but dim_location not yet migrated (county/parliamentary_constituency missing) — plausible since these look like independently-rebuilt dbt models — the missing in _EXTRA_OPTIONAL_COLUMNS check at line 385 never matches, execution falls to the final else branch, and load_school_data_as_dataframe() returns an empty DataFrame, i.e. the entire site shows zero schools instead of gracefully degrading as intended. This exact case isn't covered by the existing tests in backend/tests/test_gias_translation.py, which only exercise the unqualified and s.-prefixed cases.
## 🤖 AI Code Review (Claude Code)
This PR surfaces new GIAS/location fields (telephone, nursery provision, county, parliamentary constituency) end-to-end from SQL through the API to the detail pages, and removes the unwired Phonics/SEN-breakdown/class-size sections along with their now-dead types. The frontend and test changes are clean and consistent, but the backend's graceful-degradation fallback for missing mart columns has a regex gap that only accounts for the `s.` table alias.
### 🔴 Severe (blocks merge)
- **backend/data_loader.py**: _MISSING_COLUMN_RE (line 342) is `column "?(?:s\.)?(\w+)"? does not exist`, which only strips an optional `s.` alias prefix. The two new dim_location columns (`county`, `parliamentary_constituency`) are selected via the `l.` alias, and Postgres reports undefined qualified columns unquoted as e.g. `column l.county does not exist`. Against that message the regex fails to match at all (since `\w+` can't consume the `l.` prefix), so `_missing_column_name` returns None. If a mart rebuild state ever has dim_school already migrated (telephone/nursery_provision present) but dim_location not yet migrated (county/parliamentary_constituency missing) — plausible since these look like independently-rebuilt dbt models — the `missing in _EXTRA_OPTIONAL_COLUMNS` check at line 385 never matches, execution falls to the final `else` branch, and `load_school_data_as_dataframe()` returns an empty DataFrame, i.e. the entire site shows zero schools instead of gracefully degrading as intended. This exact case isn't covered by the existing tests in backend/tests/test_gias_translation.py, which only exercise the unqualified and `s.`-prefixed cases.
The graceful-degradation fallback keys off the column named in a Postgres
UndefinedColumn error, but the matcher only stripped an `s.` alias. The two
new dim_location columns (county, parliamentary_constituency) are selected
via the `l.` alias and Postgres reports them unquoted as
"column l.county does not exist" — which the old regex failed to match at
all, returning None.
If dim_school is rebuilt (telephone/nursery present) but dim_location is not
yet (county/parliamentary_constituency missing) — plausible since they are
independently-rebuilt dbt models — the fallback branch never matched and
load_school_data_as_dataframe() returned an empty DataFrame, showing zero
schools sitewide instead of degrading those columns to NULL.
Generalise the alias prefix to `\w+\.` and cover the l.-qualified case in
tests.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This PR surfaces new GIAS/location fields (telephone, nursery_provision, county, parliamentary_constituency) end-to-end from the mart query through the API to both detail views, with a graceful-degradation fallback query for staging DBs whose mart hasn't been rebuilt yet, and removes the dead Phonics/SEN-detail/class-size UI sections that were never populated by the backend. The fallback mechanism, regex generalization, and frontend/type cleanup are internally consistent with existing patterns in the codebase (verified against dim_school.sql/dim_location.sql, models.py, and existing has_sixth_form/legacy-name fallback precedent), and new unit/e2e tests cover the changed behavior.
✅ No issues found.
## 🤖 AI Code Review (Claude Code)
This PR surfaces new GIAS/location fields (telephone, nursery_provision, county, parliamentary_constituency) end-to-end from the mart query through the API to both detail views, with a graceful-degradation fallback query for staging DBs whose mart hasn't been rebuilt yet, and removes the dead Phonics/SEN-detail/class-size UI sections that were never populated by the backend. The fallback mechanism, regex generalization, and frontend/type cleanup are internally consistent with existing patterns in the codebase (verified against dim_school.sql/dim_location.sql, models.py, and existing has_sixth_form/legacy-name fallback precedent), and new unit/e2e tests cover the changed behavior.
✅ No issues found.
tudor
merged commit 8cf2ac47c1 into main2026-07-24 10:56:34 +00:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
What & why
Added — GIAS identity/contact details on the school detail page
Seven school-identity fields now render in the detail header (both the primary/all-through view and the secondary view):
Ages 4–11), religious character (hidden for "Does not apply"/"None"), nursery, sixth formtel:link), county, parliamentary constituencyreligious_denomination,age_rangeandhas_sixth_formwere already served by the API. telephone, nursery_provision, county, parliamentary_constituency are newly wired through:data_loader.py— added to the main marts query, with a NULL-fallback query variant for un-rebuilt marts (mirrors the existinghas_sixth_formfallback so an old mart degrades instead of failing the whole load)app.py— added to theschool_inforesponseRemoved — three unwired UI sections
The backend never populated these (always
null), so the components were dead:Along with their dead props, route plumbing, and the
SenDetail/Phonicstypes +class_size_avgfield.Verification
tsc --noEmit: cleantest_school_details,test_compare_enrichment,test_sixth_form_flag: 13/13 pass🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This PR surfaces new GIAS/location fields (telephone, nursery provision, county, parliamentary constituency) end-to-end from SQL through the API to the detail pages, and removes the unwired Phonics/SEN-breakdown/class-size sections along with their now-dead types. The frontend and test changes are clean and consistent, but the backend's graceful-degradation fallback for missing mart columns has a regex gap that only accounts for the
s.table alias.🔴 Severe (blocks merge)
column "?(?:s\.)?(\w+)"? does not exist, which only strips an optionals.alias prefix. The two new dim_location columns (county,parliamentary_constituency) are selected via thel.alias, and Postgres reports undefined qualified columns unquoted as e.g.column l.county does not exist. Against that message the regex fails to match at all (since\w+can't consume thel.prefix), so_missing_column_namereturns None. If a mart rebuild state ever has dim_school already migrated (telephone/nursery_provision present) but dim_location not yet migrated (county/parliamentary_constituency missing) — plausible since these look like independently-rebuilt dbt models — themissing in _EXTRA_OPTIONAL_COLUMNScheck at line 385 never matches, execution falls to the finalelsebranch, andload_school_data_as_dataframe()returns an empty DataFrame, i.e. the entire site shows zero schools instead of gracefully degrading as intended. This exact case isn't covered by the existing tests in backend/tests/test_gias_translation.py, which only exercise the unqualified ands.-prefixed cases.🤖 AI Code Review (Claude Code)
This PR surfaces new GIAS/location fields (telephone, nursery_provision, county, parliamentary_constituency) end-to-end from the mart query through the API to both detail views, with a graceful-degradation fallback query for staging DBs whose mart hasn't been rebuilt yet, and removes the dead Phonics/SEN-detail/class-size UI sections that were never populated by the backend. The fallback mechanism, regex generalization, and frontend/type cleanup are internally consistent with existing patterns in the codebase (verified against dim_school.sql/dim_location.sql, models.py, and existing has_sixth_form/legacy-name fallback precedent), and new unit/e2e tests cover the changed behavior.
✅ No issues found.