8000 fix(storage): index pending facts past 65,535 correctly (#445) by adityamukho · Pull Request #446 · project-minigraf/minigraf · GitHub
[go: up one dir, main page]

Skip to content

fix(storage): index pending facts past 65,535 correctly (#445) - #446

Merged
adityamukho merged 1 commit into
mainfrom
fix/445-pending-slot-overflow
Oct 6, 2026
Merged

adityamukho merged 1 commit into
mainfrom
fix/445-pending-slot-overflow

Conversation

@adityamukho
@adityamukho adityamukho 8000 commented Oct 6, 2026 •
Copy link
Copy Markdown
Collaborator

Summary

Fix for #445 on v2.x. Once a database held more than 65,535 facts that had not been checkpointed, entity lookups and attribute scans silently lost or duplicated facts. Stored data was never affected.

Cause. Each pending fact is indexed with FactRef { page_id: 0, slot_index }, where slot_index is the fact's position in FactStorage's pending list. The position was a saturating u16, so every fact from position 65,535 on shared one slot and resolved to the same fact.

Fix.

  • FactRef::slot_index is now a u32.
  • Pending positions are computed by pending_refs, which returns an error (INT-045) instead of saturating, before anything is inserted.
  • load_fact checks for a duplicate before claiming a slot, so an error leaves no partial state.
  • read_slot takes a u32; a committed slot is still bounded by the page's record count.

File format unchanged. postcard writes u16 and u32 as the same varint, so v7 index pages on disk are byte-for-byte unchanged. fact_ref_encoding_matches_the_u16_layout pins this.

Tests

  • pending_facts_past_u16_resolve_to_themselves: 70,000 pending facts via transact_batch, plus transact, retract and load_fact past slot 65,535. Entity, entity+attribute and attribute lookups each return exactly the fact's own record, with no duplicates.
  • pending_refs_are_distinct_and_checked: slots are distinct across the u16 boundary, and a position past u32::MAX is an error.
  • edge_cases_test.rs: two tests run 70,000 facts through the public API, one in memory and one file-backed (before and after checkpoint()). The file-backed run failed on v2.0.2 before this fix (count 65,536 instead of 70,000; :e65536 returned []).
  • Full suite: 1,209 passing, 8 ignored. cargo clippy --all-features -- -D warnings is clean.

Docs

CHANGELOG (Unreleased → Fixed), TEST_COVERAGE, and the test counts in CLAUDE.md and README.

Per CONTRIBUTING.md, #445 stays open until the fix ships in a published release, so this PR uses Refs #445.

Refs #445

🤖 Generated with Claude Code

https://claude.ai/code/session_015GgLEs8YozJjYhwPEYgTUn

Each pending fact was indexed with FactRef { page_id: 0, slot_index }, where
slot_index is its position in the pending list. The position was a
saturating u16, so every fact from position 65,535 on shared one slot and
entity lookups and attribute scans silently lost or duplicated facts. This
affected in-memory databases and file databases before a checkpoint.

slot_index is now a u32, and pending positions are checked rather than
saturated. postcard writes u16 and u32 as the same varint, so v7 index pages
are byte-for-byte unchanged; a test pins this.

Refs #445

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015GgLEs8YozJjYhwPEYgTUn
@codecov
codecov Bot commented Oct 6, 2026
Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 90.00000% with 3 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/storage/persistent_facts.rs 0.00% 3 Missing ⚠️

📢 Thoughts on this report? Let us know!

@adityamukho
adityamukho merged commit e361bb5 into main Oct 6, 2026
20 checks passed
@adityamukho
adityamukho deleted the fix/445-pending-slot-overflow branch October 6, 2026 02:42
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.

1 participant

0