Skip to content

Give a person with no household no household's row, and read malformed holdout rows alike in both guards - #1481

Merged
MaxGhenis merged 3 commits into
mainfrom
nzhub/f1-null-ids-holdout-rows
Nov 3, 2026
Merged

MaxGhenis merged 3 commits into
mainfrom
nzhub/f1-null-ids-holdout-rows

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Tracking issue #1183, follows #1472 and #1477. This makes shared code handle two kinds of malformed input safely: a person whose household pointer finds no household, and a holdout row whose measure isn't text. Valid frames and specs keep every output.

Scope of the harm. This change adds consistency and defence in depth. It fixes no live bug in a production run. validate_concept_tables (F/concepts.py:1403) runs where F/transport.py builds the transport frame (:593 and :876). It refuses a null or unknown person household id (_require_structure) and reports a reference person who isn't a member (_check_pointers). ConceptMapping.encode doesn't run that validator, and #1477 already gave encode a rule for a null household id in _coresident_adult_flags. The rest of _Context now follows the same rule.

F/ is packages/microcosm-frame/src/microcosm/frame/ and T/ is packages/microcosm-build/src/microcosm/build/transport/. Line numbers refer to this PR's head.

Authors. A cloud session wrote the first two commits from the NZ hub's brief 208, then stopped at its account's limit. The NZ hub adopted the PR on 2026-11-03 (#1183 comment 6600936673), ran the checks below, and added 9b49ef30, a test for the gap guard's dedupe.

Shared hunks

F/concept_mapping.py (the fix 1 hunks, 17 lines)

  • :2096-2098, _Context.__init__: a person whose household id is null gets row -1, even where the household table holds a null id of its own. get_indexer already gives -1 for an unknown id. pd.NA finds pd.NA in get_indexer, so this closes review 6595462928's "Fixes owed 3".
  • :2121-2125, household_amount_for_persons: it appends a NaN to the amounts before it gathers, so row -1 reads NaN, never the last household's amount. This works on an empty household table too.
  • :2127-2136, reference_rows: it appends -1 the same way, so a person with no household has reference row -1.
  • Nothing else changes. _matched_ids, _coresident_adult_flags and the group path (_Units) stay as on main, and encode still refuses no null or unknown pointer.

T/compose.py (the fix 3 and fix 4 hunks, 38 lines)

  • :1010-1016, new _is_holdout: one predicate, an object whose measure is text. Both guards now read holdouts through it. It matches main's old activation test (:1027 on main).
  • :1019-1049, _holdout_separation_gaps (fix 4): the guard yields one gap for each holdout_references row that isn't an object with a text measure, naming the row by index and name. It still skips such a row as a holdout. seen moves up so the guard reports each malformed row once, even when the same document arrives in two views.
  • :3015-3021, _precalibration_stages (fix 3): the stage guard keeps only _is_holdout rows and builds holdout_columns only from rows whose entity is text as well.
  • :3134, _precalibration_stages' refusal: it reads the entity with .get('entity', '?').

test_support/microcosm_frame/concept_reference.py (fix 2)

  • :57-78: household_of gives None for a null or unknown household id, and the household lookup skips a household row whose own id is null. value gives NaN and reference_of gives None for a person with no household.
  • :121-124: allocation gives 0 unless the person is their own household's reference person.

Invariants

  • I1, no household. A person whose household id is null or names no household reads no household's row. Their reference row is -1. No role flags them as a reference person or as a reference person's partner, and AllocateToReferencePerson gives them 0.
  • I2, row order. Permuting the household table's rows leaves every person's encoded values unchanged. Permuting person rows permutes those values alike.
  • I3, conservation. For each household whose reference person is one of its members, the members' AllocateToReferencePerson values sum to the household amount.
  • I4, agreement. Where every person household id names a household, encode gives exactly what main gives.
  • I5, guards agree. For any holdout rows:
    • the stage guard and activation's guard treat the same set of measures as holdouts;
    • neither guard raises (activation's guard reports gaps instead); validate_transport_activation still raises TypeError on a holdout row that isn't an object, or on a list or dict name, as main does;
    • activation reports exactly one gap per malformed holdout row.

Tests, mutants and packaged counts

The NZ hub ran these at 85efd107 on Python 3.14.7, one process at a time (-n 0), with each run's Hypothesis database outside the tree and NZ_CHRONICLE_ARTIFACT pointing at the NZ feed. CI passed all 10 checks at 85efd107.

run result time peak RSS
test_concept_mapping.py, test_group_encode.py, test_axiom_concept_mapping.py 359 passed 69 s 1.45 GB
test_transport_holdout_separation.py 50 passed 173 s 1.80 GB
test_transport_composed_graph.py -k "holdout or precalibration" 252 passed, 217 deselected 62 s 1.61 GB
test_nz_axiom_input_closure.py, test_nz_transport_declarations.py 50 passed, 2 skipped 150 s 1.51 GB

The two skips (test_nz_axiom_input_closure.py:535 and :610) need axiom_rules_engine with its dense extension and a rulespec-nz checkout at POPULACE_RULESPEC_NZ. ruff check and ruff format --check pass on the five changed files, and tools/ci_test_plan.py verify prints verification=ok.

Mutants. Each mutant undoes one hunk, or all of one fix's hunks. The hub ran each against the test file in its row, then restored the tree; git status came back empty after the last one.

mutant what it undoes test file tests red
M1 fix 1, all three hunks test_concept_mapping.py 18
M1a __init__'s -1 for a null household id same 3
M1b the NaN that household_amount_for_persons appends same 6
M1c the -1 that reference_rows appends same 18
M2 fix 2 (main's concept_reference.py) same 1
M3 fix 3, all three hunks test_transport_holdout_separation.py 6
M3a the stage guard's _is_holdout filter same 5
M3b the text-entity filter on holdout_columns same 2
M3c the refusal's .get('entity', '?') same 2
M4 fix 4's gap for a malformed row same 5
M4b the seen check on those gaps same 0

The tests each mutant turned red:

  • M1 and M1c: TestExecution::test_encode_matches_a_row_by_row_reference and 17 TestNoHousehold cases (allocation ×4, empty household table, I1, I2, reference row ×4, null household id ×2, partner flag ×4).
  • M1a: I1 and the null household id cases (×2).
  • M1b: the empty household table, I1 and the reference row cases (×4).
  • M2: test_encode_matches_a_row_by_row_reference, the differential against the reference.
  • M3: I5 and test_stage_guard_reads_a_malformed_holdout_row_as_activation_does (×5). M3a: I5 and four of those cases. M3b and M3c: I5 and one case each.
  • M4: I5 and test_activation_reports_a_malformed_holdout_row (×4).
  • M4b survived, because no test passed a holdout document in more than one view. The hub added test_guard_names_a_malformed_holdout_row_once_across_views in 9b49ef30. It passes one document with a malformed row in two views and expects one gap. F1's review (brief 210 v2) ran M4b at this head: only that test went red, out of the file's 51 (Activate the NZ graph against pinned Chronicle facts (NZ graph G8) #1183 comment 6614494025). CI passed all 10 checks at 9b49ef30 (run 41625995645).

Packaged counts (f1_packaged_counts.py at 85efd107).

  • The transport tests use three toy populations: the shared toy country's frame (12 persons, 6 households), the synthetic donor after split_for_transport (6 and 2), and NZ's forty-household toy frame (120 and 40). None has a null person household id, a person household id that names no household, or a null household id.
  • Of the five packaged specs, Belgium's declares 7 holdout rows and NZ's 6; the am, uk and us specs declare no holdout_references. Every declared row is an object with a text measure and a text entity.

So, by the hunks above, fix 1 changes no output on these frames, and fixes 3 and 4 change no outcome on the packaged specs.

The changed old test. test_non_text_measure_keeps_activation_outcome drops holdout_references from its resource parameter. Fix 4 now reports a holdout row with a non-text measure, so activation's outcome there changes by design. test_activation_reports_a_malformed_holdout_row covers that row, with list and dict measures among its cases.

Kernel hashes

make_composed_fixture installs 45 kernels. The hub hashed every one with registry.kernels.implementation_hash(ref) at F1's base on main (3995d92d) and at 85efd107, in separate processes, along with the three simulate.* kernels under AxiomEngine. Eight of the 45 moved, the same eight and the same values as the table below, and none under AxiomEngine moved. 9b49ef30 changes only a test file.

The cloud author attributed each move by hashing main with one changed source file at a time from this head. F/concept_mapping.py alone moves all eight, and T/compose.py or concept_reference.py alone moves none, so no kernel's hash reads T/compose.py. A second run at main repeated every hash.

kernel main head moved by
concepts.encode@1 c8f5382d8c08 822329c928b8 F/concept_mapping.py
concepts.encode_groups@1 6ad001e08144 ece137685e14 F/concept_mapping.py
transport.scenario_override@1 6ad001e08144 ece137685e14 F/concept_mapping.py
transport.unit_attributes@1 a3f4a1f0936a ad2f490e82d8 F/concept_mapping.py
transport.create@1 cd21a414f12a 2b93a3b26ccb F/concept_mapping.py
transport.create@2 cd21a414f12a 2b93a3b26ccb F/concept_mapping.py
transport.input_closure@1 6d0e24f02cfa 1c330fecfb73 F/concept_mapping.py
gates.battery@1 c11c40ddc996 6a0ddf4457da F/concept_mapping.py

These are the eight kernels that hash F/concept_mapping.py, as review 6595462928 listed.

US primary-QRF worker identity

F1 moves it. F/concept_mapping.py sits in the worker's static import closure, through the US adapter (F/adapters/policyengine_us.py:46 imports ConceptMapping); the shortest path runs puf_qrf_worker → microcosm.build.us_runtime → us_runtime.other_health_insurance → frame.adapters.policyengine_us → frame.concept_mapping [read; executed: brief 217]. The identity hashes each closure module's raw file bytes (packages/microcosm-build/src/microcosm/build/us_runtime/worker_identity.py:580-632), so docstring edits count too [read: brief 217].

  • primary_qrf_worker_execution_binding() at 3995d92d and at 9b49ef30 differs in three fields: semantic_identity.worker_module.transitive_imports_sha256, semantic_identity.transitive_environment_code_sha256 and semantic_identity_sha256. The worker's own source, uv.lock, the installed distributions, the interpreter and the environment agree [executed: brief 217].
  • A tree at 3995d92d that takes only F1's F/concept_mapping.py gives 9b49ef30's binding byte for byte, and 9b49ef30 with 3995d92d's copy of that file gives 3995d92d's. So that file accounts for the whole move [executed: brief 217, Activate the NZ graph against pinned Chronicle facts (NZ graph G8) #1183 comment 6613701891, with all four trees run from one worktree path, since the binding's audit_aliases record the venv path; Belgium's hub found the same].
  • A US run that recorded its pool under the old identity refuses it under the new one: packages/microcosm-build/src/microcosm/build/us_runtime/stacked_spine.py:5681-5694 raises "late primary-QRF semantic worker identity changed" [read], and the equality check returns False for 3995d92d's recorded binding against 9b49ef30's live one [executed: brief 217]. Runs that carry a legacy worker attestation take the other branch (:5672-5679), which compares the recorded worker with the attested one, so F1 doesn't touch them [read: brief 217].
  • Route A's booked releases pin main 9434c504b, so F1 doesn't reach them; a US run built from a main with F1 gets the new identity. Main already moved this identity after 9434c504b: Read US catalog units from engine source and retire the unit waiver #1484 changed F/adapters/policyengine_us.py, which sits in the same closure [inferred from the closure above]. Encode person_role bindings and their closure defaults on the person table (S2b PR 2) #1480 moves the identity through F/concept_mapping.py too, so whichever of the two lands second moves it again [inferred: brief 217].

Reviews

🤖 Generated with Claude Code

ConceptMapping.encode runs no frame validator, so a person whose household
id is null or names no household can reach _Context. get_indexer gave such
a person row -1, and the gathers in reference_rows and
household_amount_for_persons read -1 as the household table's last row. A
null person household id also found a household row whose own id was null.

_Context now gives a null household id row -1 whatever the household table
holds, and both gathers read row -1 as an appended sentinel: reference row
-1 and amount NaN. So allocation gives such a person 0, and no role flags
them as a reference person or as a reference person's partner. Frames whose
person household ids all name a household encode as before.

The row-by-row reference applies the same rule instead of raising
KeyError, and the differential test now draws null and unknown person
household ids. New example tests cover the probe cases on Int64 and UInt64
ids, a null id in the household table and an empty household table, and
Hypothesis properties check the no-household, row-order and conservation
invariants.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eCgZkFeeyUZcCZLNXec1k
The stage guard in _precalibration_stages read row["entity"] and
row["measure"] from every holdout_references row, so a list or dict
measure raised TypeError and a missing measure or entity raised KeyError.
Activation's guard skipped a row that was not an object with a text
measure, so such a row dropped silently out of the separation check.

One predicate, _is_holdout, now defines a holdout row for both guards. The
stage guard skips any other row, builds holdout_columns only from rows
whose entity is text, and names a missing entity as "?". Activation's
guard reports each row it cannot read as one gap that names the row, so a
declared holdout never drops silently out of both guards.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_014eCgZkFeeyUZcCZLNXec1k
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

NZ hub adopts F1 (#1481). The farness cloud session (brief 208) pushed 85efd107 at 06:09Z and then stopped at the account's weekly limit, with the body's test section still open and no hand-off. Its code covers all four fixes, and CI passed all 10 checks at 85efd107. The hub now runs what the brief still owes (each fix's mutants, the packaged-input counts and ci_test_plan.py verify), fills the body's test section and then files the review. If the cloud session resumes, it should stop and leave the branch to the hub.

The NZ hub's mutant M4b drops the seen check in
_holdout_separation_gaps, so the guard yields every gap it builds, and
all 50 tests in test_transport_holdout_separation.py stayed green: no
test passed a holdout document in more than one view. The new test
passes one document with a malformed row in two views and expects one
gap.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis marked this pull request as ready for review November 3, 2026 20:32
@MaxGhenis
MaxGhenis merged commit 815147d into main Nov 3, 2026
10 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged by the NZ hub (session local_e407a527) at 9b49ef3 with --squash. Gates: gh pr checks exit 0; MERGEABLE; not draft; no CHANGES_REQUESTED review. Reviewed head: 9b49ef3. Reviews: brief 210 v2 (cloud, Opus 5.5) APPROVE_WITH_NITS at 9b49ef3 (#1183 comment 6614494025), with both nits fixed in the body; brief 217 CONFIRMED the US primary-QRF identity move (#1183 comment 6613701891); Belgium's hub cleared the shared hunks at 9b49ef3. CI: all 10 checks green at 9b49ef3 (run 41625995645). 210 v2 also ran the head merged with main 31c8939: same results as the head.

@MaxGhenis
MaxGhenis deleted the nzhub/f1-null-ids-holdout-rows branch November 3, 2026 20:33
anth-volk added a commit that referenced this pull request Nov 3, 2026
Brings in #1495 (main's engine-free count test) and #1481.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Nov 3, 2026
Brings in the engine-free count test fix from #1495 and #1481's holdout
row reading. No file overlaps this branch's changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Nov 3, 2026
Brings in #1460, #1481, #1495 and #1496.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Nov 3, 2026
…-token

Brings in main's #1495 and #1481 through #1465. No file overlaps this
branch's changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Nov 3, 2026
Brings in #1495 (the binding-identity count fix), #1460, #1481 and #1496, so CI can run every engine-free test past the count test that main failed on.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis pushed a commit that referenced this pull request Nov 3, 2026
Brings in main at 79a533c: #1495, #1481 and #1460, which landed while
the 31c8939 merge was in CI preparation. The one conflict is the
transport battery's module count: #1495 counts #1342's two modules in
BINDING_MODULES_ADDED_AFTER_BASE, so this keeps main's form and the branch's
base of 198 (us_runtime.scf_education_debt, b1218b0) in place of the
hand count 200 from the previous commit.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
anth-volk added a commit that referenced this pull request Nov 3, 2026
…ams-telemetry

Brings in main's #1495 and #1481 through #1465 and #1473. No file
overlaps this branch's changes.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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.

2 participants