fix: incorrect field access in create_checkpoint #4178
Merged
Conversation
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
elias-ba
changed the title
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 4164-fix-nil-lock-version-crash #4178 +/- ##
===================================================================
+ Coverage 89.04% 89.10% +0.06%
===================================================================
Files 425 425
Lines 19658 19658
===================================================================
+ Hits 17504 17516 +12
+ Misses 2154 2142 -12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
elias-ba
force-pushed
the
4176-checkpoint-state-data-field
branch
2 times, most recently
from
bd14d5b to
6fd1ec1
Compare
elias-ba
added a commit
that referenced
this pull request
Dec 10, 2025
Move duplicated checkpoint and update query logic from Persistence and PersistenceWriter into DocumentState module where data access belongs. - Add get_checkpoint_and_updates/1, get_latest_checkpoint/1, get_updates_since/2, apply_to_doc/3, and load_into_doc/2 functions - Update Persistence to use shared DocumentState functions - Update PersistenceWriter.merge_updates/2 to use DocumentState.load_into_doc/2 - Remove duplicated load_persisted_state_into_doc/2 from PersistenceWriter - Add @type t and proper @SPEC annotations to DocumentState This eliminates code duplication while keeping the fix focused and avoiding conflicts with #4178.
The create_checkpoint/1 function was using .data instead of .state_data when accessing DocumentState records. This would cause a KeyError crash when a document reached 500 updates and triggered checkpoint creation. Closes #4176
Two bugs fixed in create_checkpoint/1: 1. Used .data instead of .state_data when accessing DocumentState records 2. Used string "checkpoint" instead of atom :checkpoint for Ecto.Enum Added tests to verify checkpoint creation works correctly: - Test creating checkpoint from multiple updates - Test merging existing checkpoint with new updates Both bugs would cause crashes when a document reached 500 updates and triggered automatic checkpoint creation. Closes #4176
elias-ba
force-pushed
the
4176-checkpoint-state-data-field
branch
from
6fd1ec1 to
93aeff0
Compare
elias-ba
changed the base branch from
main
to
4164-fix-nil-lock-version-crash
midigofrank
approved these changes
Dec 11, 2025
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.
Description
This PR fixes two bugs in
create_checkpoint/1that would crash thePersistenceWriterafter 500 document updates:.datainstead of.state_datawhen accessingDocumentStaterecords"checkpoint"instead of atom:checkpointfor theEcto.Enumversion fieldCloses #4176
Validation steps
mix test test/lightning/collaboration/document_supervisor_test.exs:818
Additional notes for the reviewer
AI Usage
Please disclose how you've used AI in this work (it's cool, we just want to know!):
You can read more details in our Responsible AI Policy
Pre-submission checklist
:owner,:admin,:editor,:viewer)