Skip to content

Fix validation of saved multi-year datasets - #2420

Merged
MaxGhenis merged 1 commit into
mainfrom
fix/2416-multiyear-validate
Oct 15, 2026
Merged

MaxGhenis merged 1 commit into
mainfrom
fix/2416-multiyear-validate

Conversation

@MaxGhenis

Copy link
Copy Markdown
Collaborator

Fixes #2416

Files saved by UKMultiYearDataset.save were rejected by the multi-year validator, then accepted as single-year tables, where loading raised TypeError. In policyengine_uk/data/dataset_schema.py:35, the single-year validator now requires pandas table metadata. In policyengine_uk/data/dataset_schema.py:318, the multi-year validator walks each entity group's year-named pandas children instead of looking for slashes in top-level h5py keys.

Invariants: saved single-year and multi-year layouts pass exactly one validator, including a multi-year layout containing only one year. Existing error text and raise_exception defaults are preserved. Ordinary single-year loading is retained. The formerly stubbed local-file test now exercises save, validation, dispatch, and calculated inputs; Hypothesis checks random year sets for both layouts.

Ownership: Budget W1 owns simulation.py; this fix leaves it untouched and limits production changes to the schema validators.

Reproducer: policyengine_uk/tests/test_simulation_dataset_sources.py::test_build_from_file_loads_local_multi_year_dataset

Validation:

  • Both new regression nodes fail against unchanged main for the reported validator bug.
  • uv run --frozen --no-sync pytest policyengine_uk/tests/test_simulation_dataset_sources.py policyengine_uk/tests/test_dataset_fiscal_year.py -n 4 -q — 16 passed.
  • uv run --frozen --no-sync make format and uvx ruff format --check . pass.
  • Default MOCK differential against main is byte-identical across six loading routes and 2025–2032: 336 calculation arrays, aggregates, and exported inputs. Both JSON files have SHA256 26b2f908d2e0f472b1703da75a8310b210951d4a803bd6839a6a53bba95857ff. The script and JSON evidence are retained in .job/ for the hub handoff.

No full suite or real-data microsimulation: this is schema dispatch and the MOCK default differential is unchanged. Documentation build omitted for the small prose addition to the existing dataset instructions.

axiom: n/a: H5 dataset schema validation infrastructure

Independent review is left to the hub. Do not merge as part of this job.

@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

Handoff for independent hub review.

Head: fd75057e82361f9e67c7728fac03e766d9029eb2 on fix/2416-multiyear-validate (one push).

Tests: uv run --frozen --no-sync pytest policyengine_uk/tests/test_simulation_dataset_sources.py policyengine_uk/tests/test_dataset_fiscal_year.py -n 4 -q — 16 passed. Both new regression nodes fail on main for the reported validator defect. make format and uvx ruff format --check . pass in the frozen dev environment.

Impact: two schema validators only; Budget W1's simulation.py is untouched. Default MOCK differential is byte-identical to main for six routes and 2025–2032, including 336 calculated arrays, aggregates and exported inputs; JSON SHA256 26b2f908d2e0f472b1703da75a8310b210951d4a803bd6839a6a53bba95857ff.

The hub handoff in .job/HANDOFF.md retains the differential script and logs. No real-data microsimulation or full local suite was run. Independent review and CI remain for the hub; no self-review or merge was performed. The separate #2417 fix follows on its own branch from main.

@MaxGhenis
MaxGhenis marked this pull request as ready for review October 15, 2026 06:49
@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

UK hub merge audit, head fd75057e82361f9e67c7728fac03e766d9029eb2.

  • Independent review: APPROVE WITH NITS at this exact head, executed (GPT-6.1 Sol, -s workspace-write, job 20261015-022817-uk2420-review).
    • Tests: 16 pass at the head (11 dataset-source, 5 fiscal-year). With only the production hunk reverted, 2 fail for the intended validator defect, with no missing-name failures.
    • Independent reproduction: on main, UKMultiYearDataset.save followed by Simulation(dataset=path) raises the reported TypeError; at this head it loads.
    • 44 randomized saved files (36 single-year; multi-year sizes 1–8 with gaps) each pass exactly one validator and load through Simulation.
    • Error contract: 24 main-vs-head comparisons of messages and raise_exception behaviour are unchanged.
    • Default MOCK differential: byte-identical (six routes, 2025–2032; SHA256 26b2f908…).
    • Callers: only the two schema constructors and build_from_file. The UKMultiYearDataset fails when downloading from HuggingFace #1256 read-only-cache fix is intact.
  • Nit (P3, follow-up): malformed multi-year files, such as one missing an entity's year or carrying an extra non-year table, pass the new any(...) check and then fail at load. Main fails on these too, so this is not a regression. Queued as validator hardening.
  • Order: this lands before Keep frozen simulation datasets aligned with loaded inputs #2421. Both touch test_simulation_dataset_sources.py, and the reviewer verified the resolution: keep this PR's real round-trip test and Keep frozen simulation datasets aligned with loaded inputs #2421's GCS-stub assignment.
  • Checks: all 15 pass, MERGEABLE CLEAN, marked ready. Enqueued in the merge queue pinned to this head.

@MaxGhenis
MaxGhenis added this pull request to the merge queue Oct 15, 2026
Merged via the queue into main with commit a726b74 Oct 15, 2026
16 checks passed
@MaxGhenis
MaxGhenis deleted the fix/2416-multiyear-validate branch October 15, 2026 07:26
@MaxGhenis

Copy link
Copy Markdown
Collaborator Author

Merged by the merge queue at reviewed head fd75057e8. The audit is above. #2416 is fixed; #2421 lands next.

MaxGhenis added a commit that referenced this pull request Oct 15, 2026
…ards

Brings in #2277, #2391, #2402, #2404, #2405, #2407, #2411, #2413, #2415,
#2418, #2419 and #2420. No conflicts.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis added a commit that referenced this pull request Oct 15, 2026
…2277) into uc-hb-split-shared-cap-lha-rules

No conflicts; no benefit cap, LHA, UC or HB file changed on main.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis

This comment has been minimized.

4 similar comments
@MaxGhenis

This comment has been minimized.

@MaxGhenis

This comment has been minimized.

@MaxGhenis

This comment has been minimized.

@MaxGhenis

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simulation(dataset=path) fails for files written by UKMultiYearDataset.save: the multi-year check never passes

1 participant