Hash object-dtype storage by content, not PyObject pointers - #909
Merged
Conversation
Prepend the lane section to the cumulative root journal; prior lanes remain below unchanged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two independently constructed equal object-dtype Series currently produce different _storage_parts value bytes because the fallback branch serializes PyObject pointers. The new cases pin content stability, leaf-type injectivity, null-bitmap separation, selection-mask handling, and a fail-closed refusal for unsupported leaves; they also pin the masked, numeric, and StringDtype encodings so the fix cannot move them. Red run: 24 failed, 20 passed, 46 deselected; direct exit 1. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_storage_parts fell through to np.ascontiguousarray(...).tobytes() for any series whose materialized numpy array is object dtype, so it serialized pointer addresses: equal content rebuilt in a second array hashed differently, and a persisted or cross-reconstruction storage comparison could never agree. Object arrays now take a length-prefixed, type-tagged content encoding modelled on the StringDtype branch. bool is tested before int (it is an int subclass), floats are packed from their exact IEEE-754 bits so -0.0 and NaN payloads survive, and any leaf outside str/bytes/int/float/bool/None raises PopulationError with the static code storage-object-leaf rather than being repr()'d. The masked, numeric, and StringDtype encodings are untouched and byte-identical. Gating on the materialized array dtype rather than series.dtype also covers extension dtypes that materialize as object arrays. 44 passed, 46 deselected; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The graph package already defines exactly one object-leaf encoding, in store._encode_object_scalar, and it is what ContentStore writes and reads back. Encoding storage bytes with a second, parallel vocabulary in population.py would have left a column unequal to its own persisted-and- reloaded self for pd.NA, pd.NaT and NumPy scalars, which the store accepts and _storage_parts would have refused. Import the canonical encoder instead, per docs/shared-constants.md, and translate its TypeError into PopulationError under the static code storage-object-leaf. Tests now pin that the object values bytes are exactly the store framing, that None/pd.NA/pd.NaT/NaN stay distinguishable despite sharing a null bitmap, and that a real ContentStore frame round trip leaves an object column storage-equal to the original. 58 passed, 46 deselected; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Object and string leaves now compare by content, but masked storage still compares the bytes under the null mask, where pandas leaves whatever the construction route wrote. Two content-equal Int64 columns built by different routes therefore still differ, verified by a new regression. Record that in the docstring so a caller does not mistake a digest folded from these parts for a cross-reconstruction content identity, and add the towncrier fragment. 105 passed in test_graph_population.py; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
A pre-fix/post-fix byte diff across a 20-dtype matrix shows the change is byte-identical for every dense, masked, string, datetime64, timedelta64, sparse and arrow-backed column, and moves bytes only where the old code was hashing addresses: object, categorical, DatetimeTZ, Period and Interval. Categorical now compares category values, which is what storage equality should mean. DatetimeTZ, Period and Interval carry leaves outside the ContentStore's object vocabulary and now raise instead of comparing addresses. That is the consistent outcome: store.py already refuses to persist those dtypes and token_for_dtype refuses to declare them. Pinned by regressions so the decision is reviewable rather than incidental. 109 passed in test_graph_population.py; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two independent adversarial reviews found the same defect: the encoder declines a leaf in two ways, not one. A lone-surrogate str makes store._encode_object_scalar raise UnicodeEncodeError from str.encode, which escaped the TypeError-only guard, so storage_equal raised something other than PopulationError and the storage-object-leaf contract did not hold for that input. Catch both. Also pin the one case where the refusal is new rather than sharper: an object column holding a leaf outside the ContentStore vocabulary compared against itself. Positional copies preserve PyObject identity, so the old pointer comparison answered True there regardless of content; it now refuses, matching ContentStore (which will not persist such a column) and token_for_dtype (which will not declare it). The storage_equal docstring now states the leaf resolution explicitly — NumPy floating compares at float64 width, NumPy integer by value — because that is what the store decodes back, and it is what makes the round trip a fixed point. 111 passed in test_graph_population.py; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The blast-radius review is right that two behaviors were implicit. Both are now stated where the claim lives and pinned by regressions. storage_equal's docstring says outright that for object columns it is no longer a total predicate: a leaf outside the ContentStore vocabulary raises PopulationError. _object_storage_values says that a NumPy scalar and its Python counterpart deliberately do NOT stay distinct, because the store's decoder hands back the Python form — refusing that would mean a column could never equal its own persisted-and-reloaded self, which is the defect. 119 passed in test_graph_population.py; direct exit 0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The review is right that "pin it on a frame identity" is incomplete advice: a frame identity covers the frame, while a population pin also needs the version, owners, weight kinds, mass ledger and design weights. Name them, and say that "in-process seal" describes what the digest can identify rather than where it may be stored — the value can legitimately be carried in a receipt and re-verified in the same process. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
MaxGhenis
added a commit
that referenced
this pull request
Sep 11, 2026
…mark the stamp as in-process Two attested identities moved with the merge ref, both re-pinned to the values the merged tree computes: the UK country bundle (main's #903 added the frs_relationships spine stage on top of this branch's attested kernel changes, so neither side's pin survives the merge), and the multispine pool tool's constants-adapter country_spec digest, which had already moved to the branch's attested value recorded in docs/evidence/spec-engine/ us-f0-coverage.json when the seed identities were re-pinned but was still asserted at main's value in test_us_multispine_pool_tool.py. survey_atomic_geography._population_stamp now says it is an in-process seal that must never be persisted or compared across reconstructions (microcosm#907 / #909), naming what a cross-call pin has to carry instead. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…amp-object-storage # Conflicts: # packages/microcosm-graph/tests/test_graph_population.py
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
… not The reviewer read "the single definition of an object leaf's bytes in this package" against executor._update_scalar, which keeps its own tagged leaf vocabulary (with a repr() fallback) for the kernel-context digest. The docstring now names that second codec and says storage hashing deliberately does not share it. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ger branch np.timedelta64 subclasses np.signedinteger at runtime, so the store's object-scalar codec encoded a timedelta whose unit int() accepts (ns, M, Y) as the bare integer: np.timedelta64(1, "ns") and 1 produced identical bytes, storage_equal called them equal, and a persisted leaf came back as int 1. The codec now refuses np.timedelta64 and np.datetime64 by type ahead of the integer branch, storage hashing inherits the refusal as a PopulationError, and the regressions pin the refusal for the units int() converts rather than relying on the day unit's incidental int() failure. Co-Authored-By: Claude Fable 5.1 <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
1 participant
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.
Fixes #907.
microcosm.graph.population._storage_partsfell through tonp.ascontiguousarray(values).tobytes()for any series whose materialized array is object dtype, so it serialized PyObject addresses: two independently constructed, equal object-dtype series (the strata axis in the US graph frames) never hashed alike, and every_population_stampthat folded those parts in changed across reconstructions. Every in-repo use compared the same objects inside one call, so nothing failed in the repository; the #893 native pilot harness, which pinned geography stages across an export pass and a final verification pass, was the first cross-reconstruction consumer and failed after a complete run.What changed
microcosm.graph.store._encode_object_scalar) rather than a second hand-written encoding, with a fail-closed refusal for leaves the codec cannot encode (including a lone-surrogatestr, found by an adversarial pass). Dense, masked, string, datetime64, timedelta64, sparse and arrow columns are byte-identical to before; bytes move only for object,Categorical(str)and the other object-backed extension dtypes listed in the tests.storage_equaldoes and does not seal is documented and pinned.packages/microcosm-graph/tests/test_graph_population.py): equal content from fresh objects hashes alike, different content differs, null handling matches the existingStringDtypebranch, the refusal fires for unencodable leaves, and a 20-dtype byte-identity table for the untouched branches. Red first against the pre-fix source (34 failed), green after.changelog.d/907-object-storage-hashing.fixed.md.Not in this PR
survey_atomic_geography._population_stamp(in-process seal; cross-call pins must use_frame_identity) is blocked here because that module exists only on the Consolidate the US survey graph build and enrichment pipeline #893 integration branch; it goes there.test_release_target_parity.py::TestRegenerationis red on this branch's base commit (twoLedgerHierarchyMetadataErrorfailures about Chronicle fact dimension labels), verified identical onorigin/main— pre-existing, untouched.Verification (direct exit codes)
Shards frame (310), graph (409), fit (100), calibrate (255), data (519) green; the 578-test graph-adjacent set green;
ruff check .,tools/ci_test_groups.py --verify,tools/spec_engine_coverage.py --check(42156/42156) clean;decl.py,kernel.py,docs/graph-interface.lock, everytest_acceptance_*,uv.lock, spec pins and evidence JSON untouched. Full lane report: an Opus build lane on a fresh worktree offorigin/main; the graph shard was re-run by the coordinating session before this push (rc 0).🤖 Generated with Claude Code