Skip to content

Refuse unsigned ids int64 cannot hold; match concept ids as int64 - #1158

Merged
MaxGhenis merged 7 commits into
mainfrom
fix/unsigned-id-int64-wrap
Oct 9, 2026
Merged

MaxGhenis merged 7 commits into
mainfrom
fix/unsigned-id-int64-wrap

Conversation

@MaxGhenis

@MaxGhenis MaxGhenis commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

What was wrong

Two shared helpers on main matched concept-frame ids by casting them to int64. The concept-frame contract accepts unsigned id dtypes: _require_structure and _is_integer_column accept dtype kind "u". The helpers:

  • concepts._check_pointers.positions, which validate_concept_tables uses, cast each pointer column with to_numpy(dtype=np.int64).
  • concept_mapping._Context.person_rows, which ConceptMapping.encode uses for relationship roles, co-resident child counts and reference-person allocation, made the same cast.

A pointer of 2**64 - 1, typed uint64 or UInt64, was therefore read as -1. Take person ids [-1, 11, 12, 13] and partner pointers UInt64 [11, 2**64 - 1, <NA>, <NA>]. validate_concept_tables returned () for person 11's dangling pointer, and encode linked it to person -1. This is the microcosm#1122 review probe. The reverse also happened: with uint64 person ids of 2**63 and above, valid links were reported as dangling and not_member.

Testing found two more id-matching faults in the same functions:

  • Narrow unsigned indexes. pd.Index(ids).get_indexer(targets) casts the targets down to a narrower unsigned index. With uint8 person ids [5, 6], a pointer of 261 matched person 5. validate_concept_tables returned () for a dangling partner, and encode counted a child for person 5. The same held for uint16 and uint32, and for household ids in encode. The nullable UInt8/UInt16/UInt32 dtypes raised a TypeError instead.
  • float64 household membership. _require_structure checked persons' households with isin, which compares int64 with uint64 through float64. A person in household 2**62 + 256 passed as a member of household 2**62. Found by the in-session counterexample review.

The fix

  • concepts._require_int64_ids(person, household, what) raises ValueError if any of the seven id columns present holds an unsigned value above 2**63 - 1. The columns are person_id, person_household_id, partner_person_id, parent_1_person_id, parent_2_person_id, household_id and reference_person_id. The message names every such column, for example Encoding matches ids as int64, and ['person.partner_person_id'] hold unsigned ids above 9223372036854775807, which int64 cannot represent. It runs before anything converts:
    • In validate_concept_tables it runs in _require_structure. That is after the existing check that the three required id columns are non-null integers, and before the uniqueness and household-membership checks, which convert ids too. _check_pointers runs only on frames that passed it.
    • In encode it runs first thing in _Context.__init__.
  • Once wide ids are refused, every integer id converts to int64 exactly, and both helpers match int64 against int64:
    • In _require_structure, uniqueness and membership use int64 indexes (get_indexer, not isin).
    • In _check_pointers, the person-id index is int64. The household-id and person→household comparisons are int64 too. On the locked NumPy 2.4.6 those two casts change nothing, because its mixed-sign comparisons are already exact (an executed equivalent-mutant check). They keep the comparisons exact independent of the NumPy version.
    • In _Context, person ids, household ids and person→household are widened through _matched_ids. Non-integer or null-holding id columns, which are outside the contract, are left exactly as before.
  • _exceeds_int64 / _INT64_MAX have the same bodies as Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's (see the overlap section).

Behaviour changes, scoped:

  • Validation narrows the contract. validate_concept_tables now refuses any frame holding an unsigned id or pointer above 2**63 - 1. That includes valid frames main validated correctly, such as uint64 ids >= 2**63 with no pointer columns. The Raises docstring and the changelog say so.
  • Refusal comes before the structural errors. A wide id is refused by name even in a frame that also has duplicate ids or a person naming no household. The non-null-integer id check still comes first.
  • encode refuses any frame holding such an id, even when no binding resolves a pointer. Before, a uint64 household id above 2**63 - 1 passed through encode untouched. Downstream code holds ids as int64: Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's unit tables, and the Axiom adapter, which already refuses unsigned outputs beyond int64.
  • Fitting ids are unchanged wherever main was exact. For ids that fit int64, the result is unchanged wherever main's matching was exact. See the differential under Evidence.

Invariants (tested)

These hold for every concept frame whose three required id columns (person_id, person_household_id, household_id) are non-null integers, and whose seven id and pointer columns use any of the 16 integer dtypes the property draws: NumPy and nullable, signed and unsigned, 8 to 64 bits. Arrow-backed integers are accepted too, and were probed by hand but are not drawn.

  1. Refusal is exact. validate_concept_tables and encode raise the wide-id ValueError if and only if some unsigned value above 2**63 - 1 is present. The message names exactly those columns, in a fixed order.
  2. Otherwise dtype-invariant. The outcome, either the return value or the ValueError raised, equals the outcome for the same values typed int64 (ids int64, pointers Int64). For validate_concept_tables that means the same violation tuple or the same error. For encode it means the same encoded tables, compared value-exactly with check_dtype=False. This is a metamorphic, dtype-invariance property: the int64 twin goes through the same code.

The property tests are TestIdDtypes in both test files, 300 examples each. They use test_support/microcosm_frame/concept_id_dtypes.py, which does the following:

  • It takes a valid concept_frames() frame and relabels its ids onto edge values: near zero, ±2^k±1 for k ∈ {7, 8, 15, 16, 31, 32, 63, 64}, beyond 2**53, and anywhere in the int64 or uint64 range.
  • It may plant shifted values on every pointer, the household pointer included. A shift is a dtype span of 2^8, 2^16, 2^32 or 2^64, a wrap image; or ±1, which float64 merges above 2**53.
  • It then draws each column's dtype from those that hold its values.
  • typed() builds nullable columns from exact NumPy values plus a mask, and asserts the round trip. pd.array(values, dtype="UInt64") would not be exact: in pandas 3.0.3 it turns [10, 2**63, 2**63 + 1] into [10, 2**63, 2**63]. That made an earlier version of this property fail under about 1 seed in 13.

The encode property runs over the four committed mappings plus a mapping with one binding per pointer-resolving transform: every Role, CoresidentChildCount and AllocateToReferencePerson.

Not claimed:

  • Out-of-contract frames. Frames whose required id columns are non-integer or hold nulls, and object or float id columns, are outside the contract and the invariants. They keep their old matching, with one exception: any unsigned id column holding a value above 2**63 - 1, nullable or not, is refused too, because _require_int64_ids inspects every unsigned column.
  • Other exceptions. The outcome comparison covers return values and ValueError. Pre-existing IndexError / InvalidIndexError cases are outside it, such as an empty person table with households. They are listed as follow-ups below.

Evidence

Run in this worktree with -p no:cacheprovider --basetemp=<worktree>/.hub-scratch/pytest -o tmp_path_retention_policy=failed, one test file per run:

  • New tests against main's two helper modules (files swapped in, tests unchanged):
    • test_concepts.py::TestIdDtypes: 37 of 39 fail.
    • test_concept_mapping.py::TestIdDtypes: 32 of 34 fail.
    • The 2 that pass in each file are the 2**63 - 1 keep-exact boundary, which is meant to pass on main.
    • Both properties failed on main under every seed tried.
  • With the fix, all 73 new TestIdDtypes tests pass, plus the 3 _Units and 4 units_per_household household cases.
    • The validate property passes under seeds 1–8, 31, 33 and 37; the last three failed before the typed() fix. The encode property passes under seeds 1–6, 31 and 33.
    • Both property bodies also pass at 2,000 examples each.
  • Full targeted files pass, each run separately with exit 0: engine_free/shared/test_concepts.py, test_concept_mapping.py and test_axiom_concept_mapping.py, plus engine_free/us/test_concept_mapping.py and engine_free/uk/test_concept_mapping.py.
  • US/UK concept paths that call these helpers, run locally only at the first commit 5cc375e7, with the --extra us and --extra uk venvs:
    • engine_contract/us/test_concept_mapping_policyengine_us.py + engine_scenario/us/test_concept_mapping_policyengine_us.py: 11 passed.
    • engine/uk/test_concept_mapping_policyengine_uk.py: 12 passed, including test_concepts_round_trip_through_the_engine.
    • concept_mapping.py has changed since, in _Units and the shared helpers. CI's engine-us and engine-uk jobs run these files at this head, and this PR is not merged before they pass.
  • Main-vs-fixed differential. I recorded outcomes for 600 deterministic id_typed_frames draws on main and on the fix:
    • For all 506 non-wide frames, fixed(twin) == main(twin), so the fix changes nothing on int64-typed frames.
    • fixed(frame) == main(frame) wherever main was exact (506 of 506 in this sample), and fixed(frame) == fixed(twin). Both hold for validate and encode.
    • All 94 wide frames are refused by name.
  • Mutation, by runtime plugin with no tracked-file edits:
    • Against the first commit, all killed: >= for > in _exceeds_int64; _exceeds_int64 always false; dropping each of the five person-table _ID_COLUMNS entries. The household_id and reference_person_id drops were not run because disk was low; the explicit by-column cases cover them.
    • Against this head: restoring main's float64 isin membership is killed by the explicit regressions every time, and by the property alone under 4 of 8 seeds.
    • Equivalent: leaving own / own_household in native dtype changes nothing on NumPy 2.4.6 (0 differences over 1,000 frames).
  • Hygiene. ruff check is clean, and tools/ci_test_plan.py verify reports ok.
    • Frozen files are untouched: graph/decl.py, kernel.py, uk_runtime/, us_runtime/ and frame/kernels.py.
    • Lines 1–880 of concepts.py are byte-identical to main, so build/nz/benefit_unit_rule.json's concepts.py:880-919 citation does not move.
    • The changelog fragment is changelog.d/concept-id-int64-matching.fixed.md.

Independent review

GPT-6.1 Sol (Subfleet hard tier, run 20261009-001541-mc1158-review-hard-3) reviewed a9d759f7 against b50897f1 and returned APPROVE_WITH_NITS, with no blocking findings.

  • It executed 81 targeted tests, including both 300-example properties.
  • It ran 555 dtype and unit probes.
  • It confirmed that all 78 added or adjusted assertions detect their assigned mutants.

Its nits were inaccurate scope claims in this body, the strategy's "every dtype" wording, the units_per_household match and the pre-existing exceptions. Commit 3aa7386a and this body address them.

An in-session Opus 5.5 reviewer then checked the delta a9d759f7..3aa7386a and returned APPROVE_WITH_NITS.

  • It ran the new regression against main's function: uint8 and UInt8 fail.
  • A 4,000-draw differential across 18 integer dtypes, Arrow included, found that main miscounted 93 draws and raised on 249. The fix was exact on all 4,000.
  • Its nits were a missing test for units_per_household's own wide-id refusal, which a bare-cast mutant survived; the missing Raises section; and wording in this body. Commit b1b8d543 closes them, adding a test that kills that mutant.

The reviewer's re-check of b1b8d543 returned APPROVE_WITH_NITS. It found that a mutant bare-casting only the unit side still survived. Commit d8484852 parametrizes the refusal test over both sides, and each side's bare-cast mutant now fails its own case (executed).

Reconciliation with #1122 (merged 2026-10-09, b50897f1)

#1122 landed first, as agreed with the NZ hub. This branch merged main (065e1b11, no conflicts), and commit a9d759f7 reconciles the two:

  • Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's TestIdRange test. test_an_unsigned_household_id_int64_cannot_hold_is_refused asserted that validation accepts a uint64 household id of 2**63. It now expects validation to refuse the same columns, which is the 4-line change from the comment on Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122.
  • One helper. _INT64_MAX / _exceeds_int64 had identical bodies in concepts.py (this PR) and concept_mapping.py (Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122). They are now defined once, in concepts.py, and concept_mapping and unit_construction import them from there.
  • Unreachable entries removed. _Units' wide-id list dropped its person.person_id, person.person_household_id and person.partner_person_id entries. _Units takes a _Context and is built only in encode_groups, after _Context(person, household), which has already refused those columns. Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's tests for them still pass on _Context's message, which names the same columns.
  • Same narrow-index wrap in _Units. _Units.household_rows matched unit households against the household ids in their native dtype. Under uint8 household ids, a family nesting in household 261 was placed in household 5, and a later check refused it with a misleading message. Under UInt8 it raised a TypeError. It now matches through _matched_ids, as _Context does. New regression: test_group_encode.py::TestRefusals::test_a_narrow_unsigned_household_cannot_catch_a_unit_past_its_range[uint8, UInt8, int64]. The uint8 and UInt8 cases fail on main and pass here; int64 is the control.
  • units_per_household (from the hard review). It reindexed household ids against the unit table's household column in its native dtype. With a uint8 unit column, household 261 counted household 5's unit; under UInt8 it raised a TypeError. Both sides now go through _int64_ids. New regression: test_unit_construction.py::TestIdRange::test_a_narrow_unsigned_unit_household_counts_only_its_own[uint8, UInt8, int64]. The uint8 and UInt8 cases fail on main.
  • NZ hub's report. On the Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122 delta review, item 9: encode_groups accepted a UInt64 household.reference_person_id of 2**64 - 1 when person -1 exists. At this head it is refused, naming ['household.reference_person_id'], as build_benefit_units does (the hub's probe, re-run here).

At a9d759f7, each of these files passes when run on its own (exit 0):

  • Frame tests: test_group_encode, test_unit_construction, test_input_closure, test_concepts, test_concept_mapping, test_axiom_concept_mapping, and the US and UK engine_free/*/test_concept_mapping.
  • Build tests: test_nz_axiom_input_closure, test_nz_spec_package, test_spec_engine_country_bundles, test_country_spec and test_transport_gate_bindings.

Follow-ups found during review, outside the two named helpers, are left for a separate change:

  • Decode's reindex narrowing.
  • encode reading the last household for a person whose household is unknown.
  • person_rows truncating float pointers.
  • validate_concept_tables raising IndexError, instead of reporting not_member, for an empty person table with a household that has a reference_person_id column.
  • Object-dtype pointers holding values >= 2**63, which raise OverflowError in encode.
  • unit_construction._int64_ids raises TypeError, not a labelled ValueError, for an Arrow-backed integer column holding a null. This is out of contract and comes from Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's helper.

axiom: n/a: infra (concept-frame id matching); no policy change.

🤖 Generated with Claude Code

validate_concept_tables (_check_pointers) and ConceptMapping.encode
(_Context.person_rows) matched pointers by casting them to int64. The
concept-frame contract accepts unsigned id dtypes, so a pointer of
2**64 - 1 was read as -1: with a person whose id is -1, a dangling
pointer validated clean and encoded as a link. Both helpers now refuse,
with a ValueError naming every such column, any unsigned id or pointer
above 2**63 - 1 before converting anything.

The same property exposed a second wrap: pandas matches against a
narrower unsigned index by casting the targets down to it, so with uint8
person ids a pointer of 261 named person 5 (and nullable UInt8 ids raised
a TypeError). Both helpers now widen the integer id columns to int64
before matching, which is exact once the wide ids are refused.

Regressions fail on main: 2**63 and 2**64 - 1 as uint64 and UInt64 in
each id and pointer column, the #1122 review probe (2**64 - 1 onto -1),
and narrow uint8/16/32 and UInt8/16/32 ids; 2**63 - 1 is kept exactly.
A Hypothesis property over 16 integer dtypes checks that every frame is
refused exactly when some unsigned value exceeds 2**63 - 1, and otherwise
validates and encodes exactly as the same ids typed int64.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MaxGhenis and others added 6 commits October 8, 2026 22:32
Review of #1158 (in-session counterexample, mutation and claims lenses):

- The committed validate property was flaky. test_support's typed() used
  pd.array(values, dtype="UInt64"), which in pandas 3.0.3 routes some
  lists above 2**63 through float64 ([10, 2**63, 2**63 + 1] becomes
  [10, 2**63, 2**63]), so a drawn frame could hold duplicate ids and fail
  under about 1 seed in 13. typed() now builds nullable columns from an
  exact NumPy array and a mask, and asserts the round trip.
- validate_concept_tables' household membership check compared int64
  with uint64 household ids through pandas' float64 isin, so a person
  naming household 2**62 + 256 passed as a member of household 2**62. The
  refusal of wide ids moves into _require_structure, ahead of the
  uniqueness and membership checks, which now run on int64; a wide id is
  refused by name even in an otherwise malformed frame.
- The id strategy now draws ids beyond 2**53, plants shifts of one, and
  plants household pointers in both properties; both properties compare
  raised errors too. New regressions cover the float64 merge and the
  refusal ordering; all fail on main.
- Docstrings: encode's Raises section, _matched_ids' summary, the
  strategy's plant options. Adds a changelog fragment.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
After merging main (with #1122):

- test_unit_construction: TestIdRange's unsigned-household case asserted
  validate_concept_tables accepts a uint64 household id of 2**63, which
  this PR refuses; it now expects validation to refuse the same columns
  (the 4-line change agreed with the NZ hub).
- _INT64_MAX / _exceeds_int64 were defined twice with the same bodies,
  once in concepts (this PR) and once in concept_mapping (#1122);
  concept_mapping and unit_construction now import them from concepts.
- _Units' wide-id list dropped its person.* entries: _Units is only
  built from a _Context, which has already refused them, so they could
  not fire.
- _Units matched unit households against the household ids in their
  native dtype, so under uint8 household ids a family nesting in
  household 261 was placed in household 5 (UInt8 raised a TypeError).
  It now matches through _matched_ids, as _Context does; a regression
  covers uint8, UInt8 and the int64 control.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the hard-tier review of a9d759f (APPROVE_WITH_NITS):

- units_per_household reindexed household ids against the unit table's
  household column in its native dtype, so with a uint8 unit column
  household 261 counted household 5's unit (UInt8 raised a TypeError).
  Both sides now go through _int64_ids, as the rest of the module does;
  a regression covers uint8, UInt8 and the int64 control.
- concept_id_dtypes: the strategy draws the NumPy and pandas nullable
  integer dtypes; the docstrings no longer claim every accepted dtype
  (Arrow-backed integers are accepted but not drawn).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the delta review of 3aa7386: a mutant casting both sides with a
bare to_numpy(dtype=np.int64) survived every test, so a uint64 household
id of 2**64 - 1 would have counted the unit in household -1. A test now
requires the refusal. units_per_household's docstring gains a Raises
section, and the strategy's UNSIGNED_64 comment notes Arrow's uint64.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From the delta re-check of b1b8d54: a mutant bare-casting only the unit
table's household column survived. The refusal test is now parametrized
over which side holds 2**64 - 1; each side's bare-cast mutant fails its
own case.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@MaxGhenis
MaxGhenis merged commit c38141e into main Oct 9, 2026
10 checks passed
@MaxGhenis

Copy link
Copy Markdown
Contributor Author

Merged at head d8484852 with a merge commit (c38141ec) under the CLAUDE.md merge gates. The gates at merge time:

  • CI: gh pr checks exited 0 on run 37894699156 at d8484852. All 10 jobs passed: engine-free 3.13/3.14, engine-us 3.13/3.14, engine-uk 3.13/3.14, integration-uk, lint, wheels and select-countries.
  • Mergeability: MERGEABLE, not a draft, no CHANGES_REQUESTED review. main is unprotected; the merge was pinned with --match-head-commit.
  • Independent review, run in rounds:
    • GPT-6.1 Sol, Subfleet hard tier, run 20261009-001541-mc1158-review-hard-3, at a9d759f7: APPROVE_WITH_NITS, no blocking findings. It executed 81 targeted tests and 555 dtype/unit probes, and confirmed the 78 added or adjusted assertions catch their mutants.
    • An Opus 5.5 delta reviewer at 3aa7386a, then again at b1b8d543: APPROVE_WITH_NITS both times, no blocking findings.
    • Every nit was addressed. The final commit d8484852 is test-only: it parametrizes units_per_household's wide-id refusal over both sides, and each side's bare-cast mutant was executed and fails its own case.
  • Coordination with Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122: Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122 landed first, as agreed with the NZ hub, who also agreed to the three reconciliation changes to Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122's code.
  • No call needed from Max: this is a bug fix with no published numbers.

Follow-ups outside this PR are queued as a separate task:

  • decode's reindex narrowing;
  • encode reading the last household for an unknown household;
  • float and object pointers;
  • the empty-persons IndexError;
  • Arrow nulls in _int64_ids.

Evidence: ~/reviews/microcosm-unsigned-id-int64-wrap-2026-10-08/.

@MaxGhenis
MaxGhenis deleted the fix/unsigned-id-int64-wrap branch October 9, 2026 07:33
MaxGhenis added a commit that referenced this pull request Oct 9, 2026
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.

1 participant