Skip to content

Add UK Chronicle targets for issue 196 - #198

Merged
juaristi22 merged 3 commits into
mainfrom
chronicle-196-uk-targets
Aug 25, 2026
Merged

Add UK Chronicle targets for issue 196#198
juaristi22 merged 3 commits into
mainfrom
chronicle-196-uk-targets

Conversation

@juaristi22

Copy link
Copy Markdown
Collaborator

Fixes #196

Summary

Verification

  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run --with ruff ruff format --check chronicle/source_package.py chronicle/bundle.py policyengine_chronicle/consumer.py tests/test_chronicle_consumer.py tests/test_chronicle_source_package.py
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run chronicle validate-package hmrc-cgt-size-of-gain-2025 --year 2025
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run chronicle validate-package ons-households-by-type-country-2025 --year 2025
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run pytest tests/test_chronicle_source_package.py -k "cgt_size_of_gain or households_by_type_country"
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run pytest tests/test_chronicle_consumer.py -k "dimensions_selector or dimension_value"
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run pytest tests/test_source_package_alias_drift.py
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run chronicle build-bundle --source hmrc-cgt-size-of-gain-2025 --source ons-households-by-type-country-2025 --out /tmp/chronicle-196-bundle --replace
  • UV_CACHE_DIR=/Users/mariajuaristi/Documents/Codex/2026-08-24/can-x20/work/.uv-cache uv run chronicle build-consumer-artifact --facts /tmp/chronicle-196-bundle --out /tmp/chronicle-196-artifact --replace

@juaristi22
juaristi22 marked this pull request as ready for review August 25, 2026 07:50
@vahid-ahmadi

Copy link
Copy Markdown

Automated review pass (Claude Code, high effort, diff only — no execution). Three findings, one of which binds a fact to the wrong series.

1. packages/ons/households_by_type_country_2025/source_package.yaml:510source_concept points at the wrong package

source_concept: ons.families_households_table7 looks copy-pasted from the separate ons-families-households-2025 package (UK-wide Table 7 of Families and households), while every other field on this measure points at the regions/countries workbook.

That matters because consumer selection resolves source_concept, and concept falls back to source_concept when there is no canonical alignment (policyengine_chronicle/consumer.py:578-582). So this single Scotland fact is silently folded into the UK Table 7 series: a Microcosm selector on ons.families_households_table7 picks up an extra Scotland-only row, while the selector documented for this package (ons.households_by_type, docs line 146) resolves only via concept and never reaches it. Nothing fails loudly — the fact just binds to the wrong series, which downstream reads as a UK figure.

2. policyengine_chronicle/consumer.py:555-560 — the new dimensions Mapping branch matches subsets, not sets

The branch does subset matching (all(dimensions.get(name) == expected ...)), whereas every other dimension selector in this function is exact-set identity. A selector like {"cgt_gain_band": "gain_12300_to_24999"} therefore also matches any row dimensioned by that band and a further dimension, and an empty {} matches every row in the candidate set. The extra rows aren't rejected — they flow into the period/assertion choice downstream, so the selector silently resolves against a larger candidate set than it names.

Related: a mis-typed dimension name yields zero matches rather than an error. That is the same silent-dead-target failure mode as the microcosm children_count batch — 8 references bound to a non-existent variable and simply never compiled. Given this layer feeds those targets, failing loudly on an unknown dimension name seems worth the strictness.

3. policyengine_chronicle/consumer.py:561-565 — duplicated list branch, and a now-dead _selector_value case

The list branch inside the new dimensions block duplicates the pre-existing list handling in _selector_matches — same code, same comment verbatim — and that path is fed by _selector_value's if key == "dimensions": return sorted(...) at 590-591, which is now unreachable for the only key that used it. The diff leaves a dead branch plus a duplicated comparison that has to be kept in sync with its twin.


Checked and clean, given the recent history in this lane: the CGT band edges are contiguous and gap-free from £12,300 up to the open-ended £5,000,000+ top band, with no overlaps; both manifests' source_url, filename, R2 key and sha256 agree with each other, so no repeat of the #188 doubled-path defect; no duplicate package or record_set_spec_id values; and the remainder of the consumer.py diff is line-reflow.

@juaristi22

Copy link
Copy Markdown
Collaborator Author

Addressed Vahid review in 329b757.

Fixed:

  • Gave the ONS country household package its own source-specific source_concept: ons.households_by_type_regions_countries_table7.
  • Updated the ONS package smoke test and UK checklist so Microcosm consumers can select the Scotland row by source_concept without folding it into the UK Table 7 series.
  • Made dimensions mapping selectors exact row matches, rejected empty mappings, and added explicit errors for unknown mapped dimension names.
  • Removed the duplicate dimensions-list selector branch while preserving list selectors as exact dimension-name-set matches.

Verification:

  • uv run --with ruff ruff format ...
  • uv run --with ruff ruff check ...
  • uv run --locked pytest -q tests/test_chronicle_consumer.py
  • focused ONS/consumer review regression tests
  • uv run --locked pytest -q tests/test_chronicle_bundle.py::test_build_bundle_writes_merged_consumer_contract
  • Both GitHub Chronicle checks passed.

@juaristi22
juaristi22 merged commit 33ca98a into main Aug 25, 2026
2 checks passed
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.

UK national gaps: CGT gain bands, housing cost aggregates, Scotland household composition, salary-sacrifice contributions, benefit cap reduction

2 participants