UK externals → DB ingest path (16,924 claims, five sources)

[vahid-ahmadi]

Part of #33 — the UK ingest path itself. (Not "Closes": the campaign-results attachment half stays open, for the reason documented below.)

What's here

• scorecard_db/ingest_uk_externals.py — the UK analogue of the harvest adapters: maps the five UK adapter outputs (data/externals/, from DWP take-up adapter (mode 1): FYE 2024 external side #43–UKMOD statistics adapter (mode 1): Country Report validation tables #47) onto ExternalScore under the same contract (fail loudly on unmapped vocabulary, tally deliberate drops, values never re-derived). Per-source stagers are individually testable, mirroring the stage_scores() pattern.
• scorecard_db/models.py — deliberate vocabulary extensions: 7 new Metric members (taxpayer_count, average_tax_rate, average_tax_amount, unclaimed_benefit_amount, gini, income_statistic, income_share), 3 UnitConcepts (gbp, benefit_units, children), and 8 documented UK STANDARD_CONDITIONS keys (country, fy, housing_costs, poverty_line, basis, welfare_cap, quantile, per).
• tests/test_uk_externals_ingest.py — mapping tests on synthetic rows (run on any branch) + a full round-trip integration test that self-activates once the adapter outputs merge.

Verified end-to-end

Run against the real adapter outputs (pulled from the #43–#47 branches): 16,924 claims — dwp_takeup 1,638 / dwp_hbai 12,888 / uk_hmrc 1,213 / obr 259 / ukmod 758 — with zero claim_id collisions through the finish() gate, and deliberate drops tallied (1,456 DWP CI-range variants; 373 UKMOD non-primary variants — score against primaries, never transcriptions).

Honesty decisions

• DWP take-up rates land consumed_as_target (pe-uk take-up parameters cite the same publication); HBAI is held_out under the census-spm doctrine; OBR 2024-25 outturn is consumed_as_target, forecast years held_out.
• HMRC ready-reckoner rows are reform claims: policy_ref worlds slugged from the verbatim change description against hmrc_indexed_baseline_spring_2025, with the description kept as conditions["option"] — so each of the 75 changes × 3 years is a uniquely-keyed world.
• UK financial years normalize to one fy label + integer end-year period; HBAI 3-year regional averages ride period_start/period_end with window_kind=annual_average.

Finding: the campaign descriptors need re-emission

The staged hmrc_reckoner_t2 family (14 PE results in sources/campaign-20260802/uk/) cannot attach yet: its descriptors under-specify. Two personal-allowance rows carry byte-identical conditions (tax_section + fy + direction_semantics) — the actual change (£100 vs 1% vs 10%) lives only in free-text pe_construction. The exactly-one-match contract in ingest_campaign.py correctly refuses this, and the repo's never-guess doctrine says the fix is re-emission with the change label, not inference. The DB claims here are keyed (option, fy) ready for that join — the integration test demonstrates the exactly-one query. The other UK families (free_joins → OBR receipts lines, obr_measures → policy costings, two_child/uprating → Resolution Foundation) additionally need their harvest claims, which are not vendored in this repo.

Sequencing

Merge after #43–#47 (the module's inputs). CI is green on this branch standalone — the integration test skips itself until the externals exist.

🤖 Generated with Claude Code

[vahid-ahmadi]

Three review defects fixed

1. Two different reforms shared one reform world. HMRC prints "Change standard rate by 1 percentage point" under both the VAT and the Insurance Premium Tax sections, and the policy slug was built from the description alone — so six claims shared one reform_key across worlds 14× apart in magnitude (£9.2bn vs £640m in 2027-28). A single PE compute of "+1pp VAT" would have been joined as the counterpart to the IPT claim. finish() could not catch this: the claim ids differ by program, but ReformRef is the executable descriptor the PE side computes against, and scorecard_db/README.md requires identical worlds to share a key and distinct worlds not to. The slug now carries the tax head, and both a unit test and a DB-level assertion (no reform_key spanning two programs) pin it.

2. Calibration decisions bypassed relationships.py. They were inline per-metric constants, even though that module's own docstring calls itself the canonical home and says the assignment is made "not defaulted". Added uk_relationship() there — keyed by (source, metric), because the UK families are publication-shaped rather than program-shaped, and the OBR's outturn/forecast split turns on the year. It raises on an unassigned pair instead of defaulting to held_out.

3. A derivative of a consumed target was labelled held-out. Entitled-non-recipient counts and unclaimed amounts were held_out while the take-up rate they are an exact algebraic function of (takeup = R/(R+ENR)) was consumed_as_target. If pe-uk is fitted to the rate and calibrated to the admin caseloads supplying R, it reproduces ENR by construction — an apparent validation win that the 2026-08-02 doctrine's "nor anything derived from such" already covers. The whole DWP take-up family is now consumed, including recipient counts. Where the per-parameter audit of pe-uk's provenance is still owed, the conservative direction is taken: consumed, never held_out.

Plus a doctrine gap this PR would have opened. PERMANENT_HOLDOUT_METRICS contained POVERTY_RATE but not POVERTY_COUNT — the same survey-derived quantity with the denominator multiplied back in. This ingest is the first population to carry poverty counts (6,528 HBAI rows), so without the addition a future ingest could mark them consumed and the guard would not fire. Added, with a test.

Re-verified end to end

Against the fixed adapter outputs from #43–#47: 16,924 claims (dwp_takeup 1,638 / dwp_hbai 13,056 — +168 from the rows #44 recovered / uk_hmrc 1,213 / obr 259 / ukmod 758), zero reform_key collisions, zero poverty claims marked consumed. Full suite 137 passed, ruff clean.

Note for merge order: this now depends on #44's and #47's fixes for its claim counts, and #47's corrected values (32 of the wrong Table 4.7 cells previously reached the DB through this path). Re-run the ingest after those merge.

Still open from the review, deliberately not done here: upsert_scores has no delete-by-registry, so this slug change orphans the 225 previously-ingested HMRC claims in any DB built from the old code. That is the idempotency contract ingest_reform_validation.py implements and this module does not — worth its own PR rather than bundling.

[vahid-ahmadi]

Position on the feed-architecture question this PR is half of (now tracked as #57): I propose #32 lands first — it is the superset (eight sources vs five, Ledger routing, and the baselines registry #13 that the mode-2 lanes #54/#56 need) — and this PR then reduces to a rebase of its three review-hardening deltas that #32 does not carry:

1. the tax-head reform_key disambiguation (VAT/IPT collision) + the no-reform_key-spans-two-programs DB assertion,
2. uk_relationship() in relationships.py with raise-on-unassigned (no silent held_out default),
3. the consumed-target-derivative audit (entitled-non-recipient counts and unclaimed amounts must inherit consumed from the take-up rate they algebraically determine).

I'll do that rebase as a follow-up PR once #32 merges, and this one can close in its favour. If maintainers prefer the opposite order, those three deltas are the part that must survive either way. Either resolution un-HOLDs #50 (its reviewer parked it on exactly this adjudication).

[DTrim99]

Review — UK externals → DB ingest path

Strong foundation PR. I ran the ingest + the full test suite against the real data/externals/ files: all 10 tests pass, finish() raises no collisions, and values pass through verbatim (spot-checked the OBR 137273241932.22 — un-rederived). Fail-loud paths (unknown variant/metric, unassigned (source, metric)) are raised and tested; per-source stagers are individually tested. One real issue and a few should-fixes below.

Should fix

• OBR aggregate_level / parent silently dropped → double-count hazard (ingest_uk_externals.py:422-447, stage_obr). The OBR file carries aggregate_level (component/subtotal/total) + parent, but the stager ignores both — all 259 rows land as sibling benefit_cost claims (total_welfare, dwp_social_security, pension_credit, …) with no marker, so any consumer summing OBR benefit_cost by FY double-counts. Add aggregate_level to conditions (and register it), or filter to leaf components.
• No require_fields gate in the UK ingest. The US harvest path fails loud on unhandled staged fields (harvest.py:41-48); the UK path has no equivalent, which is exactly why the OBR columns above pass unnoticed. Porting that gate would catch this class of drop going forward.
• component condition key used but not registered (ingest_uk_externals.py:196,202 emit conditions["component"], but it's missing from STANDARD_CONDITIONS, models.py:115). Harmless at runtime (the frozenset is documentary) but breaks the "keys adapters MUST use" registry contract.
• Headline count mismatch: the PR says 16,756 claims; the ingest actually produces 16,924 (dwp 1638 / hbai 13056 / hmrc 1213 / obr 259 / ukmod 758). Worth reconciling the number.

Minor

• HBAI poverty counts for children/pensioners/working-age all get UnitConcept.PERSONS with the subgroup riding in subgroup only — no collision, but the unit label is imprecise.

Heads-up on merge order: #52 depends on this PR's UnitConcept.GBP, and #65 also adds UnitConcept.GBP to models.py — those two will collide, worth deconflicting.

Reviewed with Claude Code assistance.

[vahid-ahmadi]

All four should-fixes addressed in 0a8bc91 (plus a merge of main so the #43–#47 adapters live under this branch and the integration tests run against regenerated outputs):

1. OBR hierarchy: stage_obr now carries aggregate_level on every claim and parent on every non-total row — the double-count hazard is marked on the rows themselves. Tested (test_obr_hierarchy_lands_in_conditions).
2. require_fields gate: _load() enforces the harvest.py contract against a per-file known-field registry; an unhandled adapter column now raises (test_load_gate_rejects_unhandled_adapter_fields). The HMRC sign-convention columns are declared known explicitly.
3. Condition registry: component, aggregate_level, parent registered in STANDARD_CONDITIONS with comments; membership tested.
4. Count reconciled: the ingest produces 16,924 (dwp 1638 / hbai 13056 / hmrc 1213 / obr 259 / ukmod 758) — title and this thread now say so; 16,756 was a stale pre-rerun figure.

On the minor: HBAI subgroup counts keep UnitConcept.PERSONS with the cut in subgroup — changing the unit label would churn claim ids for a labeling nicety, so leaving as-is unless you feel strongly. On the UnitConcept.GBP collision with #65: acknowledged — whichever merges second rebases; the enum addition is identical text so it's a trivial conflict either way.

[DTrim99]

Re-review — main findings addressed, two loose ends ✅⚠️

Applied the diff locally and ran it: all 13 tests pass and the full ingest produces exactly 16,924 claims (dwp 1638 / hbai 13056 / hmrc 1213 / obr 259 / ukmod 758).

Addressed:

• OBR aggregate_level/parent — stage_obr now emits both into conditions, with test_obr_hierarchy_lands_in_conditions, closing the double-count hazard.
• require_fields gate — _load now fails loud on unhandled adapter fields against per-file _KNOWN_FIELDS.
• component key — now registered in STANDARD_CONDITIONS, asserted by a test.

Two loose ends:

• PR body still says "16,756 claims" — the code and title now agree at 16,924; just the description is unreconciled.
• GBP merge conflict with Reform-validation population: country dimension (per-release regression history for policyengine-uk) #65 — both this PR and Reform-validation population: country dimension (per-release regression history for policyengine-uk) #65 add GBP = "gbp" to models.py UnitConcept, so whichever merges second will conflict. Trivial to resolve, but worth coordinating.

Substance looks good.

[DTrim99]

Holding a full approval only on the two loose ends from my re-review: reconcile the PR body to 16,924 (code and title already agree), and coordinate the models.py GBP = "gbp" line with #65 so whichever merges second doesn't conflict. Substance is solid — happy to approve once those are squared away.

[vahid-ahmadi]

Both squared away: the PR body was reconciled to 16,924 earlier today (the comment likely crossed with the edit — it reads 16,924 now, matching title and code). On the GBP line: proposed sequence is this PR merges first (it is the base of the #52 stack), and #65 rebases immediately after, dropping its duplicate GBP = "gbp" line — committed to on #65 as well.

[MaxGhenis]

Repaired against the adjudication's eight blockers and re-pushed (dual-gate flow, same as #16/#24): every relationship now traces to a consumption surface read at the certified pins — pe-uk-data@dd68c73's targets/sources modules and policyengine-uk's take-up parameters. Headlines: 749 admin outturn cells route to data/ledger/uk_admin_outturns.jsonl per the boundary rule (with consuming pins named); OBR consumption is per-line (the 12 Table 4.9 rows pe-uk-data reads), not per-basis; PC take-up flipped to seed_source (the parameter cites the FYE-2020 edition), HB held out on the engine's stated design; a closed identity registry (uk_aliases.py) canonicalizes cross-source axes (HBAI-relative ≡ UKMOD-60 deliberately; explicit DISTINCT pairs) and raises on unknowns; units repaired (gbp value kinds, per-week/per-month/index concepts); ingestion is one transaction with rollback verified by injection; accounting is exact (16,175 + 749 + 1,829 = 16,924, drift raises). Suite 213. Note: the committed DB is 54.3 MB — past GitHub's 50 MB warning, still under the 100 MB hard limit; the storage decision (LFS/shard/untracked) is flagged for Max before the long-tail sources land.