Follow-up to #24, which merged before this commit landed on the branch. The backend now detects the undefined _code column on a pre-migration DB and retries with a legacy name-column query, so the API works against both mart schemas instead of serving empty data until school_data_daily runs. 21/21 backend tests (1 new, TDD).
Note: until this merges AND/OR the daily DAG runs, the deployed backend is in the empty-API window — trigger school_data_daily now if not already done.
Follow-up to #24, which merged before this commit landed on the branch. The backend now detects the undefined `_code` column on a pre-migration DB and retries with a legacy name-column query, so the API works against both mart schemas instead of serving empty data until `school_data_daily` runs. 21/21 backend tests (1 new, TDD).
Note: until this merges AND/OR the daily DAG runs, the deployed backend is in the empty-API window — trigger `school_data_daily` now if not already done.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Closes the deploy window flagged by CI review — the backend now works
against both the old (name) and new (code) mart schemas.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR adds a fallback query path so load_school_data_as_dataframe can still load data from marts.dim_school when the mart predates the new GIAS code-dictionary migration (i.e. still has phase/school_type/etc. name columns instead of *_code columns), plus a unit test for the new retry path. The intent and column-mapping (including the religious_character_code → religious_denomination rename matching _GIAS_CODE_COLUMNS in the same file) are correct, but the error-detection logic used to pick the fallback branch is unreliable in a way that can regress an existing, previously-working fallback.
🔴 Severe (blocks merge)
backend/data_loader.py: The new branch at line ~300 (if any(col in str(exc) for col in _GIAS_CODE_COLUMN_NAMES)) checks whether column names appear anywhere in str(exc). SQLAlchemy's DBAPIError/ProgrammingError.__str__ includes the full offending SQL statement (the [SQL: ...] section), and _MAIN_QUERY itself contains the literal substrings phase_code, school_type_code, religious_character_code, status_code, and admissions_policy_code in its SELECT list. That means this check is true for essentially any ProgrammingError raised while executing _MAIN_QUERY — not just ones where those columns are actually missing. In particular, it will also match when the real problem is a missing has_sixth_form column (the pre-existing, previously-working fallback via _MAIN_QUERY_NO_SIXTH_FORM), since has_sixth_form and the code-column names all co-occur in the same statement text. The result: on a mart that's missing only has_sixth_form, the code takes the wrong branch, retries with _MAIN_QUERY_LEGACY_NAMES (which still references s.has_sixth_form, unchanged), that retry fails for the same reason, and the code falls into the generic except Exception as exc2 handler and returns an empty DataFrame — silently serving zero schools instead of degrading gracefully via _MAIN_QUERY_NO_SIXTH_FORM as it did before this PR. This is exactly the transitional-migration-lag scenario this fallback machinery exists to handle, so it's a real production-availability regression. The added test in test_gias_translation.py doesn't catch this because it constructs a fake ProgrammingError whose str() doesn't include the real query text, so it can't reproduce the substring collision that occurs with real SQLAlchemy exceptions.
## 🤖 AI Code Review (Claude Code)
This PR adds a fallback query path so `load_school_data_as_dataframe` can still load data from `marts.dim_school` when the mart predates the new GIAS code-dictionary migration (i.e. still has `phase`/`school_type`/etc. name columns instead of `*_code` columns), plus a unit test for the new retry path. The intent and column-mapping (including the `religious_character_code` → `religious_denomination` rename matching `_GIAS_CODE_COLUMNS` in the same file) are correct, but the error-detection logic used to pick the fallback branch is unreliable in a way that can regress an existing, previously-working fallback.
### 🔴 Severe (blocks merge)
- **backend/data_loader.py**: The new branch at line ~300 (`if any(col in str(exc) for col in _GIAS_CODE_COLUMN_NAMES)`) checks whether column names appear anywhere in `str(exc)`. SQLAlchemy's DBAPIError/`ProgrammingError.__str__` includes the full offending SQL statement (the `[SQL: ...]` section), and `_MAIN_QUERY` itself contains the literal substrings `phase_code`, `school_type_code`, `religious_character_code`, `status_code`, and `admissions_policy_code` in its SELECT list. That means this check is true for essentially *any* ProgrammingError raised while executing `_MAIN_QUERY` — not just ones where those columns are actually missing. In particular, it will also match when the real problem is a missing `has_sixth_form` column (the pre-existing, previously-working fallback via `_MAIN_QUERY_NO_SIXTH_FORM`), since `has_sixth_form` and the code-column names all co-occur in the same statement text. The result: on a mart that's missing only `has_sixth_form`, the code takes the wrong branch, retries with `_MAIN_QUERY_LEGACY_NAMES` (which still references `s.has_sixth_form,` unchanged), that retry fails for the same reason, and the code falls into the generic `except Exception as exc2` handler and returns an empty DataFrame — silently serving zero schools instead of degrading gracefully via `_MAIN_QUERY_NO_SIXTH_FORM` as it did before this PR. This is exactly the transitional-migration-lag scenario this fallback machinery exists to handle, so it's a real production-availability regression. The added test in test_gias_translation.py doesn't catch this because it constructs a fake `ProgrammingError` whose `str()` doesn't include the real query text, so it can't reproduce the substring collision that occurs with real SQLAlchemy exceptions.
str(ProgrammingError) embeds the full SQL, which contains every column
name — the substring check matched any error and could take the wrong
retry branch. Parse the missing column from exc.orig instead.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This PR fixes a real bug in load_school_data_as_dataframe's fallback-matching logic: previously it checked whether "has_sixth_form" appeared in str(exc), which always matched because the full SQL statement (embedding every column name) is included in that string, so the fallback branch fired regardless of the actual missing column. The new _missing_column_name() helper parses the underlying DBAPI error (exc.orig) with a regex to extract the real offending column, and adds a second fallback path for marts that predate the GIAS code-dictionary migration, aliasing legacy name columns to match the API contract (verified consistent with the _GIAS_CODE_COLUMNS mapping). The change is well covered by new unit tests, and the fix itself is a net correctness improvement.
🟡 Minor
backend/data_loader.py: The two fallback branches don't compose: if a mart is stale enough to be missing both a GIAS *_code column and has_sixth_form simultaneously, the code-column branch retries with _MAIN_QUERY_LEGACY_NAMES, which still references s.has_sixth_form. That retry will raise again, and the generic except Exception as exc2 handler just returns an empty DataFrame instead of falling further back to a combined legacy+no-sixth-form query, so the whole school dataset would silently disappear from the app in that scenario rather than degrading gracefully.
## 🤖 AI Code Review (Claude Code)
This PR fixes a real bug in load_school_data_as_dataframe's fallback-matching logic: previously it checked whether "has_sixth_form" appeared in str(exc), which always matched because the full SQL statement (embedding every column name) is included in that string, so the fallback branch fired regardless of the actual missing column. The new _missing_column_name() helper parses the underlying DBAPI error (exc.orig) with a regex to extract the real offending column, and adds a second fallback path for marts that predate the GIAS code-dictionary migration, aliasing legacy name columns to match the API contract (verified consistent with the _GIAS_CODE_COLUMNS mapping). The change is well covered by new unit tests, and the fix itself is a net correctness improvement.
### 🟡 Minor
- **backend/data_loader.py**: The two fallback branches don't compose: if a mart is stale enough to be missing both a GIAS *_code column and has_sixth_form simultaneously, the code-column branch retries with _MAIN_QUERY_LEGACY_NAMES, which still references s.has_sixth_form. That retry will raise again, and the generic `except Exception as exc2` handler just returns an empty DataFrame instead of falling further back to a combined legacy+no-sixth-form query, so the whole school dataset would silently disappear from the app in that scenario rather than degrading gracefully.
tudor
merged commit d9223a6d6e into main2026-07-09 19:13:55 +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.
Follow-up to #24, which merged before this commit landed on the branch. The backend now detects the undefined
_codecolumn on a pre-migration DB and retries with a legacy name-column query, so the API works against both mart schemas instead of serving empty data untilschool_data_dailyruns. 21/21 backend tests (1 new, TDD).Note: until this merges AND/OR the daily DAG runs, the deployed backend is in the empty-API window — trigger
school_data_dailynow if not already done.🤖 Generated with Claude Code
🤖 AI Code Review (Claude Code)
This PR adds a fallback query path so
load_school_data_as_dataframecan still load data frommarts.dim_schoolwhen the mart predates the new GIAS code-dictionary migration (i.e. still hasphase/school_type/etc. name columns instead of*_codecolumns), plus a unit test for the new retry path. The intent and column-mapping (including thereligious_character_code→religious_denominationrename matching_GIAS_CODE_COLUMNSin the same file) are correct, but the error-detection logic used to pick the fallback branch is unreliable in a way that can regress an existing, previously-working fallback.🔴 Severe (blocks merge)
if any(col in str(exc) for col in _GIAS_CODE_COLUMN_NAMES)) checks whether column names appear anywhere instr(exc). SQLAlchemy's DBAPIError/ProgrammingError.__str__includes the full offending SQL statement (the[SQL: ...]section), and_MAIN_QUERYitself contains the literal substringsphase_code,school_type_code,religious_character_code,status_code, andadmissions_policy_codein its SELECT list. That means this check is true for essentially any ProgrammingError raised while executing_MAIN_QUERY— not just ones where those columns are actually missing. In particular, it will also match when the real problem is a missinghas_sixth_formcolumn (the pre-existing, previously-working fallback via_MAIN_QUERY_NO_SIXTH_FORM), sincehas_sixth_formand the code-column names all co-occur in the same statement text. The result: on a mart that's missing onlyhas_sixth_form, the code takes the wrong branch, retries with_MAIN_QUERY_LEGACY_NAMES(which still referencess.has_sixth_form,unchanged), that retry fails for the same reason, and the code falls into the genericexcept Exception as exc2handler and returns an empty DataFrame — silently serving zero schools instead of degrading gracefully via_MAIN_QUERY_NO_SIXTH_FORMas it did before this PR. This is exactly the transitional-migration-lag scenario this fallback machinery exists to handle, so it's a real production-availability regression. The added test in test_gias_translation.py doesn't catch this because it constructs a fakeProgrammingErrorwhosestr()doesn't include the real query text, so it can't reproduce the substring collision that occurs with real SQLAlchemy exceptions.🤖 AI Code Review (Claude Code)
This PR fixes a real bug in load_school_data_as_dataframe's fallback-matching logic: previously it checked whether "has_sixth_form" appeared in str(exc), which always matched because the full SQL statement (embedding every column name) is included in that string, so the fallback branch fired regardless of the actual missing column. The new _missing_column_name() helper parses the underlying DBAPI error (exc.orig) with a regex to extract the real offending column, and adds a second fallback path for marts that predate the GIAS code-dictionary migration, aliasing legacy name columns to match the API contract (verified consistent with the _GIAS_CODE_COLUMNS mapping). The change is well covered by new unit tests, and the fix itself is a net correctness improvement.
🟡 Minor
except Exception as exc2handler just returns an empty DataFrame instead of falling further back to a combined legacy+no-sixth-form query, so the whole school dataset would silently disappear from the app in that scenario rather than degrading gracefully.