From 98d1cfa1a89f2ecd4a976b4c0b7ad8f21a9dee91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Juaristi?= <127882282+juaristi22@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:16:58 +0200 Subject: [PATCH 1/3] Derive band edges from the compiled register, never the pruned roster MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../792-band-edges-compiled-register.fixed.md | 1 + .../microcosm/build/target_materialization.py | 23 ++- .../tests/test_target_materialization.py | 188 ++++++++++++++++++ 3 files changed, 208 insertions(+), 4 deletions(-) create mode 100644 changelog.d/792-band-edges-compiled-register.fixed.md diff --git a/changelog.d/792-band-edges-compiled-register.fixed.md b/changelog.d/792-band-edges-compiled-register.fixed.md new file mode 100644 index 00000000..afe982e6 --- /dev/null +++ b/changelog.d/792-band-edges-compiled-register.fixed.md @@ -0,0 +1 @@ +Banded calibration targets now derive upper edges from the compiled pre-exclusion register, fixing microcosm#792 so excluding a banded target no longer widens its surviving lower neighbour. The materialization seam now refuses a pruned register unless callers provide its compiled band-edge source. diff --git a/packages/microcosm-build/src/microcosm/build/target_materialization.py b/packages/microcosm-build/src/microcosm/build/target_materialization.py index 79fdfb22..41468008 100644 --- a/packages/microcosm-build/src/microcosm/build/target_materialization.py +++ b/packages/microcosm-build/src/microcosm/build/target_materialization.py @@ -21,9 +21,11 @@ # encodings are in use — a numeric lower bound (HMRC SPI income bands) and a # published range label (DWP award bands, in monthly units, hence # ``band_period_factor``) — and both reduce to one lower edge, because no -# reference anywhere declares an upper bound. A band's upper edge is -# therefore its sibling's lower edge within the same contract target, and the -# top band runs to infinity. +# reference anywhere declares an upper bound. A band's upper edge is its +# sibling's lower edge within the same compiled contract target. Edges must +# never derive from an exclusion-pruned roster, or excluding a band silently +# widens its lower neighbour; only the compiled register's top band runs to +# infinity. # Entity-count indicators: a value of one per record of the owning entity. _COUNT_VALUE_VARIABLES = frozenset( {"household_count", "person_count", "benunit_count"} @@ -126,6 +128,7 @@ def resolve_target_measures( period: int | str, max_rounds: int = 8, contract_targets: Mapping[str, Mapping[str, Any]] | None = None, + band_edge_registry: TargetRegistry | None = None, ) -> MeasureResolution: """Resolve provider-computed inputs until target materialization binds. @@ -159,6 +162,7 @@ def resolve_target_measures( registry, contract, period=period, + band_edge_registry=band_edge_registry, ) skipped = tuple(result.skipped) round_receipt = { @@ -414,6 +418,13 @@ def _band_bounds( f"groupby_variable {binding['groupby_variable']!r} but its spec " "carries no readable band edge" ) + if lower not in band_edges: + raise ValueError( + f"banded measure {getattr(spec, 'measure', '?')!r} carries band lower edge " + f"{lower!r} that is absent from its contract target's band-edge set " + f"{list(band_edges)!r}; the band-edge register does not cover this spec's " + "band — pass the compiled (pre-exclusion) register this spec was pruned from." + ) upper = math.inf for edge in band_edges: if edge > lower: @@ -429,11 +440,15 @@ def materialize_target_bindings( *, period: int | str, providers: Mapping[str, Provider] | None = None, + band_edge_registry: TargetRegistry | None = None, ) -> TargetMaterializationResult: """Prepare measure columns declared by compiled Ledger target specs.""" provider_registry = {**default_provider_registry(), **(providers or {})} - band_edges = _band_edges_by_group(registry, contract_targets) + band_edges = _band_edges_by_group( + registry if band_edge_registry is None else band_edge_registry, + contract_targets, + ) skipped: list[MaterializationSkip] = [] for spec in registry.specs: if hasattr(adapter, "has_column") and adapter.has_column( diff --git a/packages/microcosm-build/tests/test_target_materialization.py b/packages/microcosm-build/tests/test_target_materialization.py index b4af50fb..1d726f3c 100644 --- a/packages/microcosm-build/tests/test_target_materialization.py +++ b/packages/microcosm-build/tests/test_target_materialization.py @@ -512,6 +512,63 @@ def test_bands_slice_the_population_and_partition_it(): assert list(total) == [1.0, 1.0, 1.0] +def test_band_measures_are_roster_invariant_under_sibling_exclusion(): + registry = _banded_registry() + full_adapter = StubAdapter() + + result = materialize_target_bindings( + full_adapter, registry, _BANDED_CONTRACT, period=2025 + ) + + assert result.skipped == () + snapshots = { + label: full_adapter.tables["person"][f"income_band_{label}"].copy() + for label in ("0", "20", "40") + } + + middle_pruned = TargetRegistry( + [spec for spec in registry.specs if spec.name != "band_20"], + country="uk", + ) + middle_adapter = StubAdapter() + + result = materialize_target_bindings( + middle_adapter, + middle_pruned, + _BANDED_CONTRACT, + period=2025, + band_edge_registry=registry, + ) + + assert result.skipped == () + for label in ("0", "40"): + assert np.array_equal( + middle_adapter.tables["person"][f"income_band_{label}"], + snapshots[label], + ) + + top_pruned = TargetRegistry( + [spec for spec in registry.specs if spec.name != "band_40"], + country="uk", + ) + top_adapter = StubAdapter() + + result = materialize_target_bindings( + top_adapter, + top_pruned, + _BANDED_CONTRACT, + period=2025, + band_edge_registry=registry, + ) + + assert result.skipped == () + for label in ("0", "20"): + assert np.array_equal( + top_adapter.tables["person"][f"income_band_{label}"], + snapshots[label], + ) + + def test_adjacent_bands_are_not_identical(): # The regression that would have caught the unsliced-measure defect: # before banding was implemented every band returned the same unsliced @@ -582,6 +639,137 @@ def test_published_range_labels_band_in_model_units(): assert list(adapter.tables["person"]["award_high"]) == [0.0, 0.0, 1.0] +def test_published_range_label_edges_survive_sibling_exclusion(): + registry = TargetRegistry( + [ + TargetSpec( + name="award_low", + entity="person", + measure="award_low", + value=1.0, + source="test", + family="dwp_universal_credit", + metadata={ + "contract_target_id": "uc.award_bands", + "ledger_filter_family_type": "Single, no children", + "ledger_filter_monthly_award_bands": "£1.01 to £2.00", + }, + ), + TargetSpec( + name="award_high", + entity="person", + measure="award_high", + value=1.0, + source="test", + family="dwp_universal_credit", + metadata={ + "contract_target_id": "uc.award_bands", + "ledger_filter_monthly_award_bands": "£2.01 to £3.00", + }, + ), + ], + country="uk", + ) + contract = { + "uc.award_bands": { + "bindings": { + "policyengine": { + "value_variable": "person_count", + "groupby_variable": "income", + "from_entity": "person", + "band_period_factor": 12, + } + } + } + } + pruned = TargetRegistry( + [spec for spec in registry.specs if spec.name != "award_high"], + country="uk", + ) + adapter = StubAdapter() + + result = materialize_target_bindings( + adapter, + pruned, + contract, + period=2025, + band_edge_registry=registry, + ) + + assert result.skipped == () + assert list(adapter.tables["person"]["award_low"]) == [0.0, 1.0, 0.0] + + +def test_band_bounds_refuse_a_spec_absent_from_the_band_edge_register(): + registry = TargetRegistry([_banded_registry().specs[0]], country="uk") + adapter = StubAdapter() + + result = materialize_target_bindings( + adapter, + registry, + _BANDED_CONTRACT, + period=2025, + band_edge_registry=TargetRegistry([], country="uk"), + ) + + assert len(result.skipped) == 1 + assert result.skipped[0].name == "band_0" + assert "absent from its contract target's band-edge set" in result.skipped[0].reason + assert "income_band_0" not in adapter.tables["person"] + + +def test_resolve_target_measures_threads_the_band_edge_registry(): + class BandedInputProvider(StubMeasureProvider): + def compute(self, entity, variable): + assert (entity, variable) == ("person", "input_a") + return np.array([1.0, 2.0, 3.0]), "stub:person.input_a" + + source = { + "person": pd.DataFrame( + { + "person_id": [1, 2, 3], + "income": [10.0, 20.0, 30.0], + } + ) + } + registry = _banded_registry() + pruned = TargetRegistry( + [spec for spec in registry.specs if spec.name != "band_20"], + country="uk", + ) + probes = [] + + def adapter_factory(): + adapter = ResolutionAdapter(source) + probes.append(adapter) + return adapter + + resolution = resolve_target_measures( + adapter_factory, + pruned, + BandedInputProvider(), + period=2025, + contract_targets={ + "spi.income_by_band": { + "bindings": { + "policyengine": { + "value_variable": "input_a", + "groupby_variable": "income", + "from_entity": "person", + } + } + } + }, + band_edge_registry=registry, + ) + + assert resolution.receipt["attached"] == { + "person.input_a": "stub:person.input_a" + } + assert list(probes[-1].tables["person"]["income_band_0"]) == [1.0, 0.0, 0.0] + assert list(probes[-1].tables["person"]["income_band_40"]) == [0.0, 0.0, 0.0] + + def test_unreadable_band_is_skipped_not_silently_unsliced(): adapter = StubAdapter() registry = TargetRegistry( From 5958bad23a0ddac5207abc1d6348fced97f10bb8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Juaristi?= <127882282+juaristi22@users.noreply.github.com> Date: Thu, 27 Aug 2026 17:17:10 +0200 Subject: [PATCH 2/3] Thread the compiled band-edge register through the UK calibration seam MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../build/uk_runtime/calibration_run.py | 49 +++++++ .../build/uk_runtime/ledger_targets.py | 2 + .../build/uk_runtime/national_calibration.py | 7 + .../tests/test_uk_calibration_run.py | 132 ++++++++++++++++++ .../tests/test_uk_calibration_seam_driver.py | 6 +- .../tests/test_uk_measure_simulation.py | 6 +- .../tests/test_uk_national_calibration.py | 38 +++++ tools/calibrate_uk_national_dataset.py | 1 + 8 files changed, 237 insertions(+), 4 deletions(-) diff --git a/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py b/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py index 5bb27c48..28fc29ed 100644 --- a/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py +++ b/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py @@ -197,6 +197,7 @@ def run_uk_calibration( input_sha256: str, ledger_artifact: Any, register_registry: TargetRegistry, + band_edge_registry: TargetRegistry | None = None, calibration_year: int, exclusion_receipt: Mapping[str, Mapping[str, str]], doctrine: Any, @@ -211,6 +212,15 @@ def run_uk_calibration( started_at = time.perf_counter() started_ts = datetime.now(UTC) + # Pure-argument validation precedes every environment probe: a pruned + # register without its compiled band-edge source must refuse identically + # whether or not a git checkout or Logbook chain is reachable. + _validate_band_edge_registry( + register_registry=register_registry, + band_edge_registry=band_edge_registry, + exclusion_receipt=exclusion_receipt, + ) + edge_registry = band_edge_registry or register_registry code_pin = git_code_pin(_REPOSITORY) # Predecessor configuration is validated before anything is written: a # disagreeing chain must refuse with no artifact on disk, not after a @@ -230,6 +240,7 @@ def run_uk_calibration( "ledger": _ledger_provenance(ledger_artifact), **dict(run_config_extra), } + run_config["band_edge_register_sha256"] = edge_registry.version state = AttemptState( # Attempts are distinct rows even when they re-run one release: both # the local chain and the store refuse a repeated build id. @@ -246,6 +257,7 @@ def run_uk_calibration( input_sha256=input_sha256, ledger_artifact=ledger_artifact, register_registry=register_registry, + band_edge_registry=edge_registry, calibration_year=calibration_year, exclusion_receipt=exclusion_receipt, doctrine=doctrine, @@ -287,6 +299,41 @@ def _new_calibration_attempt_id(*, timestamp: datetime) -> str: ) +def _validate_band_edge_registry( + *, + register_registry: TargetRegistry, + band_edge_registry: TargetRegistry | None, + exclusion_receipt: Mapping[str, Mapping[str, str]], +) -> None: + if exclusion_receipt and band_edge_registry is None: + raise ValueError( + "a pruned register cannot derive published band edges from itself; " + "pass the compiled pre-exclusion register." + ) + if band_edge_registry is None: + return + register_names = _registry_spec_names(register_registry) + edge_names = _registry_spec_names(band_edge_registry) + excluded_names = {str(name) for name in exclusion_receipt} + if not register_names <= edge_names: + missing = sorted(register_names - edge_names) + raise ValueError( + "band-edge register must include every materialized registry spec; " + f"missing={missing}." + ) + extra = edge_names - register_names + if extra != excluded_names: + raise ValueError( + "band-edge register extra spec names must match the measure " + f"exclusion receipt; extra={sorted(extra)}, " + f"receipt={sorted(excluded_names)}." + ) + + +def _registry_spec_names(registry: TargetRegistry) -> set[str]: + return {str(spec.name) for spec in registry.specs} + + def _record_failed_attempt( *, error: BaseException, @@ -336,6 +383,7 @@ def _run_uk_calibration_attempt( input_sha256: str, ledger_artifact: Any, register_registry: TargetRegistry, + band_edge_registry: TargetRegistry, calibration_year: int, exclusion_receipt: Mapping[str, Mapping[str, str]], doctrine: Any, @@ -374,6 +422,7 @@ def _run_uk_calibration_attempt( period=calibration_year, doctrine=doctrine, measure_resolver=measure_resolver, + band_edge_registry=band_edge_registry, ) calibrated = stage(frame) append_phase(state, "national_calibration_solved") diff --git a/packages/microcosm-build/src/microcosm/build/uk_runtime/ledger_targets.py b/packages/microcosm-build/src/microcosm/build/uk_runtime/ledger_targets.py index 7e3608fa..0a1f0493 100644 --- a/packages/microcosm-build/src/microcosm/build/uk_runtime/ledger_targets.py +++ b/packages/microcosm-build/src/microcosm/build/uk_runtime/ledger_targets.py @@ -88,6 +88,7 @@ def materialize_uk_ledger_targets( registry: TargetRegistry, *, period: int | str, + band_edge_registry: TargetRegistry | None = None, ) -> TargetMaterializationResult: """Materialize compiled UK Ledger target bindings on an adapter.""" @@ -102,6 +103,7 @@ def materialize_uk_ledger_targets( "baseline_flag_crosstab": _uk_baseline_flag_crosstab, "input_substitution_counterfactual": _uk_input_substitution, }, + band_edge_registry=band_edge_registry, ) diff --git a/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py b/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py index 4ff938bb..df294ee2 100644 --- a/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py +++ b/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py @@ -48,6 +48,7 @@ def __init__( period: int, doctrine: UKNationalSolveDoctrine = UK_NATIONAL_SOLVE_DOCTRINE, measure_resolver: object | None = None, + band_edge_registry: TargetRegistry | None = None, ) -> None: self.compilation = ( registry @@ -55,6 +56,9 @@ def __init__( else UKLedgerTargetCompilation(registry=registry, unsupported=()) ) self.registry = self.compilation.registry + self.band_edge_registry = ( + band_edge_registry if band_edge_registry is not None else self.registry + ) # The materialization period is the declared calibration year the # registry was compiled at — never the input frame's base-year # time_period, which lags it (survey 2024, calibration 2025). @@ -90,6 +94,7 @@ def __call__(self, frame: Frame) -> Frame: adapter, self.registry, period=self.period, + band_edge_registry=self.band_edge_registry, ) if materialized.skipped: skipped = [skip.__dict__ for skip in materialized.skipped] @@ -209,12 +214,14 @@ def _resolve_measures(self, frame: Frame) -> MeasureResolution | None: lambda: _CalibrationFrameAdapter(frame), self.registry, period=self.period, + band_edge_registry=self.band_edge_registry, ) return resolve_target_measures( lambda: _CalibrationFrameAdapter(frame), self.registry, self.measure_resolver, period=self.period, + band_edge_registry=self.band_edge_registry, ) def checkpoint_metadata(self) -> Mapping[str, object]: diff --git a/packages/microcosm-build/tests/test_uk_calibration_run.py b/packages/microcosm-build/tests/test_uk_calibration_run.py index 42420d26..96865c3d 100644 --- a/packages/microcosm-build/tests/test_uk_calibration_run.py +++ b/packages/microcosm-build/tests/test_uk_calibration_run.py @@ -87,6 +87,16 @@ def _registry(): ) +def _paths(tmp_path: Path) -> UKCalibrationRunPaths: + return UKCalibrationRunPaths( + input_h5=tmp_path / "input.h5", + staging_h5=tmp_path / "staged.h5", + diagnostics_json=tmp_path / "diagnostics.json", + build_record_json=tmp_path / "build_record.json", + terminal_gate_json=tmp_path / "terminal_gates.json", + ) + + def _sha(path: Path) -> str: return hashlib.sha256(path.read_bytes()).hexdigest() @@ -265,6 +275,128 @@ def test_run_uk_calibration_writes_cross_pinned_outputs(monkeypatch, tmp_path: P assert result.logbook_spool.exists() +def test_run_uk_calibration_refuses_pruned_register_without_band_edge_register( + tmp_path: Path, +): + paths = _paths(tmp_path) + + with pytest.raises(ValueError, match="compiled pre-exclusion register"): + run_uk_calibration( + paths=paths, + input_sha256="a" * 64, + ledger_artifact=object(), + register_registry=_registry(), + calibration_year=2025, + exclusion_receipt={"excluded.target": {"reason": "reviewed"}}, + doctrine=UKNationalSolveDoctrine(epochs=1), + doctrine_overrides={}, + measure_resolver=None, + source_pins={}, + run_config_extra={}, + release_id="pruned-without-edge-register", + ) + + assert not paths.staging_h5.exists() + assert not paths.diagnostics_json.exists() + assert not paths.build_record_json.exists() + + +def test_run_uk_calibration_refuses_incoherent_band_edge_register(tmp_path: Path): + paths = _paths(tmp_path) + edge_registry = TargetRegistry( + [ + *_registry().specs, + TargetSpec( + name="different.excluded", + entity="benunit", + measure="different/excluded", + value=1.0, + source="test", + metadata={"contract_target_id": "different.excluded"}, + ), + ], + country="uk", + ) + + with pytest.raises(ValueError, match="exclusion receipt"): + run_uk_calibration( + paths=paths, + input_sha256="a" * 64, + ledger_artifact=object(), + register_registry=_registry(), + band_edge_registry=edge_registry, + calibration_year=2025, + exclusion_receipt={"other.excluded": {"reason": "reviewed"}}, + doctrine=UKNationalSolveDoctrine(epochs=1), + doctrine_overrides={}, + measure_resolver=None, + source_pins={}, + run_config_extra={}, + release_id="incoherent-edge-register", + ) + + assert not paths.staging_h5.exists() + assert not paths.diagnostics_json.exists() + assert not paths.build_record_json.exists() + + +def test_run_uk_calibration_records_band_edge_register_sha256( + monkeypatch, tmp_path: Path +): + pytest.importorskip("tables") # pandas HDF backend + monkeypatch.setattr( + calibration_run, + "uk_aggregate_admin_totals", + lambda frame, manifest: (_admin_anchor_values(), []), + ) + input_h5 = tmp_path / "input.h5" + frame = _frame() + write_uk_national_frame(frame, input_h5) + _write_spine_sidecar(input_h5, frame) + paths = _paths(tmp_path) + register = _registry() + edge_registry = TargetRegistry( + [ + TargetSpec( + name="dwp.uc.households", + entity="benunit", + measure="dwp/uc/households", + value=99.0, + source="test", + family="dwp_universal_credit", + metadata={"contract_target_id": "dwp.uc.households"}, + ) + ], + country="uk", + ) + + result = run_uk_calibration( + paths=paths, + input_sha256=_sha(input_h5), + ledger_artifact=object(), + register_registry=register, + band_edge_registry=edge_registry, + calibration_year=2025, + exclusion_receipt={}, + doctrine=UKNationalSolveDoctrine(epochs=5), + doctrine_overrides={}, + measure_resolver=None, + source_pins={ + "input_h5": { + "sha256": _sha(input_h5), + "size_bytes": input_h5.stat().st_size, + } + }, + run_config_extra={}, + release_id="band-edge-provenance", + ) + + assert ( + result.build_record["run_config"]["band_edge_register_sha256"] + == edge_registry.version + ) + + def test_run_uk_calibration_refuses_input_sha_before_outputs(tmp_path: Path): pytest.importorskip("tables") # pandas HDF backend input_h5 = tmp_path / "input.h5" diff --git a/packages/microcosm-build/tests/test_uk_calibration_seam_driver.py b/packages/microcosm-build/tests/test_uk_calibration_seam_driver.py index 8ed5850e..f8cf5b3a 100644 --- a/packages/microcosm-build/tests/test_uk_calibration_seam_driver.py +++ b/packages/microcosm-build/tests/test_uk_calibration_seam_driver.py @@ -129,6 +129,7 @@ def test_driver_threads_registry_exclusions_resolver_and_overrides(monkeypatch, driver = _load_driver_module() calls = [] registry = _registry() + pruned_registry = TargetRegistry([], country="uk") artifact = SimpleNamespace( path=tmp_path / "ledger", facts=({"fact": 1},), @@ -152,7 +153,7 @@ def test_driver_threads_registry_exclusions_resolver_and_overrides(monkeypatch, monkeypatch.setattr( driver, "apply_uk_calibration_measure_exclusions", - lambda reg, exclusions: (reg, {"excluded": "reviewed"}), + lambda reg, exclusions: (pruned_registry, {"excluded": "reviewed"}), ) class FakeResolver: @@ -183,7 +184,8 @@ def fake_run(**kwargs): assert result == 0 call = calls[0] - assert call["register_registry"] is registry + assert call["register_registry"] is pruned_registry + assert call["band_edge_registry"] is registry assert call["calibration_year"] == 2025 assert call["exclusion_receipt"] == {"excluded": "reviewed"} assert call["doctrine"].epochs == 128 diff --git a/packages/microcosm-build/tests/test_uk_measure_simulation.py b/packages/microcosm-build/tests/test_uk_measure_simulation.py index 4e380007..59ba0648 100644 --- a/packages/microcosm-build/tests/test_uk_measure_simulation.py +++ b/packages/microcosm-build/tests/test_uk_measure_simulation.py @@ -289,8 +289,10 @@ def test_packaged_exclusions_load(): assert entry["tracking"] == "microcosm#791", entry["name"] # The lever targets are deliberately NOT excluded: the six UC - # caseload / two-child-limit cells ride the would_claim_uc lever run, - # and the two expected-to-resolve cells ride the exclusion re-run. + # caseload / two-child-limit cells ride the would_claim_uc lever run. + # Exclusions cannot move surviving cells because band edges are pinned + # to the compiled register (#792); the non-excluded cells are expected + # to pass at published band widths on the next seam run. excluded = set(names) for riding in ( "dwp.uc.households", diff --git a/packages/microcosm-build/tests/test_uk_national_calibration.py b/packages/microcosm-build/tests/test_uk_national_calibration.py index 1886242e..099c577b 100644 --- a/packages/microcosm-build/tests/test_uk_national_calibration.py +++ b/packages/microcosm-build/tests/test_uk_national_calibration.py @@ -21,6 +21,7 @@ UK_NATIONAL_TARGET_LOSS_CAP, UK_NATIONAL_TARGET_WEIGHT_RULE, UKNationalSolveDoctrine, + national_calibration, uk_doctrine_with_overrides, uk_national_target_loss_weights, ) @@ -372,6 +373,43 @@ def test_stage_manifest_omits_measure_resolution_without_resolver() -> None: assert "measure_resolution" not in stage.manifest +def test_stage_threads_band_edge_registry_to_materialization(monkeypatch) -> None: + captured = [] + real_materialize = national_calibration.materialize_uk_ledger_targets + + def capture_materialize(*args, **kwargs): + captured.append(kwargs) + return real_materialize(*args, **kwargs) + + monkeypatch.setattr( + national_calibration, + "materialize_uk_ledger_targets", + capture_materialize, + ) + sentinel = TargetRegistry([], country="uk") + stage = UKNationalCalibrationStage( + _registry(), + period=2025, + doctrine=UKNationalSolveDoctrine(epochs=1), + band_edge_registry=sentinel, + ) + + stage(_frame()) + + assert captured[-1]["band_edge_registry"] is sentinel + + captured.clear() + default_stage = UKNationalCalibrationStage( + _registry(), + period=2025, + doctrine=UKNationalSolveDoctrine(epochs=1), + ) + + default_stage(_frame()) + + assert captured[-1]["band_edge_registry"] is default_stage.registry + + def test_activated_unresolvable_compiled_reference_aborts_loudly() -> None: stage = UKNationalCalibrationStage( UKLedgerTargetCompilation( diff --git a/tools/calibrate_uk_national_dataset.py b/tools/calibrate_uk_national_dataset.py index 2f969784..c6e2118a 100644 --- a/tools/calibrate_uk_national_dataset.py +++ b/tools/calibrate_uk_national_dataset.py @@ -98,6 +98,7 @@ def main(argv: list[str] | None = None) -> int: input_sha256=args.input_sha256, ledger_artifact=artifact, register_registry=registry, + band_edge_registry=compilation.registry, calibration_year=calibration_year, exclusion_receipt=exclusion_receipt, doctrine=doctrine, From 2a445b86c8080c2eee65b508597c0c63c3ad8c4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mar=C3=ADa=20Juaristi?= <127882282+juaristi22@users.noreply.github.com> Date: Fri, 28 Aug 2026 11:35:39 +0200 Subject: [PATCH 3/3] Disposition the #803 review: required band-edge register, refusing coverage 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 --- .../792-band-edges-compiled-register.fixed.md | 2 +- .../microcosm/build/target_materialization.py | 13 +++++- .../build/uk_runtime/calibration_run.py | 28 ++++++----- .../build/uk_runtime/national_calibration.py | 10 ++-- .../tests/test_target_materialization.py | 25 ++++++---- .../tests/test_uk_calibration_run.py | 46 ++++++++++++++++++- .../tests/test_uk_national_calibration.py | 33 +++++++++---- 7 files changed, 117 insertions(+), 40 deletions(-) diff --git a/changelog.d/792-band-edges-compiled-register.fixed.md b/changelog.d/792-band-edges-compiled-register.fixed.md index afe982e6..cf46b53f 100644 --- a/changelog.d/792-band-edges-compiled-register.fixed.md +++ b/changelog.d/792-band-edges-compiled-register.fixed.md @@ -1 +1 @@ -Banded calibration targets now derive upper edges from the compiled pre-exclusion register, fixing microcosm#792 so excluding a banded target no longer widens its surviving lower neighbour. The materialization seam now refuses a pruned register unless callers provide its compiled band-edge source. +Banded calibration targets now derive upper edges from the compiled pre-exclusion register, fixing microcosm#792 so excluding a banded target no longer widens its surviving lower neighbour. The band-edge register is a required argument of the UK calibration stage and run seam — never defaulted, always reconciled against the materialized roster and the exclusion receipt (an empty receipt demands name-identical rosters) — and a register that cannot bound a spec refuses the run rather than letting the target drop out of the solve. diff --git a/packages/microcosm-build/src/microcosm/build/target_materialization.py b/packages/microcosm-build/src/microcosm/build/target_materialization.py index 41468008..a64f787a 100644 --- a/packages/microcosm-build/src/microcosm/build/target_materialization.py +++ b/packages/microcosm-build/src/microcosm/build/target_materialization.py @@ -75,6 +75,17 @@ class MeasureResolution: receipt: Mapping[str, Any] +class BandEdgeCoverageError(RuntimeError): + """A supplied band-edge register does not cover a materialized spec. + + Deliberately not a :class:`ValueError`: the per-spec materialization loop + converts ValueErrors into :class:`MaterializationSkip` entries, and a + register that cannot bound a spec is a wrong-register problem for the + whole run, not a per-spec data defect — it must refuse, never let the + target quietly drop out of the solve (#792 review finding 2). + """ + + class MeasureResolutionError(RuntimeError): """Raised when simulated measure resolution cannot bind the registry.""" @@ -419,7 +430,7 @@ def _band_bounds( "carries no readable band edge" ) if lower not in band_edges: - raise ValueError( + raise BandEdgeCoverageError( f"banded measure {getattr(spec, 'measure', '?')!r} carries band lower edge " f"{lower!r} that is absent from its contract target's band-edge set " f"{list(band_edges)!r}; the band-edge register does not cover this spec's " diff --git a/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py b/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py index 28fc29ed..6e7cdfcc 100644 --- a/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py +++ b/packages/microcosm-build/src/microcosm/build/uk_runtime/calibration_run.py @@ -197,7 +197,7 @@ def run_uk_calibration( input_sha256: str, ledger_artifact: Any, register_registry: TargetRegistry, - band_edge_registry: TargetRegistry | None = None, + band_edge_registry: TargetRegistry, calibration_year: int, exclusion_receipt: Mapping[str, Mapping[str, str]], doctrine: Any, @@ -212,15 +212,15 @@ def run_uk_calibration( started_at = time.perf_counter() started_ts = datetime.now(UTC) - # Pure-argument validation precedes every environment probe: a pruned - # register without its compiled band-edge source must refuse identically + # Pure-argument validation precedes every environment probe: an + # incoherent register/receipt/band-edge triple must refuse identically # whether or not a git checkout or Logbook chain is reachable. _validate_band_edge_registry( register_registry=register_registry, band_edge_registry=band_edge_registry, exclusion_receipt=exclusion_receipt, ) - edge_registry = band_edge_registry or register_registry + edge_registry = band_edge_registry code_pin = git_code_pin(_REPOSITORY) # Predecessor configuration is validated before anything is written: a # disagreeing chain must refuse with no artifact on disk, not after a @@ -302,16 +302,20 @@ def _new_calibration_attempt_id(*, timestamp: datetime) -> str: def _validate_band_edge_registry( *, register_registry: TargetRegistry, - band_edge_registry: TargetRegistry | None, + band_edge_registry: TargetRegistry, exclusion_receipt: Mapping[str, Mapping[str, str]], ) -> None: - if exclusion_receipt and band_edge_registry is None: - raise ValueError( - "a pruned register cannot derive published band edges from itself; " - "pass the compiled pre-exclusion register." - ) - if band_edge_registry is None: - return + """Require the band-edge register to reconstitute the compiled roster. + + The reconciliation always runs — an empty receipt is a claim that nothing + was pruned, so the two rosters must then be name-identical; it is never + permission to skip the check. Receipt keys are spec names by the + applier's construction: ``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, not a heuristic. + """ + register_names = _registry_spec_names(register_registry) edge_names = _registry_spec_names(band_edge_registry) excluded_names = {str(name) for name in exclusion_receipt} diff --git a/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py b/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py index df294ee2..e27be91f 100644 --- a/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py +++ b/packages/microcosm-build/src/microcosm/build/uk_runtime/national_calibration.py @@ -48,7 +48,7 @@ def __init__( period: int, doctrine: UKNationalSolveDoctrine = UK_NATIONAL_SOLVE_DOCTRINE, measure_resolver: object | None = None, - band_edge_registry: TargetRegistry | None = None, + band_edge_registry: TargetRegistry, ) -> None: self.compilation = ( registry @@ -56,9 +56,11 @@ def __init__( else UKLedgerTargetCompilation(registry=registry, unsupported=()) ) self.registry = self.compilation.registry - self.band_edge_registry = ( - band_edge_registry if band_edge_registry is not None else self.registry - ) + # Required, never defaulted: the stage cannot tell a pruned registry + # from a full one, so a fallback to self.registry would quietly + # restate #792 for any caller holding an exclusion-pruned roster. + # A caller whose registry is unpruned passes it explicitly. + self.band_edge_registry = band_edge_registry # The materialization period is the declared calibration year the # registry was compiled at — never the input frame's base-year # time_period, which lags it (survey 2024, calibration 2025). diff --git a/packages/microcosm-build/tests/test_target_materialization.py b/packages/microcosm-build/tests/test_target_materialization.py index 1d726f3c..c9865cb9 100644 --- a/packages/microcosm-build/tests/test_target_materialization.py +++ b/packages/microcosm-build/tests/test_target_materialization.py @@ -3,6 +3,7 @@ import pytest from microcosm.build.target_materialization import ( + BandEdgeCoverageError, MeasureResolutionError, assert_calibration_input_finite, materialize_target_bindings, @@ -701,20 +702,24 @@ def test_published_range_label_edges_survive_sibling_exclusion(): def test_band_bounds_refuse_a_spec_absent_from_the_band_edge_register(): + # A register that cannot bound a spec is a wrong-register problem for the + # whole run: it must propagate as a refusal, never degrade into a skipped + # target that quietly drops out of the solve (#803 review finding 2). registry = TargetRegistry([_banded_registry().specs[0]], country="uk") adapter = StubAdapter() - result = materialize_target_bindings( - adapter, - registry, - _BANDED_CONTRACT, - period=2025, - band_edge_registry=TargetRegistry([], country="uk"), - ) + with pytest.raises( + BandEdgeCoverageError, + match="absent from its contract target's band-edge set", + ): + materialize_target_bindings( + adapter, + registry, + _BANDED_CONTRACT, + period=2025, + band_edge_registry=TargetRegistry([], country="uk"), + ) - assert len(result.skipped) == 1 - assert result.skipped[0].name == "band_0" - assert "absent from its contract target's band-edge set" in result.skipped[0].reason assert "income_band_0" not in adapter.tables["person"] diff --git a/packages/microcosm-build/tests/test_uk_calibration_run.py b/packages/microcosm-build/tests/test_uk_calibration_run.py index 96865c3d..5781eb77 100644 --- a/packages/microcosm-build/tests/test_uk_calibration_run.py +++ b/packages/microcosm-build/tests/test_uk_calibration_run.py @@ -218,6 +218,7 @@ def test_run_uk_calibration_writes_cross_pinned_outputs(monkeypatch, tmp_path: P input_sha256=_sha(input_h5), ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=5), @@ -275,12 +276,15 @@ def test_run_uk_calibration_writes_cross_pinned_outputs(monkeypatch, tmp_path: P assert result.logbook_spool.exists() -def test_run_uk_calibration_refuses_pruned_register_without_band_edge_register( +def test_run_uk_calibration_requires_the_band_edge_register( tmp_path: Path, ): + # Required, never defaulted: an empty receipt is a claim that nothing was + # pruned, not permission to skip the reconciliation, so the seam takes no + # register-without-edges path at all (#803 review findings 1 and 3). paths = _paths(tmp_path) - with pytest.raises(ValueError, match="compiled pre-exclusion register"): + with pytest.raises(TypeError, match="band_edge_registry"): run_uk_calibration( paths=paths, input_sha256="a" * 64, @@ -301,6 +305,37 @@ def test_run_uk_calibration_refuses_pruned_register_without_band_edge_register( assert not paths.build_record_json.exists() +def test_run_uk_calibration_reconciles_an_empty_receipt_as_no_prunes( + tmp_path: Path, +): + # A pruned register handed in with an empty receipt must refuse: with + # nothing declared excluded, the two rosters have to be name-identical. + paths = _paths(tmp_path) + full = _registry() + pruned = TargetRegistry([], country="uk") + + with pytest.raises(ValueError, match="exclusion receipt"): + run_uk_calibration( + paths=paths, + input_sha256="a" * 64, + ledger_artifact=object(), + register_registry=pruned, + band_edge_registry=full, + calibration_year=2025, + exclusion_receipt={}, + doctrine=UKNationalSolveDoctrine(epochs=1), + doctrine_overrides={}, + measure_resolver=None, + source_pins={}, + run_config_extra={}, + release_id="empty-receipt-pruned-register", + ) + + assert not paths.staging_h5.exists() + assert not paths.diagnostics_json.exists() + assert not paths.build_record_json.exists() + + def test_run_uk_calibration_refuses_incoherent_band_edge_register(tmp_path: Path): paths = _paths(tmp_path) edge_registry = TargetRegistry( @@ -414,6 +449,7 @@ def test_run_uk_calibration_refuses_input_sha_before_outputs(tmp_path: Path): input_sha256="0" * 64, ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=1), @@ -447,6 +483,7 @@ def test_run_uk_calibration_refuses_absent_input_sidecar(tmp_path: Path): input_sha256=_sha(input_h5), ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=1), @@ -499,6 +536,7 @@ def test_run_uk_calibration_refuses_unbound_input_sidecar( input_sha256=_sha(input_h5), ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=1), @@ -567,6 +605,7 @@ def test_seam_never_modifies_data_variables(monkeypatch, tmp_path: Path): input_sha256=_sha(input_h5), ledger_artifact=object(), register_registry=pulling_registry, + band_edge_registry=pulling_registry, calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=50), @@ -697,6 +736,7 @@ def test_refusal_records_a_failed_attempt_and_stages_nothing(tmp_path: Path): input_sha256="0" * 64, ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=1), @@ -758,6 +798,7 @@ def test_attempt_ids_are_unique_across_reruns_of_one_release( input_sha256=_sha(input_h5), ledger_artifact=object(), register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=5), @@ -809,6 +850,7 @@ def test_verified_ledger_identity_reaches_the_run_evidence(monkeypatch, tmp_path input_sha256=_sha(input_h5), ledger_artifact=artifact, register_registry=_registry(), + band_edge_registry=_registry(), calibration_year=2025, exclusion_receipt={}, doctrine=UKNationalSolveDoctrine(epochs=5), diff --git a/packages/microcosm-build/tests/test_uk_national_calibration.py b/packages/microcosm-build/tests/test_uk_national_calibration.py index 099c577b..58a0a8c2 100644 --- a/packages/microcosm-build/tests/test_uk_national_calibration.py +++ b/packages/microcosm-build/tests/test_uk_national_calibration.py @@ -291,6 +291,7 @@ def test_uc_calibration_compiles_and_moves_weighted_count_towards_fact() -> None frame = _frame() stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=200, learning_rate=0.05), ) @@ -323,6 +324,7 @@ def test_uc_calibration_stage_accepts_benunit_grain_reference_on_nested_frame() frame = _nested_frame() stage = UKNationalCalibrationStage( _registry(value=60.0), + band_edge_registry=_registry(value=60.0), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -343,6 +345,7 @@ def test_stage_measure_resolver_injects_columns_then_restores_pristine_output() original_columns = {entity: set(frame.table(entity).columns) for entity in frame.entities} stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), measure_resolver=resolver, @@ -364,6 +367,7 @@ def test_stage_measure_resolver_injects_columns_then_restores_pristine_output() def test_stage_manifest_omits_measure_resolution_without_resolver() -> None: stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -398,16 +402,14 @@ def capture_materialize(*args, **kwargs): assert captured[-1]["band_edge_registry"] is sentinel - captured.clear() - default_stage = UKNationalCalibrationStage( - _registry(), - period=2025, - doctrine=UKNationalSolveDoctrine(epochs=1), - ) - - default_stage(_frame()) - - assert captured[-1]["band_edge_registry"] is default_stage.registry + # The parameter is required, never defaulted: a stage cannot tell a + # pruned registry from a full one (#803 review finding 1). + with pytest.raises(TypeError, match="band_edge_registry"): + UKNationalCalibrationStage( + _registry(), + period=2025, + doctrine=UKNationalSolveDoctrine(epochs=1), + ) def test_activated_unresolvable_compiled_reference_aborts_loudly() -> None: @@ -422,6 +424,7 @@ def test_activated_unresolvable_compiled_reference_aborts_loudly() -> None: }, ), ), + band_edge_registry=TargetRegistry([], country="uk"), period=2025, doctrine=UKNationalSolveDoctrine(epochs=1), ) @@ -501,6 +504,7 @@ def test_packaged_binding_classes_materialize_through_national_stage() -> None: resolver.contract_targets = _uk_contract_targets() stage = UKNationalCalibrationStage( registry, + band_edge_registry=registry, period=2025, doctrine=UKNationalSolveDoctrine(epochs=1, learning_rate=0.01), measure_resolver=resolver, @@ -561,6 +565,7 @@ def test_packaged_materialization_skip_aborts_national_stage() -> None: ) stage = UKNationalCalibrationStage( registry, + band_edge_registry=registry, period=2025, doctrine=UKNationalSolveDoctrine(epochs=1), ) @@ -573,6 +578,7 @@ def test_calibration_preserves_entity_ids_and_national_integrity() -> None: frame = _frame() stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -589,6 +595,7 @@ def test_checkpoint_metadata_round_trips_calibration_evidence() -> None: frame = _frame() stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -598,6 +605,7 @@ def test_checkpoint_metadata_round_trips_calibration_evidence() -> None: resumed = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -609,6 +617,7 @@ def test_checkpoint_metadata_round_trips_calibration_evidence() -> None: drifted = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -617,6 +626,7 @@ def test_checkpoint_metadata_round_trips_calibration_evidence() -> None: empty = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -634,6 +644,7 @@ def test_checkpoint_metadata_round_trips_calibration_evidence() -> None: unrun = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -645,6 +656,7 @@ def test_prepared_slash_columns_are_not_returned_to_the_writer(tmp_path) -> None pytest.importorskip("tables") stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5), ) @@ -826,6 +838,7 @@ def capturing_calibrate(*args, **kwargs): ) stage = UKNationalCalibrationStage( _registry(), + band_edge_registry=_registry(), period=2025, doctrine=UKNationalSolveDoctrine(epochs=5, target_weight_rule=rule), )