From e53ce3dff4259fbcbeec77ca7981ea5b8d2fa146 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 18 Oct 2026 14:30:55 -0400 Subject: [PATCH 1/4] Bind the ACS local artifact to the code that wrote it Follow-up to #1309's review (GPT-6.1 Sol, approve with minors): - calibrate records its code identity in consumer_export.json; package refuses to name a release after any other commit, a dirty writer without --allow-dirty, or a checkpoint that records no writer. - The interim WIC limitation states whom the assumption covers (the people without a stored decision, all on the ACS spine), and the receipt records how many people were allowed to lack one. - _booleans accepts only bools and int/float 0/1, as policyengine.py's reader does (a Fraction is refused). - consumer_ready's gate summary carries the live-input receipt and the writer's code. - Tests: every stored Boolean dtype (pandas BooleanDtype with NA, numpy bool, float 0/1/NaN, object), an unreadable or absent spine tag refuses rather than fills, the writer binding, a mask built from person_spine; the spine-blindness entry is in order with an exact comment. Co-Authored-By: Claude Opus 5.5 --- ...s-local-artifact-writer-binding.changed.md | 1 + .../build/us_runtime/retired_stored_inputs.py | 9 +- .../us/test_us_acs_local_release_tool.py | 92 ++++++++++++++++++- .../us/test_us_retired_stored_inputs.py | 53 +++++++++-- .../engine_free/us/test_us_spine_blindness.py | 9 +- tools/build_us_acs_local_release.py | 46 +++++++++- 6 files changed, 189 insertions(+), 21 deletions(-) create mode 100644 changelog.d/1263-us-acs-local-artifact-writer-binding.changed.md diff --git a/changelog.d/1263-us-acs-local-artifact-writer-binding.changed.md b/changelog.d/1263-us-acs-local-artifact-writer-binding.changed.md new file mode 100644 index 000000000..94bc3629c --- /dev/null +++ b/changelog.d/1263-us-acs-local-artifact-writer-binding.changed.md @@ -0,0 +1 @@ +The ACS local-area release's package stage now refuses to name a release after code that did not write its calibrated artifact: calibrate records its code identity in `consumer_export.json`, and package requires the same commit. The interim WIC limitation now states exactly whom the assumption covers, and its receipt records how many people were allowed to lack a decision. diff --git a/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py b/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py index 203d5dbb2..bb1e1c4b1 100644 --- a/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py +++ b/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py @@ -37,7 +37,6 @@ from collections.abc import Collection from dataclasses import dataclass -from numbers import Real from typing import Any import numpy as np @@ -179,6 +178,7 @@ def _live_wic( "stored_decisions_kept": int(present.sum()), "stored_decisions_true": int(decisions[present].sum()), "people_assumed_true": int(assumed.sum()), + "people_allowed_without_decision": int(allowed.sum()), "rule": ACS_WIC_ASSUMPTION, "status": "interim", "flip_condition": ACS_WIC_FLIP_CONDITION, @@ -192,10 +192,9 @@ def _booleans(values: pd.Series) -> np.ndarray: for position, value in enumerate(values.to_numpy(dtype=object)): if isinstance(value, (bool, np.bool_)): out[position] = bool(value) - elif ( - isinstance(value, Real) - and not isinstance(value, (bool, np.bool_)) - and value in (0, 1) + elif isinstance(value, (int, float, np.integer, np.floating)) and value in ( + 0, + 1, ): out[position] = bool(value) else: diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py index 946a79696..28086ec92 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py @@ -118,6 +118,12 @@ def _spm_measurement_gate_passes(monkeypatch): ) +def _writer_code(module) -> dict: + """This checkout's code identity, as the calibrate stage records it.""" + + return module._repo_code_identity(allow_dirty=True) + + def _load_tool_module(): root = _TEST_PATHS.repository path = root / "tools" / "build_us_acs_local_release.py" @@ -332,6 +338,7 @@ def limitation(consumer_export): "from": "would_claim_wic", "to": "takes_up_wic_if_eligible", "people_assumed_true": 12, + "people_allowed_without_decision": 20, "flip_condition": "Drop it once microcosm#1156 lands.", } } @@ -340,6 +347,10 @@ def limitation(consumer_export): assert entry["status"] == "reviewed_interim_assumption" assert entry["column"] == "takes_up_wic_if_eligible" assert entry["people_assumed_true"] == 12 + # The text claims only what the rule did: the people without a decision. + assert ( + "every person without one (12, all on the ACS spine, of 20" in entry["reason"] + ) assert entry["treatment"] == "Drop it once microcosm#1156 lands." assert entry["calibration_blocker"] is False assert limitation(None) is None @@ -400,6 +411,71 @@ def test_only_acs_spine_people_may_lack_a_wic_decision() -> None: ) +@pytest.mark.parametrize( + "spine", + [np.nan, None, b"acs_2024_1yr", "ACS_2024_1YR"], + ids=["nan", "none", "bytes", "other-label"], +) +def test_an_unreadable_spine_tag_refuses_rather_than_fills(spine) -> None: + """A person whose tag is not exactly the ACS spine may not lack one.""" + + module = _load_tool_module() + frame = _spine_frame(["acs_2024_1yr", "acs_2024_1yr"], [None, None]) + person = frame.table("person") + person["person_spine"] = pd.Series(["acs_2024_1yr", spine], dtype=object) + + with pytest.raises(SystemExit, match="did not allow"): + module._with_live_stored_inputs(frame) + + +def test_a_person_table_without_the_spine_tag_is_refused() -> None: + from microcosm.frame import Frame + + module = _load_tool_module() + frame = _spine_frame(["acs_2024_1yr"], [None]) + tables = {entity: frame.table(entity) for entity in frame.entities} + tables["person"] = tables["person"].drop(columns="person_spine") + untagged = Frame( + tables, + frame.schema, + {entity: frame.weights_for(entity) for entity in frame.weighted_entities}, + ) + + with pytest.raises(SystemExit, match="person_spine"): + module._with_live_stored_inputs(untagged) + + +@pytest.mark.parametrize( + ("written_by", "allow_dirty", "match"), + [ + (None, True, "records no code identity"), + ({"sha": ""}, True, "records no code identity"), + ({"sha": "0000000", "dirty": False}, True, "written by code 0000000"), + ({"sha": "HEAD", "dirty": True}, False, "dirty tree"), + ], + ids=["absent", "empty", "other-commit", "dirty-writer"], +) +def test_package_names_the_release_only_after_the_code_that_wrote_it( + written_by, allow_dirty, match +) -> None: + """The release id names the packaging code, so that code must have + written the calibrated bytes (review of microcosm#1309).""" + + module = _load_tool_module() + code = {"sha": "HEAD", "dirty": False, "branch": "main"} + export = {} if written_by is None else {"code": written_by} + + with pytest.raises(SystemExit, match=match): + module._require_artifact_written_by(export, code, allow_dirty=allow_dirty) + + module._require_artifact_written_by( + {"code": {"sha": "HEAD", "dirty": False}}, code, allow_dirty=False + ) + module._require_artifact_written_by( + {"code": {"sha": "HEAD", "dirty": True}}, code, allow_dirty=True + ) + + def test_parse_args_enforces_stage_requirements(tmp_path: Path) -> None: module = _load_tool_module() base = [ @@ -588,7 +664,10 @@ def _package_evidence_args(module, tmp_path: Path, monkeypatch, *, hours_report) "artifact_sha256": artifact_sha, "per_spine": {}, }, - "consumer_export.json": {"staging_sha256": module._sha256(staging)}, + "consumer_export.json": { + "staging_sha256": module._sha256(staging), + "code": _writer_code(module), + }, "held_back_columns.json": {"total": 0}, "reviewed_null_fills.json": {"columns_filled": []}, "materialize_rss.json": { @@ -1401,7 +1480,10 @@ def test_finalize_report_round_trips_into_package(tmp_path, monkeypatch) -> None "artifact_sha256": artifact_sha, "per_spine": {}, }, - "consumer_export.json": {"staging_sha256": artifact_sha}, + "consumer_export.json": { + "staging_sha256": artifact_sha, + "code": _writer_code(module), + }, "consumer_reviewed_null_fills.json": {"columns_filled": []}, } for name, value in evidence.items(): @@ -1472,6 +1554,7 @@ def test_a_sparse_era_calibration_is_bound_through_finalize_and_package( "run_identity_sha256": stamp, "out_h5_sha256": artifact_sha, "weights_sha256": "w", + "code": _writer_code(module), } qa = { "plain_consumption": True, @@ -1559,7 +1642,10 @@ def _package_args_with_hours(module, tmp_path, monkeypatch, *, gate_state): "artifact_sha256": artifact_sha, "per_spine": {}, }, - "consumer_export.json": {"staging_sha256": artifact_sha}, + "consumer_export.json": { + "staging_sha256": artifact_sha, + "code": _writer_code(module), + }, "consumer_reviewed_null_fills.json": {"columns_filled": []}, } for name, value in evidence.items(): diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py b/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py index 62fdd90cd..c09b20b64 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py @@ -1,8 +1,10 @@ """Moving the donor's retired stored inputs to the live ones (microcosm#1263). Every frame here is invented. The engine is a variable-name set, so no -policyengine-us is imported; ``test_us_retired_stored_inputs_engine`` checks -the names against the installed engine. +policyengine-us is imported. The installed engine's side of the premise (the +live input is a Person Boolean defaulting to True, and neither retired name is +defined) is pinned by +``packages/microcosm-data/tests/engine_contract/us/test_stored_inputs.py``. """ from __future__ import annotations @@ -32,13 +34,11 @@ def _remedy(frame, engine=ENGINE, allowed=None): - """The rule with the release tool's mask: the ACS people may lack one.""" + """The rule with the release tool's mask: the ACS-spine people may lack one.""" if allowed is None: - channels = frame.table("person")["person_support_channel"].map( - lambda value: value.decode() if isinstance(value, bytes) else value - ) - allowed = channels.eq(ACS_CHANNEL).to_numpy(dtype=bool) + spine = frame.table("person")["person_spine"] + allowed = spine.eq(ACS_CHANNEL).fillna(False).to_numpy(dtype=bool) return with_live_stored_inputs( frame, engine_variables=engine, may_lack_decision=allowed ) @@ -50,6 +50,9 @@ def _frame(channels, decisions=None, part_b=None) -> Frame: frame = acs_frame([[(20, 40.0)] for _ in channels]) person = frame.table("person").copy() person["person_support_channel"] = channels + person["person_spine"] = [ + ACS_CHANNEL if channel == ACS_CHANNEL else "asec_puf" for channel in channels + ] if decisions is not None: person[RETIRED_WIC_INPUT] = pd.Series(decisions, dtype=object) if part_b is not None: @@ -80,6 +83,7 @@ def test_donor_decisions_are_kept_and_acs_gaps_take_wic_up(): assert wic["stored_decisions_kept"] == 2 assert wic["stored_decisions_true"] == 1 assert wic["people_assumed_true"] == 2 + assert wic["people_allowed_without_decision"] == 2 assert wic["status"] == "interim" assert "microcosm#1156" in wic["flip_condition"] @@ -166,6 +170,41 @@ def test_it_refuses_rather_than_guessing(channels, decisions, match): _remedy(_frame(channels, decisions)) +@pytest.mark.parametrize( + ("stored", "expected"), + [ + (pd.array([True, False, None], dtype="boolean"), [True, False, True]), + (np.array([True, False, True]), [True, False, True]), + (np.array([1.0, 0.0, np.nan]), [True, False, True]), + (pd.Series([True, False, None], dtype=object), [True, False, True]), + ], + ids=["pandas-boolean-NA", "numpy-bool", "float-0-1-nan", "object"], +) +def test_every_stored_boolean_dtype_reads_alike(stored, expected): + frame = _frame(["asec", "puf_tax_detail", ACS_CHANNEL]) + tables = {entity: frame.table(entity) for entity in frame.entities} + tables["person"] = tables["person"].assign(**{RETIRED_WIC_INPUT: stored}) + typed = Frame( + tables, + frame.schema, + {entity: frame.weights_for(entity) for entity in frame.weighted_entities}, + ) + + person = _remedy(typed).frame.table("person") + + assert person[LIVE_WIC_INPUT].dtype == bool + assert person[LIVE_WIC_INPUT].tolist() == expected + + +def test_a_fraction_is_not_read_as_a_decision(): + """Only bools and int/float 0/1, as policyengine.py's reader accepts.""" + + from fractions import Fraction + + with pytest.raises(ValueError, match="not a Boolean"): + _remedy(_frame(["asec"], [Fraction(1)])) + + def test_zero_one_numbers_are_read_as_decisions(): frame = _frame(["asec", "asec", "asec"], [1, 0.0, np.float64(1.0)]) diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_spine_blindness.py b/packages/microcosm-build/tests/engine_free/us/test_us_spine_blindness.py index 77338f117..83b372021 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_spine_blindness.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_spine_blindness.py @@ -307,6 +307,11 @@ "release_gate_preflight.py", "release_input_coverage.py", "release_target_parity.py", + # Consumer-artifact remedy for two retired stored inputs (microcosm#1263): + # renames the WIC draw (keeping stored decisions and defaulting missing + # ones to True) and drops a dead column. It reads no provenance column; + # the release tool passes the people who may lack a decision. + "retired_stored_inputs.py", "scf_auto_loans.py", "scf_wealth.py", "sipp_financial_assets.py", @@ -342,10 +347,6 @@ "spm_universe_source.py", # Reviewed held-out SSI/Social Security diagnostic; no population treatment. "ssi_social_security_holdout.py", - # Consumer-artifact remedy for two retired stored inputs (microcosm#1263): - # renames the WIC draw and drops a dead column. It reads no provenance - # column; the release tool passes the people who may lack a decision. - "retired_stored_inputs.py", "stacked_spine.py", # Provenance owner (#578 revision); see owners list. "support_provenance.py", "take_up.py", diff --git a/tools/build_us_acs_local_release.py b/tools/build_us_acs_local_release.py index ebe36a549..084dcec60 100644 --- a/tools/build_us_acs_local_release.py +++ b/tools/build_us_acs_local_release.py @@ -2341,6 +2341,9 @@ def _write_calibrated_artifact( "filled_columns": len(fills), "total_values_filled": sum(f["filled_rows"] for f in fills), "live_stored_inputs": live_inputs.provenance, + # The code that wrote these bytes. Package names the release + # after its own code and refuses any other writer. + "code": _repo_code_identity(allow_dirty=True), "note": ( "The engine-pass contract (input-schema projection + " "reviewed-null default fill) is applied to the published " @@ -2487,6 +2490,40 @@ def do_qa(args) -> None: ) +def _require_artifact_written_by( + consumer_export: dict, code: dict, *, allow_dirty: bool +) -> None: + """Refuse to name a release after code that did not write its artifact. + + The calibrate stage writes the published bytes and records its code in + ``consumer_export.json``; the release id names the packaging code. They + must be the same commit, so the id names the code that produced the + artifact. A checkpoint that records no writer is refused, not trusted. + """ + + written_by = consumer_export.get("code") + if not isinstance(written_by, dict) or not written_by.get("sha"): + raise SystemExit( + "consumer_export.json records no code identity for the calibrated " + "H5, so the release id cannot be shown to name the code that wrote " + "it. Re-run --stage calibrate (then qa and finalize) with the " + "current tool." + ) + if written_by["sha"] != code["sha"]: + raise SystemExit( + f"The calibrated H5 was written by code {written_by['sha']}, but " + f"packaging runs {code['sha']} and would name the release after " + "it. Package with the code that wrote the artifact, or re-run " + "--stage calibrate (then qa and finalize) with this code." + ) + if written_by.get("dirty") and not allow_dirty: + raise SystemExit( + "The calibrated H5 was written from a dirty tree; a packaged " + "release names a committed code vintage. Re-run --stage calibrate " + "from a clean tree, or pass --allow-dirty." + ) + + def _repo_code_identity(allow_dirty: bool) -> dict[str, object]: def _git(*parts: str) -> str: return subprocess.check_output( @@ -2635,8 +2672,10 @@ def _acs_wic_takeup_limitation(consumer_export: dict | None) -> dict | None: "people, and the donor stores its decisions under the retired " f"input name {wic.get('from')!r}, which the installed " "policyengine-us ignores. The consumer artifact stores them as " - f"{wic.get('to')!r}: every donor decision is kept, and every ACS " - "person is set to take WIC up when eligible. That is the engine's " + f"{wic.get('to')!r}: every stored decision is kept, and every " + f"person without one ({assumed}, all on the ACS spine, of " + f"{wic.get('people_allowed_without_decision')} ACS-spine people) " + "is set to take WIC up when eligible. That is the engine's " "default and the assumption policyengine.py applies to the same " "cells today; it is a modelling assumption, not observed " "participation." @@ -3098,6 +3137,8 @@ def do_finalize(args) -> None: "filled_columns", "total_values_filled", "staging_sha256", + "live_stored_inputs", + "code", ) } if consumer_export @@ -3411,6 +3452,7 @@ def do_package(args) -> dict: ) code = _repo_code_identity(args.allow_dirty) + _require_artifact_written_by(consumer_export, code, allow_dirty=args.allow_dirty) timestamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") release_id = f"{RELEASE_ID_PREFIX}-{code['sha']}-{timestamp}" release_dir = args.out / "releases" / release_id From 6fc902a54c5241bb2d1448b66983db38d32f6b80 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 18 Oct 2026 14:34:13 -0400 Subject: [PATCH 2/4] Compare full commits when binding the artifact to its writer The abbreviated sha's length depends on the checkout: Modal's shallow clone abbreviates to 7 characters, a full clone to 9 or more, so comparing short shas could refuse an artifact written by the same commit. Record and compare the full commit; the release id keeps the abbreviated sha. Co-Authored-By: Claude Opus 5.5 --- .../us/test_us_acs_local_release_tool.py | 27 ++++++++++++++----- tools/build_us_acs_local_release.py | 18 +++++++++---- 2 files changed, 33 insertions(+), 12 deletions(-) diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py index 28086ec92..2f4acd154 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py @@ -449,11 +449,12 @@ def test_a_person_table_without_the_spine_tag_is_refused() -> None: ("written_by", "allow_dirty", "match"), [ (None, True, "records no code identity"), - ({"sha": ""}, True, "records no code identity"), - ({"sha": "0000000", "dirty": False}, True, "written by code 0000000"), - ({"sha": "HEAD", "dirty": True}, False, "dirty tree"), + ({"sha": "abc1234"}, True, "records no code identity"), + ({"commit": ""}, True, "records no code identity"), + ({"commit": "0" * 40, "dirty": False}, True, "written by code 0000000"), + ({"commit": "a" * 40, "dirty": True}, False, "dirty tree"), ], - ids=["absent", "empty", "other-commit", "dirty-writer"], + ids=["absent", "short-sha-only", "empty", "other-commit", "dirty-writer"], ) def test_package_names_the_release_only_after_the_code_that_wrote_it( written_by, allow_dirty, match @@ -462,20 +463,32 @@ def test_package_names_the_release_only_after_the_code_that_wrote_it( written the calibrated bytes (review of microcosm#1309).""" module = _load_tool_module() - code = {"sha": "HEAD", "dirty": False, "branch": "main"} + code = {"sha": "aaaaaaaaa", "commit": "a" * 40, "dirty": False, "branch": "main"} export = {} if written_by is None else {"code": written_by} with pytest.raises(SystemExit, match=match): module._require_artifact_written_by(export, code, allow_dirty=allow_dirty) + # The abbreviated sha may differ in length between checkouts; only the + # full commit is compared. module._require_artifact_written_by( - {"code": {"sha": "HEAD", "dirty": False}}, code, allow_dirty=False + {"code": {"sha": "aaaaaaa", "commit": "a" * 40, "dirty": False}}, + code, + allow_dirty=False, ) module._require_artifact_written_by( - {"code": {"sha": "HEAD", "dirty": True}}, code, allow_dirty=True + {"code": {"commit": "a" * 40, "dirty": True}}, code, allow_dirty=True ) +def test_the_recorded_code_identity_carries_the_full_commit() -> None: + module = _load_tool_module() + code = module._repo_code_identity(allow_dirty=True) + + assert len(code["commit"]) == 40 + assert code["commit"].startswith(code["sha"]) + + def test_parse_args_enforces_stage_requirements(tmp_path: Path) -> None: module = _load_tool_module() base = [ diff --git a/tools/build_us_acs_local_release.py b/tools/build_us_acs_local_release.py index 084dcec60..272cb26c7 100644 --- a/tools/build_us_acs_local_release.py +++ b/tools/build_us_acs_local_release.py @@ -2502,17 +2502,19 @@ def _require_artifact_written_by( """ written_by = consumer_export.get("code") - if not isinstance(written_by, dict) or not written_by.get("sha"): + if not isinstance(written_by, dict) or not written_by.get("commit"): raise SystemExit( "consumer_export.json records no code identity for the calibrated " "H5, so the release id cannot be shown to name the code that wrote " "it. Re-run --stage calibrate (then qa and finalize) with the " "current tool." ) - if written_by["sha"] != code["sha"]: + # Full commits: the abbreviated sha's length depends on the checkout (a + # shallow clone abbreviates shorter), so it cannot be compared. + if written_by["commit"] != code["commit"]: raise SystemExit( - f"The calibrated H5 was written by code {written_by['sha']}, but " - f"packaging runs {code['sha']} and would name the release after " + f"The calibrated H5 was written by code {written_by['commit']}, but " + f"packaging runs {code['commit']} and would name the release after " "it. Package with the code that wrote the artifact, or re-run " "--stage calibrate (then qa and finalize) with this code." ) @@ -2531,6 +2533,7 @@ def _git(*parts: str) -> str: ).strip() sha = _git("rev-parse", "--short", "HEAD") + commit = _git("rev-parse", "HEAD") dirty = bool(_git("status", "--porcelain")) if dirty and not allow_dirty: raise SystemExit( @@ -2538,7 +2541,12 @@ def _git(*parts: str) -> str: "committed code vintage. Commit first or pass --allow-dirty " "(the manifest will record dirty=true)." ) - return {"sha": sha, "dirty": dirty, "branch": _git("branch", "--show-current")} + return { + "sha": sha, + "commit": commit, + "dirty": dirty, + "branch": _git("branch", "--show-current"), + } def _ess_concentration_limitation(diagnostics: dict) -> dict: From 30556c70d12621836ecc2fe4e7f45bfe1b6e4ccf Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 18 Oct 2026 14:46:20 -0400 Subject: [PATCH 3/4] Record a dirty writer in the release and test the binding end to end Review of #1311 (GPT-6.1 Sol r1, approve): - F1: build_dirty is true when either the packaging tree or the tree that wrote the artifact was dirty; _require_artifact_written_by returns the writer's flag. - F2: do_package refuses another writer before any release directory exists, and a dirty writer marks a clean-packaged release dirty. - F3: the spine-tag test builds its frame with the tag instead of editing a Frame's stored table. - F4: nullable Int64/Float64 with NA and int8/uint8 read alike; Decimal and complex are refused, and the docstring no longer overstates parity with policyengine.py (this rule is stricter about complex). - F5: the Modal runbook says package runs at the commit calibrate ran at. Co-Authored-By: Claude Opus 5.5 --- docs/us-modal-stage-runbook.md | 7 ++++ .../build/us_runtime/retired_stored_inputs.py | 2 +- .../us/test_us_acs_local_release_tool.py | 36 +++++++++++++++++-- .../us/test_us_retired_stored_inputs.py | 25 ++++++++++--- tools/build_us_acs_local_release.py | 17 ++++++--- 5 files changed, 75 insertions(+), 12 deletions(-) diff --git a/docs/us-modal-stage-runbook.md b/docs/us-modal-stage-runbook.md index 378656d60..0bc135019 100644 --- a/docs/us-modal-stage-runbook.md +++ b/docs/us-modal-stage-runbook.md @@ -263,6 +263,13 @@ one stage of a run at a time, because two concurrent stages would race on the same state directory. The attempt ledger enforces this, best effort (see "Preemption and restarts"). +For `us-acs-local-release`, run package at the commit calibrate ran at. +Calibrate records the code that wrote the calibrated H5 in +`consumer_export.json`, and package refuses to name a release after any +other commit (microcosm#1263). A commit landing between the two means +re-running calibrate, qa and finalize at the new commit, or packaging at the +old one. + For a gated or private Hub input, set `MICROCOSM_MODAL_HF_SECRET` to the name of a Modal secret that holds `HF_TOKEN`, for example `huggingface-token` in the `policyengine` workspace. Otherwise no secret is attached. Only the diff --git a/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py b/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py index bb1e1c4b1..be0de764f 100644 --- a/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py +++ b/packages/microcosm-build/src/microcosm/build/us_runtime/retired_stored_inputs.py @@ -186,7 +186,7 @@ def _live_wic( def _booleans(values: pd.Series) -> np.ndarray: - """Stored decisions as Booleans: bools, or real numbers equal to 0 or 1.""" + """Stored decisions as Booleans: bools, or int or float numbers equal to 0 or 1.""" out = np.empty(len(values), dtype=bool) for position, value in enumerate(values.to_numpy(dtype=object)): diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py index 2f4acd154..f2f40534a 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py @@ -420,9 +420,7 @@ def test_an_unreadable_spine_tag_refuses_rather_than_fills(spine) -> None: """A person whose tag is not exactly the ACS spine may not lack one.""" module = _load_tool_module() - frame = _spine_frame(["acs_2024_1yr", "acs_2024_1yr"], [None, None]) - person = frame.table("person") - person["person_spine"] = pd.Series(["acs_2024_1yr", spine], dtype=object) + frame = _spine_frame(["acs_2024_1yr", spine], [None, None]) with pytest.raises(SystemExit, match="did not allow"): module._with_live_stored_inputs(frame) @@ -1698,6 +1696,38 @@ def test_package_accepts_passing_hours_gate_bound_to_the_packaged_bytes( assert gate["artifact_sha256"] == result["root_artifact"]["sha256"] == copied_sha +def _set_writer(args, **changes) -> None: + path = args.checkpoint_dir / "consumer_export.json" + export = json.loads(path.read_text()) + export["code"] = {**export["code"], **changes} + path.write_text(json.dumps(export)) + + +def test_package_refuses_another_writer_before_any_release_dir(tmp_path, monkeypatch): + """The writer binding runs before the release directory exists.""" + + module = _load_tool_module() + args = _package_args_with_hours(module, tmp_path, monkeypatch, gate_state="passed") + _set_writer(args, commit="0" * 40) + + with pytest.raises(SystemExit, match="written by code 0000000"): + module.do_package(args) + assert not (args.out / "releases").exists() + + +def test_a_dirty_writer_marks_the_release_dirty(tmp_path, monkeypatch): + """A clean packaging tree cannot hide a dirty writer (review of #1311).""" + + module = _load_tool_module() + args = _package_args_with_hours(module, tmp_path, monkeypatch, gate_state="passed") + _set_writer(args, dirty=True) + + release_dir = Path(module.do_package(args)["release_dir"]) + + build = json.loads((release_dir / "build_manifest.json").read_text()) + assert build["build_dirty"] is True + + @requires_pytables def test_package_records_the_stored_input_gate_bound_to_the_packaged_bytes( tmp_path, monkeypatch diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py b/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py index c09b20b64..e05bacfda 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_retired_stored_inputs.py @@ -177,8 +177,21 @@ def test_it_refuses_rather_than_guessing(channels, decisions, match): (np.array([True, False, True]), [True, False, True]), (np.array([1.0, 0.0, np.nan]), [True, False, True]), (pd.Series([True, False, None], dtype=object), [True, False, True]), + (pd.array([1, 0, None], dtype="Int64"), [True, False, True]), + (pd.array([1.0, 0.0, None], dtype="Float64"), [True, False, True]), + (np.array([1, 0, 1], dtype=np.int8), [True, False, True]), + (np.array([1, 0, 1], dtype=np.uint8), [True, False, True]), + ], + ids=[ + "pandas-boolean-NA", + "numpy-bool", + "float-0-1-nan", + "object", + "nullable-Int64-NA", + "nullable-Float64-NA", + "int8", + "uint8", ], - ids=["pandas-boolean-NA", "numpy-bool", "float-0-1-nan", "object"], ) def test_every_stored_boolean_dtype_reads_alike(stored, expected): frame = _frame(["asec", "puf_tax_detail", ACS_CHANNEL]) @@ -196,13 +209,17 @@ def test_every_stored_boolean_dtype_reads_alike(stored, expected): assert person[LIVE_WIC_INPUT].tolist() == expected -def test_a_fraction_is_not_read_as_a_decision(): - """Only bools and int/float 0/1, as policyengine.py's reader accepts.""" +@pytest.mark.parametrize("value", ["fraction", "decimal", "complex"]) +def test_only_bools_and_real_zero_or_one_are_decisions(value): + """Bools and int/float 0/1 only. policyengine.py's reader refuses a + Fraction and a Decimal too; this rule is stricter about complex.""" + from decimal import Decimal from fractions import Fraction + stored = {"fraction": Fraction(1), "decimal": Decimal(1), "complex": 1 + 0j} with pytest.raises(ValueError, match="not a Boolean"): - _remedy(_frame(["asec"], [Fraction(1)])) + _remedy(_frame(["asec"], [stored[value]])) def test_zero_one_numbers_are_read_as_decisions(): diff --git a/tools/build_us_acs_local_release.py b/tools/build_us_acs_local_release.py index 272cb26c7..223a5eea1 100644 --- a/tools/build_us_acs_local_release.py +++ b/tools/build_us_acs_local_release.py @@ -2492,13 +2492,16 @@ def do_qa(args) -> None: def _require_artifact_written_by( consumer_export: dict, code: dict, *, allow_dirty: bool -) -> None: +) -> bool: """Refuse to name a release after code that did not write its artifact. The calibrate stage writes the published bytes and records its code in ``consumer_export.json``; the release id names the packaging code. They must be the same commit, so the id names the code that produced the artifact. A checkpoint that records no writer is refused, not trusted. + + Returns whether the writer's tree was dirty, so the release can record + it even when packaging runs from a clean tree. """ written_by = consumer_export.get("code") @@ -2518,12 +2521,14 @@ def _require_artifact_written_by( "it. Package with the code that wrote the artifact, or re-run " "--stage calibrate (then qa and finalize) with this code." ) - if written_by.get("dirty") and not allow_dirty: + writer_dirty = bool(written_by.get("dirty")) + if writer_dirty and not allow_dirty: raise SystemExit( "The calibrated H5 was written from a dirty tree; a packaged " "release names a committed code vintage. Re-run --stage calibrate " "from a clean tree, or pass --allow-dirty." ) + return writer_dirty def _repo_code_identity(allow_dirty: bool) -> dict[str, object]: @@ -3460,7 +3465,9 @@ def do_package(args) -> dict: ) code = _repo_code_identity(args.allow_dirty) - _require_artifact_written_by(consumer_export, code, allow_dirty=args.allow_dirty) + writer_dirty = _require_artifact_written_by( + consumer_export, code, allow_dirty=args.allow_dirty + ) timestamp = datetime.now(UTC).strftime("%Y%m%dT%H%M%SZ") release_id = f"{RELEASE_ID_PREFIX}-{code['sha']}-{timestamp}" release_dir = args.out / "releases" / release_id @@ -3572,7 +3579,9 @@ def _version(package: str) -> str: build_manifest = { "build_id": release_id, "build_sha": code["sha"], - "build_dirty": code["dirty"], + # Dirty if either the packaging tree or the tree that wrote the + # artifact was. + "build_dirty": bool(code["dirty"]) or writer_dirty, "created_at": datetime.now(UTC).isoformat(), "code": {"branch": code["branch"], "repo": "microcosm"}, "runtime": { From b388570c118b554ee4e30526b2d2d39a9f1f7e56 Mon Sep 17 00:00:00 2001 From: Max Ghenis Date: Sun, 18 Oct 2026 14:53:50 -0400 Subject: [PATCH 4/4] Skip the writer tests without PyTables; pin a clean packaging tree Review of #1311 round 2 (approve): the two new do_package tests write H5 and need the requires_pytables marker, and the dirty-writer test now patches a clean packaging identity so it cannot pass on a dirty checkout without the fix. Co-Authored-By: Claude Opus 5.5 --- .../tests/engine_free/us/test_us_acs_local_release_tool.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py index f2f40534a..6fd804e12 100644 --- a/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py +++ b/packages/microcosm-build/tests/engine_free/us/test_us_acs_local_release_tool.py @@ -1703,6 +1703,7 @@ def _set_writer(args, **changes) -> None: path.write_text(json.dumps(export)) +@requires_pytables def test_package_refuses_another_writer_before_any_release_dir(tmp_path, monkeypatch): """The writer binding runs before the release directory exists.""" @@ -1715,10 +1716,15 @@ def test_package_refuses_another_writer_before_any_release_dir(tmp_path, monkeyp assert not (args.out / "releases").exists() +@requires_pytables def test_a_dirty_writer_marks_the_release_dirty(tmp_path, monkeypatch): """A clean packaging tree cannot hide a dirty writer (review of #1311).""" module = _load_tool_module() + # A clean packaging tree, whatever this checkout's state, so only the + # writer's flag can make the release dirty. + clean = {**module._repo_code_identity(allow_dirty=True), "dirty": False} + monkeypatch.setattr(module, "_repo_code_identity", lambda allow_dirty: dict(clean)) args = _package_args_with_hours(module, tmp_path, monkeypatch, gate_state="passed") _set_writer(args, dirty=True)