Skip to content

Detect a torn packed-cache read instead of serving wrong rows - #81

Merged
thorwhalen merged 1 commit into
masterfrom
fix/packed-cache-torn-read-detection
Sep 22, 2026
Merged

thorwhalen merged 1 commit into
masterfrom
fix/packed-cache-torn-read-detection

Conversation

@thorwhalen

Copy link
Copy Markdown
Member

Detects a torn packed-cache read instead of silently serving mismatched rows.

CorpusStore._save_packed writes matrix.npy, ids.json, metas.json and
sig.json as four independent calls. Two concurrent rebuilds of a same-size
corpus can leave the matrix from writer A beside ids/metas from writer B; the
old _load_packed only checked len(ids) == mat.shape[0], so the mixture
passed every check while row i no longer belonged to ids[i] — searches
answered confidently wrong, nothing raised or logged.

sig.json now also carries a sha256 content_sig over the exact ids/metas
bytes written with that matrix, plus the matrix shape. On load, a
disagreement is treated as a cache miss and the matrix rebuilds from records
— the same fallback path already used for a missing sig, bad format, or
OSError.

This is option 2 of #77 (detect cheaply and loudly), not option 1 (atomic
directory-pointer swap); a store-level lock / true write atomicity remains
open there, so this closes #77 for the corruption-detection half of it.

Backward/forward compatible: checks are guarded on the new fields being
present, so a cache written before this loads on the length checks alone, and
_PACKED_FORMAT stays at 1 (no cache invalidation). No public name,
signature or return type changes — every packed helper is module-private,
and stores opened without packed_dir (e.g. CorpusStore.memory()) never
reach this code.

What I did (rebase + gate)

Dependents check

fleet_dependents.json lists raglab and truffle as importers of ir.

  • raglab (present on this box) only calls CorpusStore.memory() and the
    public CorpusStore surface in its tests/source — it never sets
    packed_dir and never touches _load_packed/_save_packed, so it cannot
    reach this code path at all (confirmed by grep, matching the commit's own
    claim that stores without packed_dir never reach this code).
  • truffle is not cloned on this box, so it could not be checked directly.
    Given the change touches only module-private packed-cache helpers with no
    public signature/return-type/name change, this is not treated as a
    breaking change requiring truffle's tests — noted here for the record.

Closes #77

🤖 Generated with Claude Code

`_save_packed` writes matrix.npy, ids.json, metas.json and sig.json as four
independent calls. Writing sig last defends against a crash mid-write, but not
against a *second* writer: two rebuilds of a same-size corpus leave the matrix
from writer A beside ids/metas from writer B, and `_load_packed` validated only
`len(ids) == mat.shape[0] and len(metas) == len(ids)`, so the mixture passed
every check while row i no longer belonged to ids[i]. Searches then answered
confidently wrong, with nothing raised and nothing logged.

sig.json now also carries a sha256 `content_sig` over the exact ids.json /
metas.json bytes written with that matrix, plus the matrix `shape`. On load, a
disagreement reads as a cache miss and the matrix is rebuilt from records --
the same path already taken for a missing sig, a bad format, or an OSError.

This is option 2 of the issue (detect cheaply and loudly), not option 1 (the
atomic directory-pointer swap); a store-level lock and true write atomicity
remain open there.

Backward compatible in both directions: the new checks are guarded on the
fields being present, so a cache written before them still loads on the length
checks alone, and an older `ir` reading a new cache ignores the extra keys
(it only reads `format`). `_PACKED_FORMAT` stays at 1 -- no cache is
invalidated. No public name, signature or return type changes; every packed
helper is module-private, and stores opened without `packed_dir` never reach
this code.

Claude-Session: https://claude.ai/code/session_01L1aQPB34n7PU7jmbztSjBe
@thorwhalen
thorwhalen merged commit ebd617d into master Sep 22, 2026
12 checks passed
@thorwhalen
thorwhalen deleted the fix/packed-cache-torn-read-detection branch September 22, 2026 12:53
thorwhalen added a commit that referenced this pull request Sep 22, 2026
#83)

The content_sig added in #81 hashes ids/metas only. The interleaving
"writer A saves matrix.npy, writer B saves a whole set, A then writes
ids/metas/sig" leaves B's same-shape matrix under A's ids and a sig that
vouches for them, so rows were still served under the wrong ids.

Each save now writes matrix/ids/metas under a fresh generation token that
only that writer touches, and publishes sig.json last via os.replace; a
reader follows the sig to exactly one writer's complete set. Older
generations are swept after publishing. Legacy flat-layout caches still
load. Regression test drives the real interleaving through _save_packed.

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
@thorwhalen

Copy link
Copy Markdown
Member Author

Post-merge refute review: content_sig covered ids/metas but not the matrix. The interleaving "A saves matrix, B saves a full set, A writes ids/metas/sig" still served B's rows under A's ids. Fixed in #83 (per-writer generation files + atomic sig publish), with a regression test that drives the real interleaving.

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.

CorpusStore packed writes are not atomic: concurrent rebuilds can silently desynchronise matrix rows from ids

1 participant