Reform-validation population: country dimension (per-release regression history for policyengine-uk)

[vahid-ahmadi]

Parameterises the reform-validation ingest by country instead of forking it, per #62.

What changed

• COUNTRIES config map in ingest_reform_validation.py: raw dir, run-id prefix, registry mark, lane name, claim currency, and engine-pin map per country. ingest(..., country="US") is the default.
• US byte-stability, verified: built the DB from origin/main's module and from this branch and diffed — 241 claims identical modulo the new publication.country: "US" field (claim ids hash conditions, not publication, so no id moves), 675 pe_results byte-identical.
• UK lane declared, not invented: COUNTRIES["UK"] carries raw_uk/, the populace-uk-rv- run prefix, a distinct registry mark, GBP claims — and a deliberately empty release map. The first reform_validation.json a managed populace-uk run produces adds its release_manifest.json pin; until then a UK ingest fails loudly at "no artifacts". No fake release ids or artifacts anywhere in the module.
• Country-neutral artifact contract documented in the module docstring + scorecard_db/README.md: same JSON shape; non-US artifacts use categories Reform (scored reform delta) / Program actual (benefit-cost level) with a required explicit jct.publisher — the US category→publisher inference is US vocabulary and never runs for them. UnitConcept.GBP added.
• Disjoint replacement scopes: run prefixes are non-overlapping by test, so one country's wholesale replace can never delete another country's results; lane rows are per country.

Tests

New: US rows carry country US with the run prefix unchanged; unknown country raises; UK/US replacement scopes disjoint; a synthetic UK fixture (fixture-only release id, engine pin injected via monkeypatch — never the module map) ingests with country UK, GBP units, and the UK run prefix; a neutral-category row without jct.publisher raises. Suite: 168 passed, 4 skipped; ruff format --check clean.

Waits on the first UK managed run

The UK engine-pin map, the actual raw_uk/ artifacts, and the app's per-release history view for UK (that's #42/#50's country dimension) all populate after the first managed populace-uk reform_validation.json exists.

Builds #62.

🤖 Generated with Claude Code

[DTrim99]

Review — Reform-validation population: country dimension

Clean parameterize-don't-fork refactor, and the key risk (silently changing US output) is provably avoided: ExternalScore.claim_id() (models.py:316-333) hashes only source/reform/metric/unit/period/basis/conditions — publication is excluded, so the additive publication["country"] can't move any US claim id, and the new Reform/neutral-publisher branches are dead for US (US raw categories use none of them). US rows still emit literal UnitConcept.USD / REGISTRY_MARK, so the new currency/registry_mark params don't touch them. The UK lane is correctly declared-not-invented (engine_versions == {}, empty raw_uk/ → loud SystemExit, missing pin → loud SystemExit), and that's tested.

Should address

• _obbba_results is not parameterized (ingest_reform_validation.py:610, 620, 640) — it hardcodes the module-global REGISTRY_MARK and run_id=f"{RUN_PREFIX}…". It's reachable from ingest(country=…) but fires only for cat=="OBBBA" (US-only), so it's latent, not an active bug. Risk: a future non-US OBBBA-shaped row would get the US prefix/mark and could then be swept up by the US wholesale-delete — the exact cross-country deletion the disjoint-prefix invariant exists to prevent. Parameterize it or assert country=="US" at entry.
• US byte-stability lock is partial — test_us_rows_carry_country_and_nothing_else_changed asserts the country stamp + run-id prefix but doesn't pin the claim-id set/count against a pre-PR snapshot, so the "241 claims / 675 results identical" result rests on manual verification. A claim-id-count regression assert would lock it in CI.

Suggestions

• The loud-failure message (:667) still literally says ENGINE_VERSIONS (the US-named global) in the country-generalized path — reword to the per-country engine_versions map for a UK operator.
• Cross-PR: this PR and UK externals → DB ingest path (16,924 claims, five sources) #48 both add UnitConcept.GBP to models.py — they'll conflict; worth deconflicting before the second one merges.

Reviewed with Claude Code assistance.

[vahid-ahmadi]

All three items addressed in f854739:

• _obbba_results parameterized — takes country/run_prefix/registry_mark, and refuses country != "US" at entry with the cross-country-deletion rationale in the error. While in there: _map_row had one remaining global REGISTRY_MARK use (line 479) despite already taking the parameter — same latent class, fixed too. Both paths tested (test_obbba_path_refuses_non_us).
• US byte-stability locked in CI — test_us_claim_id_set_is_byte_stable pins the full 241-claim id set as a sha256 digest. The pinned digest was computed from a fresh origin/main build and re-verified identical on this branch (288326d7… both sides), so the manual "241/675 identical" verification is now a CI assert.
• Loud-failure wording — the missing-pin SystemExit now names the per-country engine_versions map in COUNTRIES.

On the cross-PR UnitConcept.GBP conflict with #48: agreed — whichever merges second rebases; both add the identical enum member so it's a trivial dedup, noted on #57's merge-order thread.

Suite: 170 passed / 4 skipped; ruff clean.

[DTrim99]

Re-review — all addressed ✅

• _obbba_results now takes country/run_prefix/registry_mark and raises ValueError on non-US before minting (the stronger guard) — test_obbba_path_refuses_non_us.
• US byte-stability now locked by test_us_claim_id_set_is_byte_stable (pins len==241 and a sha256 of the sorted claim-id set against a pre-refactor build) — stronger than the count-only assert I asked for.
• Loud-failure message reworded to the per-country engine_versions map (no longer names the US global).

One cross-PR item: this PR and #48 both add GBP = "gbp" to models.py, so whichever merges second will hit a textual conflict on that line — trivial to resolve. Otherwise looks good.

[DTrim99]

Everything in my review is addressed — the one thing holding a full approval is cross-PR: this and #48 both add GBP = "gbp" to models.py UnitConcept, so they'll textually conflict on that line. Sequence the merge (or drop the duplicate line from whichever lands second) and I'll approve.

[vahid-ahmadi]

Sequencing committed: #48 merges first (it anchors the #52 stack), and this PR rebases right after, dropping its duplicate GBP = "gbp" line from models.py — a one-line resolution I'll handle the same day #48 lands. If #48 stalls on the #57 adjudication, the reverse order works identically (then #48 drops the line); either way the second-to-merge sheds the duplicate.

[vahid-ahmadi]

Rebased onto main as committed: the duplicate GBP = "gbp" line is dropped (this branch landed second, #48's stays), and the rebase reconciles #71's executed-baseline provenance with the country config — ENGINE_VERSIONS now lives in COUNTRIES["US"] (back-compat alias kept) and _obbba_results carries both #71's position argument and the country parameters. Suite 248 passed / 6 skipped, format clean. Ready for the approval.

[MaxGhenis]

Gate round 1 — honest about the missing artifact; six findings before the #79 artifact can activate this

The absent-artifact handling is genuinely honest (empty pin map, loud failures on direct ingest) — the findings are about the wiring around it:

1. High — absent from the post-The database leaves git: derived artifact, built in CI, published to Supabase storage #74 build path. build_db.py invokes only the default US reform-validation ingest; this PR never modifies it, so issue UK compute campaign, run 4: produce the first populace-uk reform_validation.json (activates the #65 UK lane) #79's future artifact would be silently ignored by deterministic builds — indistinguishable from "awaiting artifact." The UK step needs registering after uk_externals/uk_deductions.

2. High — UK rows export as US. Country is stamped into publication, but the exporter reads conditions.get("country", "US") — the rows would be selected and then filed under US, and the country-keyed app view would filter them out of the UK page. Country belongs in conditions (the claim-side convention the whole country dimension rides on).

3. High — executed-baseline provenance hard-coded. Every neutral UK result gets _CURRENT_LAW_KEY; only the US OBBBA path derives real worlds. No UK artifact field declares the executed baseline, so a stacked or non-current-law run would be silently mis-stamped rather than keyed distinctly or rejected.

4. High — UK FY identity fail-open. A bare 2026 period in the fixture becomes an annual-2026 claim with status=comparable. UK claims must require the explicit FY label, key the END year (FY2026-27 → 2027), and record/assert the engine-period offset as result provenance — the UK externals → DB ingest path (16,924 claims, five sources) #48/UC deductions: FRR family re-harvested from primary sources + DB ingest #52 convention, now load-bearing across three merged ingests.

5. High — no attachment path to canonical UK claims. Neutral rows always mint new claims, with fixture slugs hmrc/dwp instead of canonical uk_hmrc/dwp_takeup and none of the closed identity vocabulary — they'd create parallel identities beside the 15,858 ingested claims instead of attaching. Existing-claim resolution (the campaign's exactly-one-match or claim_id-direct forms) is the pattern.

6. Medium — the UK lane never reaches mission control: ingest writes it to SQLite but the exporter's lane sync lists only the US lane, so both tracked feeds omit it while the no-drift gates stay green.

🤖 Generated with Claude Code

[vahid-ahmadi]

All six addressed in cf0d656 (rebased onto post-#74 main).

1. Build path. Registered in build_db after uk_deductions. Since UK compute campaign, run 4: produce the first populace-uk reform_validation.json (activates the #65 UK lane) #79's artifact does not exist yet, the step writes the lane as an explicit "registered / declared empty rather than absent" row — your point exactly: a build that never invoked the UK ingest was indistinguishable from one where a future artifact existed and was silently ignored. When the artifact lands the same call ingests it with no further wiring.

2. Country. Moved into conditions. You were right that stamping it into publication only meant the rows were selected and then filed under US. US claims stay country-less, so no existing US claim id moves — asserted directly in a test.

3. Executed baseline. A UK artifact must now declare executed_baseline, the value must be a world baselines.py describes, and an unregistered one raises. No more hard-coded _CURRENT_LAW_KEY.

4. FY identity. UK rows require an explicit FY label, key the END year (2026-27 → 2027), and each result records the engine-period offset as provenance (PE-UK keys the FY START year), asserted rather than assumed. A bare "2026" now raises instead of minting an annual-2026 comparable claim.

5. Attachment. UK_PUBLISHERS maps the artifact's hmrc/dwp shorthand to canonical uk_hmrc/dwp_takeup and raises on anything unregistered; neutral rows resolve against the ingested catalog first — exactly-one-match attaches, nothing matching mints, two matches raise. Attached-vs-minted is tallied in the summary.

6. Mission control. Both the exporter's lane map and the ingest's own feed sync list the UK lane now, filed under UK, using the UK family's shared FEED_UPDATED literal so data/lanes.json cannot drift with ingest order.

Suite 267 passed, two builds agree on content_hash, no-drift clean (committed lanes.json regenerated and mirrored to app/public), ruff format clean.

[DTrim99]

Re-reviewed — the one thing I was holding on is resolved: the #48↔#65 GBP conflict is gone. #48 landed GBP = "gbp" in models.py, and this branch was rebased to drop its duplicate, so there's exactly one enum member and #65 no longer touches models.py.

All three of my earlier fixes still hold: _obbba_results is country-guarded (raises on non-US), US byte-stability is locked by the 241-claim sha256 test, and the loud-failure message names the per-country engine_versions map. The new UK country-dimension code is sound (neutral categories require a publisher, UK results resolve to the canonical claim, US claims stay country-less so their ids don't move), and the 27-case country-dimension suite passes.

Note: test_integration_committed_db fails, but it's a pre-existing stale committed-DB artifact that fails identically on main — not a regression from this PR. Approving.

[vahid-ahmadi]

Thanks for the re-review. One correction on the note, because it's the second time this artifact has been read as a main defect and it'll keep costing people time.

test_integration_committed_db does not fail on main. I checked out clean main, deleted data/scorecard.db, rebuilt it, and ran the suite: 262 passed, 0 failed.

What actually happens is a stale-artifact trap introduced by #74. data/scorecard.db is gitignored now, so it is not removed by a branch switch — it survives, and the suite then runs against a database whose claims don't match the working tree. That produces exactly the symptom you saw, and it is reproducible on demand:

# build on branch A, then:
git checkout main
pytest tests/ -q      # fails in the campaign-producer / integration tests
rm data/scorecard.db
python -m scorecard_db.build_db data/scorecard.db
pytest tests/ -q      # passes

So: rm data/scorecard.db after any checkout before running the suite. It bit me twice in this session before I traced it, and I flagged the same thing on #91 where it surfaced as a phantom BE: 2 mismatch.

It might be worth a CI-adjacent guard or a line in the README — the failure is confusing precisely because it looks like a real regression in whatever branch you happen to be on. Happy to add one if you think it's worth it.

Everything else in your re-review matches what I have: the GBP conflict is gone, _obbba_results is country-guarded, US byte-stability is locked, and the country-dimension suite passes.