Schools without KS2/KS4 results (special post-16 institutions, sixth-form centres, PRUs, brand-new schools) 500 on GET /api/schools/{urn}: the marts LEFT JOIN yields NaN in every numeric column, school_info passed those raw pandas values into JSONResponse (which renders with allow_nan=False), and serialization raised ValueError. The frontend collapses the failed fetch into notFound(), so every such school — e.g. /school/150275-west-london-performing-arts-academy-wlpa-academy — is a dead 404 SEO landing page. All nine year-null schools sampled on prod reproduce it.
Fix: pass every school_info value through the existing convert_to_native(), the same treatment yearly_data already gets from clean_for_json().
Tests:
New backend/tests/test_school_details.py (TestClient + monkeypatched loader) reproducing the 500 — red before the fix, green after; PR checks now run pytest backend/tests (with httpx<0.28 for the pinned Starlette).
New e2e journey: finds a results-less school via the search API and asserts the detail API returns 200 and the page renders. Verified it fails against prod today on exactly the fixed assertion.
Follow-up candidate (not in this PR): the school page maps any fetch failure to notFound(), which is how a 500 got misfiled as a routing 404 — worth distinguishing 5xx from real 404s.
Schools without KS2/KS4 results (special post-16 institutions, sixth-form centres, PRUs, brand-new schools) 500 on `GET /api/schools/{urn}`: the marts LEFT JOIN yields NaN in every numeric column, `school_info` passed those raw pandas values into `JSONResponse` (which renders with `allow_nan=False`), and serialization raised `ValueError`. The frontend collapses the failed fetch into `notFound()`, so every such school — e.g. `/school/150275-west-london-performing-arts-academy-wlpa-academy` — is a dead 404 SEO landing page. All nine year-null schools sampled on prod reproduce it.
**Fix:** pass every `school_info` value through the existing `convert_to_native()`, the same treatment `yearly_data` already gets from `clean_for_json()`.
**Tests:**
- New `backend/tests/test_school_details.py` (TestClient + monkeypatched loader) reproducing the 500 — red before the fix, green after; PR checks now run `pytest backend/tests` (with `httpx<0.28` for the pinned Starlette).
- New e2e journey: finds a results-less school via the search API and asserts the detail API returns 200 and the page renders. Verified it fails against prod today on exactly the fixed assertion.
**Follow-up candidate (not in this PR):** the school page maps any fetch failure to `notFound()`, which is how a 500 got misfiled as a routing 404 — worth distinguishing 5xx from real 404s.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Schools without KS2/KS4 results (special post-16 institutions, sixth-form
centres, PRUs, new schools) come back from the marts LEFT JOIN with NaN in
every numeric column. school_info passed those raw pandas values straight
into JSONResponse, which renders with allow_nan=False, so the detail
endpoint 500d and the frontend turned that into a 404 on every such SEO
landing page.
Run school_info values through convert_to_native (the same treatment
yearly_data already gets), add backend unit tests plus a pytest step in PR
checks, and an e2e journey that finds a results-less school via the search
API and asserts its page renders.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This PR fixes a real bug where schools with no performance rows (post-16 institutions, PRUs, new schools) return NaN in GIAS/location fields, causing JSONResponse to raise ValueError and 500 the /api/schools/{urn} endpoint; it now runs every school_info field through convert_to_native, matching the treatment yearly_data already gets via clean_for_json. Backend pytest and a corresponding Playwright e2e regression test are added and wired into CI. The change is small, well-tested, and low risk — no severe issues found.
🟡 Minor
backend/tests/pycache/: Compiled .pyc binary artifacts (init.cpython-312.pyc and test_school_details.cpython-312-pytest-9.1.1.pyc) were committed to the repo. .gitignore only excludes backend/pycache, not backend/tests/pycache, so these bytecode files leaked in; they should be removed and the gitignore pattern broadened (e.g. **/pycache/).
.gitea/workflows/pr-checks.yml: pytest and "httpx<0.28" are installed ad hoc in the CI step but not added to requirements.txt or a requirements-dev.txt, so local test runs can silently use different/newer httpx than CI, and the dependency isn't discoverable from the repo's normal dependency manifest.
## 🤖 AI Code Review (Claude Code)
This PR fixes a real bug where schools with no performance rows (post-16 institutions, PRUs, new schools) return NaN in GIAS/location fields, causing JSONResponse to raise ValueError and 500 the /api/schools/{urn} endpoint; it now runs every school_info field through convert_to_native, matching the treatment yearly_data already gets via clean_for_json. Backend pytest and a corresponding Playwright e2e regression test are added and wired into CI. The change is small, well-tested, and low risk — no severe issues found.
### 🟡 Minor
- **backend/tests/__pycache__/**: Compiled .pyc binary artifacts (__init__.cpython-312.pyc and test_school_details.cpython-312-pytest-9.1.1.pyc) were committed to the repo. .gitignore only excludes backend/__pycache__, not backend/tests/__pycache__, so these bytecode files leaked in; they should be removed and the gitignore pattern broadened (e.g. **/__pycache__/).
- **.gitea/workflows/pr-checks.yml**: pytest and "httpx<0.28" are installed ad hoc in the CI step but not added to requirements.txt or a requirements-dev.txt, so local test runs can silently use different/newer httpx than CI, and the dependency isn't discoverable from the repo's normal dependency manifest.
This PR fixes a real production bug: schools with no performance rows return NaN through the LEFT JOIN, which crashes JSON serialization on the /api/schools/{urn} school_info dict (yearly_data was already protected via clean_for_json, but school_info wasn't). The fix wraps school_info values with the existing convert_to_native helper, and the PR adds solid backend unit test and e2e coverage plus a CI step to run pytest. Overall a clean, well-tested, low-risk fix.
🟡 Minor
backend/app.py: convert_to_native() nulls out values equal to the exam-suppression sentinels "SUPP"/"NE"/"NA"/"NP". It's now applied to descriptive GIAS metadata fields (school_type, gender, religious_denomination, age_range, trust_name) in addition to numeric performance data. If any of these ever legitimately hold one of those literal strings, the value would be silently nulled instead of just NaN/inf being converted.
.gitea/workflows/pr-checks.yml: pytest and "httpx<0.28" are installed ad-hoc in the CI step rather than being tracked in a requirements/requirements-dev file, so a developer running the new backend/tests locally won't get the right httpx version (needed for FastAPI 0.109's TestClient) unless they read the workflow file.
## 🤖 AI Code Review (Claude Code)
This PR fixes a real production bug: schools with no performance rows return NaN through the LEFT JOIN, which crashes JSON serialization on the `/api/schools/{urn}` school_info dict (yearly_data was already protected via clean_for_json, but school_info wasn't). The fix wraps school_info values with the existing convert_to_native helper, and the PR adds solid backend unit test and e2e coverage plus a CI step to run pytest. Overall a clean, well-tested, low-risk fix.
### 🟡 Minor
- **backend/app.py**: convert_to_native() nulls out values equal to the exam-suppression sentinels "SUPP"/"NE"/"NA"/"NP". It's now applied to descriptive GIAS metadata fields (school_type, gender, religious_denomination, age_range, trust_name) in addition to numeric performance data. If any of these ever legitimately hold one of those literal strings, the value would be silently nulled instead of just NaN/inf being converted.
- **.gitea/workflows/pr-checks.yml**: pytest and "httpx<0.28" are installed ad-hoc in the CI step rather than being tracked in a requirements/requirements-dev file, so a developer running the new backend/tests locally won't get the right httpx version (needed for FastAPI 0.109's TestClient) unless they read the workflow file.
tudor
merged commit 85484a80c4 into main2026-07-07 08:56:23 +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.
Schools without KS2/KS4 results (special post-16 institutions, sixth-form centres, PRUs, brand-new schools) 500 on
GET /api/schools/{urn}: the marts LEFT JOIN yields NaN in every numeric column,school_infopassed those raw pandas values intoJSONResponse(which renders withallow_nan=False), and serialization raisedValueError. The frontend collapses the failed fetch intonotFound(), so every such school — e.g./school/150275-west-london-performing-arts-academy-wlpa-academy— is a dead 404 SEO landing page. All nine year-null schools sampled on prod reproduce it.Fix: pass every
school_infovalue through the existingconvert_to_native(), the same treatmentyearly_dataalready gets fromclean_for_json().Tests:
backend/tests/test_school_details.py(TestClient + monkeypatched loader) reproducing the 500 — red before the fix, green after; PR checks now runpytest backend/tests(withhttpx<0.28for the pinned Starlette).Follow-up candidate (not in this PR): the school page maps any fetch failure to
notFound(), which is how a 500 got misfiled as a routing 404 — worth distinguishing 5xx from real 404s.🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This PR fixes a real bug where schools with no performance rows (post-16 institutions, PRUs, new schools) return NaN in GIAS/location fields, causing JSONResponse to raise ValueError and 500 the /api/schools/{urn} endpoint; it now runs every school_info field through convert_to_native, matching the treatment yearly_data already gets via clean_for_json. Backend pytest and a corresponding Playwright e2e regression test are added and wired into CI. The change is small, well-tested, and low risk — no severe issues found.
🟡 Minor
🤖 AI Code Review (Claude Code)
This PR fixes a real production bug: schools with no performance rows return NaN through the LEFT JOIN, which crashes JSON serialization on the
/api/schools/{urn}school_info dict (yearly_data was already protected via clean_for_json, but school_info wasn't). The fix wraps school_info values with the existing convert_to_native helper, and the PR adds solid backend unit test and e2e coverage plus a CI step to run pytest. Overall a clean, well-tested, low-risk fix.🟡 Minor