Skip to content

Fix #792: derive band edges from the compiled register, never the pruned roster - #803

Open
juaristi22 wants to merge 3 commits into
mainfrom
792-band-edge-fix
Open

Fix #792: derive band edges from the compiled register, never the pruned roster#803
juaristi22 wants to merge 3 commits into
mainfrom
792-band-edge-fix

Conversation

@juaristi22

@juaristi22 juaristi22 commented Aug 27, 2026

Copy link
Copy Markdown
Collaborator

Closes #792 (code side). Based on main's head (#793 merged as 58a09bd; rebased onto it, trees byte-identical to the verified state).

The defect — deterministic, not RNG

Banded measures took each band's upper edge from the lower edges of the sibling specs surviving in the pruned registry (_band_edges_by_group iterates registry.specs; the seam applies the measure-exclusion register before materialization). Excluding a band therefore silently widened its lower neighbour to the next surviving edge — or to infinity at the top.

This is the whole of the 5→47 register movement: every one of the 12 movers' 361-roster initial estimates equals its 403-roster value plus the 403-roster values of exactly the excluded sibling bands its widened band absorbed (max residual 1.1e-5 on values up to 2.6e10). Replaying the edge logic predicts 21 widened bands; the 12 with model mass in the absorbed region moved, the other 9 are bit-stable. The issue's RNG hypothesis is refuted three ways: formula-time randomness is statically banned in policyengine-core; every UK draw is minted spine-side, identity-keyed, and frozen into the H5; and the operator's own rng_control receipts show 0/403 rows differing across code pins. The 4 blocked uk_target_fit cells attributed to this class are artifacts — those bands pass (or nearly) at published widths.

The fix

  • Kernel (target_materialization.py): materialize_target_bindings and resolve_target_measures accept a keyword-only band_edge_registry (default None = derive from the materialized registry, bit-identical for every existing caller). Band edges come from it; the materialization loop stays on the pruned set. _band_bounds now refuses a spec whose own lower edge is absent from the supplied edge set, rather than guessing — the same posture the function already took for unreadable edges.
  • UK seam threading: materialize_uk_ledger_targetsUKNationalCalibrationStagerun_uk_calibration; the seam tool passes compilation.registry. run_uk_calibration refuses, before any artifact is written, (a) a non-empty exclusion receipt without a band-edge register, and (b) a band-edge register whose extra names do not exactly reconstitute the receipt (the applier already fails exclusions matching zero specs, so pruned + receipt == compiled is exact). Provenance: run_config.band_edge_register_sha256 (the registry content hash) flows into the build record and identity digest.
  • Regression fence: survivors must be bit-identical under sibling exclusion — covered for numeric HMRC-style edges and published DWP range labels (× band_period_factor), plus refusal and threading tests at every seam layer. The middle-band prune flips [0,20) to [0,40) on pre-fix semantics, so the fence fails on the old code by construction.

Deliberately untouched: certify_uk_release_cut.py's pruned reference registry (a name@period roster-parity instrument — it never materializes values, which is exactly why its parity arm could not catch this drift), and the scoring path (prepare_uk_target_frame — the frozen scoring register is pinned to the compiled registry before the prune, so its self-derived edges are already the published ones). Register census counts are invariant: compiled/excluded/calibrated do not change, only survivors' measured values return to published band widths.

Verification

Ruff, ci_test_groups.py --verify, the six touched test files, and the full shared-spec and uk CI group file lists all pass locally under the locked engine env (run while the PR was stacked on the pre-merge #793 branch; now that the base is main, PR CI runs the same lanes). The data claim — the 12 movers reverting to 403-roster values and the 4 artifact cells reading at published widths — is a licensed seam-run fact, verified on the next seam run outside PR CI, per the PR-CI/certification boundary.

🤖 Generated with Claude Code

@vahid-ahmadi

Copy link
Copy Markdown
Contributor

Automated review pass (Claude Code, high effort, diff only — no execution). Four findings. The first two matter most: between them, the default path reintroduces #792 and the refusal degrades into a silent drop.

1. uk_runtime/national_calibration.py:59 — the default falls back to the pruned roster

self.band_edge_registry = band_edge_registry if band_edge_registry is not None else self.registry falls back to self.compilation.registry, which is the post-compilation roster — UKLedgerTargetCompilation drops unsupported specs. The parameter defaults to None, so every existing caller takes that path, and edges are derived from a pruned roster with the surviving lower neighbour silently widened. That is #792 restated: the fix holds only for callers that thread the compiled register, and none of them do yet.

The refusal added in calibration_run does not cover this path, so there is currently no signal that the fallback was taken. Either require the register (no default) or refuse when the fallback would be used, so a caller that has not been updated fails rather than silently getting the old behaviour.

2. target_materialization.py:421 — the refusal degrades into a skipped target

The new "band lower edge absent from the band-edge set" condition is raised as a ValueError inside the per-spec loop, and the PR's own test — test_band_bounds_refuse_a_spec_absent_from_the_band_edge_register — asserts it lands in result.skipped rather than propagating. So a band-edge register that genuinely does not cover a spec does not refuse: the target quietly drops out of the solve and the run reports success with one fewer target.

That is a weaker outcome than the interim fix it replaces, which raised naming every candidate. A missing target is materially worse than a wrongly-banded one in one respect — a wrong band shows up as a bad fit, whereas a dropped target shows up as nothing at all. Worth deciding deliberately whether an uncovered spec is a skip or a refusal; the test name says refuse and the assertion says skip.

3. uk_runtime/calibration_run.py:308 — an empty receipt reads as permission

if exclusion_receipt and band_edge_registry is None: raise gates on truthiness, so an empty-but-present receipt ({}) passes the guard, and the function then returns at if band_edge_registry is None: return with no check performed at all. Absence of receipt entries is not evidence the register was unpruned — a pruned register with an empty or not-yet-populated receipt produces the wrong edges with no error. Distinguishing "no receipt" from "a receipt saying nothing was excluded" is the fix.

4. uk_runtime/calibration_run.py:320-327 — the reconciliation may be comparing different key spaces

The consumed/compiled reconciliation compares spec names against exclusion_receipt keys ({str(name) for name in exclusion_receipt}) and asserts strict set equality. Nothing in the diff establishes that receipt keys are spec names — the type is Mapping[str, Mapping[str, str]], which would equally admit keying by measure or by family. If the key spaces differ, every pruned run raises spuriously; if the receipt's key space is coarser than spec names, the equality can hold while individual excluded bands go unaccounted for.

Separately, the reconciliation checks only that the name sets line up — nothing asserts that every compiled band edge was actually consumed by some spec. An edge compiled but never used is the same rot the register is meant to prevent, one level down.


1 and 2 I would resolve before merge. Both leave the fixed code path correct while the path everything actually takes keeps the old behaviour — and in 2's case the PR's own test pins the weaker outcome, so a future reader would reasonably conclude the skip was intended.

juaristi22 and others added 3 commits August 28, 2026 11:26
Banded measures took each band's upper edge from the lower edges of
sibling specs in the live registry, so excluding a band silently widened
its lower neighbour to the next surviving edge (or to infinity at the
top) — the #792 movers: 12 of 361 shared measures shifted by up to +858%
when the exclusion register grew 5->47, each by exactly the mass of the
absorbed excluded siblings. materialize_target_bindings and
resolve_target_measures now accept a band_edge_registry (default: the
materialized registry, bit-identical for existing callers), and
_band_bounds refuses a spec whose own lower edge is absent from the
supplied edge set rather than guessing. Regression fence: survivors must
be bit-identical under sibling exclusion, for numeric edges and
published range labels.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The seam prunes measure exclusions before materialization, so the stage
only ever saw the pruned registry. run_uk_calibration now takes the
compiled pre-exclusion register as band_edge_registry and refuses, before
any artifact is written, a non-empty exclusion receipt without it — and
a register whose extra names do not exactly reconstitute the receipt
(the applier already fails exclusions that match zero specs, so
pruned + receipt == compiled is exact). The registry content hash is
recorded as run_config.band_edge_register_sha256, flowing into the build
record and identity digest. The seam tool passes compilation.registry;
census counts are unchanged — only survivors' measured values return to
published band widths. The stale prose in test_uk_measure_simulation
that read the widening artifact as the resolution mechanism is corrected.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…verage fence

Findings 1 and 3 (vahid-ahmadi): band_edge_registry is now REQUIRED at
UKNationalCalibrationStage and run_uk_calibration - never defaulted. The
stage cannot tell a pruned registry from a full one, so the old fallback
to self.registry restated #792 for any direct caller holding a pruned
roster, and gating the run-level check on receipt truthiness let an
empty-but-present receipt skip reconciliation entirely. The
reconciliation now always runs: an empty receipt is a claim that nothing
was pruned, so the rosters must be name-identical. Receipt keys are spec
names by the applier's construction (it raises on zero-match exclusions
and builds the receipt from matched spec names), documented at the check
(finding 4's key-space concern).

Finding 2: the coverage fence in _band_bounds now raises
BandEdgeCoverageError, a RuntimeError the per-spec skip catch does not
swallow - a register that cannot bound a spec is a wrong-register
problem for the whole run, and the target must never quietly drop out of
the solve. The refusal test asserts propagation instead of a skip, and
new tests pin the TypeError on omission and the empty-receipt
reconciliation.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@juaristi22

Copy link
Copy Markdown
Collaborator Author

All four dispositioned in 2a445b8 (the branch is also now rebased onto main's head, 1225a27 — clean, zero file overlap with #755).

1 — fixed as proposed (required, no default). band_edge_registry is now a required keyword argument of UKNationalCalibrationStage: the stage cannot tell a pruned registry from a full one, so any fallback restates #792 for a direct caller. One correction for the record: the production path did thread the compiled register before this change (seam tool → run_uk_calibration → stage, pinned by test_driver_threads_registry_exclusions_resolver_and_overrides), so the exposure was direct stage construction, not "every existing caller" — but the finding stands: that path existed and defaulted silently. On the unsupported nuance: compilation-dropped specs never publish edges into the compiled register either, and the seam tool exits on non-empty unsupported before binding — the pruned-roster mechanism here is the exclusion applier, not compilation.

2 — fixed (refusal, not skip). The coverage fence now raises BandEdgeCoverageError, a RuntimeError deliberately outside the per-spec except (KeyError, TypeError, ValueError) — so it propagates from materialize_target_bindings itself rather than degrading into a skip, and the test asserts propagation. Worth noting the blast radius before the fix was narrower than "the run reports success": the stage already hard-fails on any skip (national_calibration.py, "could not materialize every activated target reference"), so the seam path could not lose a target silently — but a bare kernel caller could, and the test pinned the weaker semantics as if intended. Both wrong; both fixed. The unreadable-edge ValueError keeps skip semantics on purpose — that one is a per-spec data defect with its own pinning test.

3 — fixed, subsumed by 1. With the parameter required at run_uk_calibration too, there is no "no receipt" path: the reconciliation always runs, and an empty receipt is treated as a claim that nothing was pruned — the rosters must then be name-identical. New test: a pruned register with an empty receipt refuses.

4 — documented; the key spaces are aligned by construction. Receipt keys are spec names: apply_uk_calibration_measure_exclusions builds the receipt from the matched spec.name set and raises on any exclusion matching zero registry specs, so pruned + receipt keys == compiled is an exact identity — now stated in _validate_band_edge_registry's docstring rather than left implicit. On the second half: a compiled edge consumed by no surviving spec is not rot — it is the fix. An excluded band's lower edge exists precisely to serve as its surviving neighbour's upper bound; requiring every edge to be consumed would re-couple the measure definition to the roster, which is the defect this PR removes.

Verification: ruff, ci_test_groups.py --verify, and the six touched test files (95 tests) green locally; PR CI re-runs the full lanes on the push.

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.

Band edges derive from the exclusion-pruned register, silently widening surviving banded targets

2 participants