fix: return writable numpy/Series reads and surface DataFrame/Series corruption clearly#191
Conversation
…corruption clearly #157: np.frombuffer reconstruction returned read-only arrays aliasing the source buffer (the L1-cached bytes on a hit); mutating a cached value crashed or risked corrupting the entry. .copy() at all four sites (top-level numpy, nested numpy, numeric DataFrame column, numeric Series) makes reads writable and self-owned. #156: the DataFrame/Series branches swallowed a checksum mismatch from retrieve() in a broad except and re-parsed the corrupt bytes as raw msgpack, losing the diagnostic. A retrieve() failure now raises a clear SerializationError (mirrors StandardSerializer); unpack/build moved outside the guard so post-retrieve errors surface as themselves. Integrity-off path unchanged. Closes #156. Closes #157.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughFixes two deserialization correctness defects in ChangesAutoSerializer deserialization correctness
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Summary
Two robustness fixes in
AutoSerializer's deserialize path (the integrity batch from triage).Closes #157. Closes #156.
#157 — cached numpy / Series reads were read-only and aliased the buffer
Reconstruction via
np.frombuffer(...)returns a read-only array that aliases the source bytes — on an L1 hit, the live cached buffer. A caller mutating a cached value then hitValueError: assignment destination is read-onlywhere an uncached call would not, and mutation-by-view risked corrupting the cache entry (DATA-IS-SACRED adjacency).Fix:
.copy()at all four reconstruction sites (top-level numpy, nested numpy in msgpack, numeric DataFrame column, numeric Series) so reads are writable and own their data. The copy is required for correctness, not gratuitous — it restores the same contract an uncached return value has.#156 — DataFrame/Series corruption was swallowed, losing the diagnostic
The dataframe/series branches wrapped
ByteStorage.retrieve()in a broadexcept Exception, logged at DEBUG, and fell through to re-parsing the corrupt bytes as raw msgpack. A genuine xxHash3 checksum mismatch (surfaced from Rust asValueError) was therefore swallowed — the system still failed closed (recompute), but the corruption signal was lost and a confusingTypeErrorwas logged instead.Fix: a
retrieve()failure now raises a clearSerializationError("…corrupted cache entry…"), mirroringStandardSerializer. The unpack/build step moved outside the guard so a genuine post-retrieve error surfaces as itself rather than being mistaken for corruption. The integrity-off (direct msgpack) path is unchanged.Notes
ArrowSerializerwhen pyarrow is installed, so the columnar msgpack path (the"dataframe"branch + its frombuffer site) is reached only without pyarrow; Series always use the columnar path. Tests cover both (the no-pyarrow path via a disabled arrow serializer).exceptshape for other formats — that's the shared path tracked under the AutoSerializer DataFrame/Series deserialize swallows checksum-mismatch (degraded corruption diagnostic) #156 umbrella and needs the Rust binding to distinguish corruption from format errors; this PR fixes the live, metadata-carrying DataFrame/Series branches.Tests
New
tests/unit/test_auto_serializer_mutation_and_corruption.py: writability + non-aliasing for numpy/nested/Series/DataFrame-column reads, and clearSerializationErroron corrupted DataFrame/Series. Full suite: 1850 passed, 0 failures; basedpyright 0 errors.Summary by CodeRabbit
Bug Fixes
Tests