Repository navigation
Refuse unsigned ids int64 cannot hold; match concept ids as int64 - #1158
Merged
Merged
Conversation
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>
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>
Contributor
Author
|
Merged at head
Follow-ups outside this PR are queued as a separate task:
Evidence: |
MaxGhenis
added a commit
that referenced
this pull request
Oct 9, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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_structureand_is_integer_columnaccept dtype kind"u". The helpers:concepts._check_pointers.positions, whichvalidate_concept_tablesuses, cast each pointer column withto_numpy(dtype=np.int64).concept_mapping._Context.person_rows, whichConceptMapping.encodeuses 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 pointersUInt64 [11, 2**64 - 1, <NA>, <NA>].validate_concept_tablesreturned()for person 11's dangling pointer, andencodelinked it to person -1. This is the microcosm#1122 review probe. The reverse also happened: with uint64 person ids of2**63and above, valid links were reported asdanglingandnot_member.Testing found two more id-matching faults in the same functions:
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_tablesreturned()for a dangling partner, andencodecounted a child for person 5. The same held for uint16 and uint32, and for household ids inencode. The nullableUInt8/UInt16/UInt32dtypes raised aTypeErrorinstead._require_structurechecked persons' households withisin, which compares int64 with uint64 through float64. A person in household2**62 + 256passed as a member of household2**62. Found by the in-session counterexample review.The fix
concepts._require_int64_ids(person, household, what)raisesValueErrorif any of the seven id columns present holds an unsigned value above2**63 - 1. The columns areperson_id,person_household_id,partner_person_id,parent_1_person_id,parent_2_person_id,household_idandreference_person_id. The message names every such column, for exampleEncoding matches ids as int64, and ['person.partner_person_id'] hold unsigned ids above 9223372036854775807, which int64 cannot represent.It runs before anything converts:validate_concept_tablesit 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_pointersruns only on frames that passed it.encodeit runs first thing in_Context.__init__._require_structure, uniqueness and membership use int64 indexes (get_indexer, notisin)._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._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_MAXhave 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:
validate_concept_tablesnow refuses any frame holding an unsigned id or pointer above2**63 - 1. That includes valid frames main validated correctly, such as uint64 ids>= 2**63with no pointer columns. TheRaisesdocstring and the changelog say so.frame/transport.py, from Add donor transport operators: reader, stable seeds, currency bridge and quantile map (NZ v0, G3b) #1130. It validates the US donor bank's ids (line 545) and decoded concepts (line 809).transport's own tests,test_transport_donor.pyandtest_transport_quantile.py, pass at this head. None of them exercise the wide-id refusal: a no-op refusal passes them too, as the delta review executed.encode_groupsmeets the refusal through_Context.encoderefuses any frame holding such an id, even when no binding resolves a pointer. Before, a uint64 household id above2**63 - 1passed throughencodeuntouched. 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.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.validate_concept_tablesandencoderaise the wide-idValueErrorif and only if some unsigned value above2**63 - 1is present. The message names exactly those columns, in a fixed order.ValueErrorraised, equals the outcome for the same values typed int64 (idsint64, pointersInt64). Forvalidate_concept_tablesthat means the same violation tuple or the same error. Forencodeit means the same encoded tables, compared value-exactly withcheck_dtype=False. This is a metamorphic, dtype-invariance property: the int64 twin goes through the same code.The property tests are
TestIdDtypesin both test files, 300 examples each. They usetest_support/microcosm_frame/concept_id_dtypes.py, which does the following: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}, beyond2**53, and anywhere in the int64 or uint64 range.2**53.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
encodeproperty runs over the four committed mappings plus a mapping with one binding per pointer-resolving transform: everyRole,CoresidentChildCountandAllocateToReferencePerson.Not claimed:
2**63 - 1, nullable or not, is refused too, because_require_int64_idsinspects every unsigned column.ValueError. Pre-existingIndexError/InvalidIndexErrorcases 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:test_concepts.py::TestIdDtypes: 37 of 39 fail.test_concept_mapping.py::TestIdDtypes: 32 of 34 fail.2**63 - 1keep-exact boundary, which is meant to pass on main.TestIdDtypestests pass, plus the 3_Unitsand 4units_per_householdhousehold cases.typed()fix. The encode property passes under seeds 1–6, 31 and 33.engine_free/shared/test_concepts.py,test_concept_mapping.pyandtest_axiom_concept_mapping.py, plusengine_free/us/test_concept_mapping.pyandengine_free/uk/test_concept_mapping.py.5cc375e7, with the--extra usand--extra ukvenvs: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, includingtest_concepts_round_trip_through_the_engine.concept_mapping.pyhas changed since, in_Unitsand the shared helpers. CI'sengine-usandengine-ukjobs run these files at this head, and this PR is not merged before they pass.id_typed_framesdraws on main and on the fix: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), andfixed(frame) == fixed(twin). Both hold for validate and encode.>=for>in_exceeds_int64;_exceeds_int64always false; dropping each of the five person-table_ID_COLUMNSentries. Thehousehold_idandreference_person_iddrops were not run because disk was low; the explicit by-column cases cover them.isinmembership is killed by the explicit regressions every time, and by the property alone under 4 of 8 seeds.own/own_householdin native dtype changes nothing on NumPy 2.4.6 (0 differences over 1,000 frames).ruff checkis clean, andtools/ci_test_plan.py verifyreports ok.graph/decl.py,kernel.py,uk_runtime/,us_runtime/andframe/kernels.py.concepts.pyare byte-identical to main, sobuild/nz/benefit_unit_rule.json'sconcepts.py:880-919citation does not move.changelog.d/concept-id-int64-matching.fixed.md.Independent review
GPT-6.1 Sol (Subfleet hard tier, run
20261009-001541-mc1158-review-hard-3) revieweda9d759f7againstb50897f1and returned APPROVE_WITH_NITS, with no blocking findings.Its nits were inaccurate scope claims in this body, the strategy's "every dtype" wording, the
units_per_householdmatch and the pre-existing exceptions. Commit3aa7386aand this body address them.An in-session Opus 5.5 reviewer then checked the delta
a9d759f7..3aa7386aand returned APPROVE_WITH_NITS.uint8andUInt8fail.units_per_household's own wide-id refusal, which a bare-cast mutant survived; the missingRaisessection; and wording in this body. Commitb1b8d543closes them, adding a test that kills that mutant.The reviewer's re-check of
b1b8d543returned APPROVE_WITH_NITS. It found that a mutant bare-casting only the unit side still survived. Commitd8484852parametrizes 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 commita9d759f7reconciles the two:TestIdRangetest.test_an_unsigned_household_id_int64_cannot_hold_is_refusedasserted that validation accepts a uint64 household id of2**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._INT64_MAX/_exceeds_int64had identical bodies inconcepts.py(this PR) andconcept_mapping.py(Build benefit units from concept pointers and execute group encoding (NZ v0, G4) #1122). They are now defined once, inconcepts.py, andconcept_mappingandunit_constructionimport them from there._Units' wide-id list dropped itsperson.person_id,person.person_household_idandperson.partner_person_identries._Unitstakes a_Contextand is built only inencode_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._Units._Units.household_rowsmatched 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. UnderUInt8it raised aTypeError. It now matches through_matched_ids, as_Contextdoes. New regression:test_group_encode.py::TestRefusals::test_a_narrow_unsigned_household_cannot_catch_a_unit_past_its_range[uint8, UInt8, int64]. Theuint8andUInt8cases fail on main and pass here;int64is 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; underUInt8it raised aTypeError. 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]. Theuint8andUInt8cases fail on main.encode_groupsaccepted a UInt64household.reference_person_idof2**64 - 1when person -1 exists. At this head it is refused, naming['household.reference_person_id'], asbuild_benefit_unitsdoes (the hub's probe, re-run here).At
a9d759f7, each of these files passes when run on its own (exit 0):test_group_encode,test_unit_construction,test_input_closure,test_concepts,test_concept_mapping,test_axiom_concept_mapping, and the US and UKengine_free/*/test_concept_mapping.test_nz_axiom_input_closure,test_nz_spec_package,test_spec_engine_country_bundles,test_country_specandtest_transport_gate_bindings.Follow-ups found during review, outside the two named helpers, are left for a separate change:
reindexnarrowing.encodereading the last household for a person whose household is unknown.person_rowstruncating float pointers.validate_concept_tablesraisingIndexError, instead of reportingnot_member, for an empty person table with a household that has areference_person_idcolumn.>= 2**63, which raiseOverflowErrorinencode.unit_construction._int64_idsraisesTypeError, not a labelledValueError, 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