[claude] Resume snapshot replays from checkpoints - #107
Open
myieye wants to merge 3 commits into
Open
Conversation
Adding several commits at once only keeps a snapshot for every other one. A commit dated between a commit with no snapshot and the next one that has one makes the replay start after its own parent, while the entity resumes from the older snapshot it still has, so the commits in between are applied by nobody. Both tests fail: one loses an edit, the other revives a cascade-deleted definition whose word is still deleted and breaks the foreign key. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (1)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe pull request adds two regression tests for late commits after snapshot pruning. The tests cover word edits and cascade-deleted definition edits, including state restoration and preservation of ChangesLate commit regression tests
Estimated code review effort: 2 (Simple) | ~10 minutes Merge Risk: 🟡 Moderate · up to This change adds regression coverage for late commits after snapshot pruning, but the new tests currently fail because the underlying replay behavior is not yet fixed. Merge after the accompanying fix makes these scenarios pass. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Notes from the investigation into #105: the completeness invariant, why a hole is an interval rather than a commit, the rules that make checkpoints safe, and the dead ends so nobody repeats them. Records the measurements too, since most of the arguments here turn on numbers. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
myieye
changed the title
Fixes #105. A commit dated before commits already in the database made the replay resume each entity from whatever snapshot survived, which can predate edits whose snapshots were pruned, so nothing re-applied them. One symptom lost an edit, the other revived a cascade-deleted entity and broke the FK to its deleted parent. Commits now carry IsSnapshotCheckpoint: a position where every entity's newest snapshot at or before it is that entity's state there, so a replay can resume from it. SnapshotCheckpointPolicy picks every 8th commit of a replayed batch plus its last, and the pruner keeps whatever snapshots that choice needs, which replaces the CommitIndex % 2 rule. Reading state at an old commit resumes from a checkpoint too, so a change no longer reads a neighbour's state as of the commit being asked about rather than the position being replayed. Flags are only ever written for commits inside a window being replayed. When there is no checkpoint before the late commit, nothing safe exists to resume from, so everything is dropped and regenerated: that is both the repair and the bootstrap for databases written before this existed. Consumers need a migration for the new Commits column. +semver: minor Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
myieye
changed the title
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.
[Claude, autonomous]
Fixes #105. Commits now carry a local
IsSnapshotCheckpointflag marking positions a replay may resume from, and the pruner keeps whatever snapshots that choice needs, replacing theCommitIndex % 2rule. Design and the dead ends behind it: docs/snapshot-checkpoints.md.Commitscolumn, hence+semver: minoron the commit.