Repository navigation
fix: derive FactRefs from on-disk layout when rebuilding indexes on open (#370) - #372
Merged
Merged
Conversation
…pen (#370) The rebuild-on-load path (index checksum mismatch, e.g. after a crash mid-save) re-packed all facts contiguously with pack_facts() to derive FactRefs. Every save() starts a fresh page, so a file written by multiple checkpoints has partially filled pages that a contiguous re-pack does not reproduce. From the second checkpoint onward, all four rebuilt indexes pointed at the wrong page/slot, so entity-bound lookups returned nothing (or errored) while the file opened cleanly. Read facts together with their real (page, slot) via the new packed_pages::read_all_with_refs and build the indexes from those. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PuKbXn6NaHBMDVkScdBNQZ
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
adityamukho
added a commit
that referenced
this pull request
Sep 25, 2026
Patch release on the v2.x line; file format unchanged (v7), no API changes. Ships #372: the index rebuild on open now uses each fact's real on-disk location, so entity-bound lookups no longer come back empty after a crash-triggered rebuild of a multi-checkpoint file. Doc sync: CHANGELOG (v2.0.1 entry + known issues #371/#287, #373/#374), CLAUDE.md, README.md, ROADMAP.md (v2.x on main, v3.0.0 on the `v3` branch), docs/TEST_COVERAGE.md (1154 tests). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01BedF9ZmqVZLkvDwn91fuKy
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.
Summary
Fixes the root cause behind the silent entity-lookup loss reported in #370.
When the index checksum doesn't match on open (e.g. after a kill mid-
save(), which overwrites old index pages before committing the header),load()rebuilds all four indexes from the fact pages. It derived each fact'sFactRefby re-packing every fact contiguously withpack_facts(&all_facts, 1). But eachsave()starts a fresh page, so a file written by several checkpoints has partially filled pages that a contiguous re-pack does not reproduce. From the second checkpoint onward, every rebuilt index entry pointed at the wrong page/slot, so entity-bound lookups returned nothing (or errored) while the file opened cleanly, andcheckpoint()copied the bad refs forward.Changes
packed_pages::read_all_with_refs: reads facts together with their real(page_id, slot_index);read_all_from_pagesnow delegates to it.persistent_facts.rsbuilds the indexes from those refs instead of re-packing.test_rebuild_after_multiple_checkpoints_keeps_entity_lookups: five checkpoints, corrupt the index checksum, reopen, look up every entity. Fails before the fix withSlot 1 out of bounds (page has 1 records); passes after.Caveats
save()) are tracked separately, so this usesRefs, notCloses.Test plan
cargo test(757 lib tests + integration + doc)cargo clippy --lib,cargo fmtRefs #370
🤖 Generated with Claude Code
https://claude.ai/code/session_01PuKbXn6NaHBMDVkScdBNQZ