Merge integrity: fix record-layer duplication, persist conflict state, add merge rubric - #157
Merged
Merged
Conversation
Adds 26_merge_ordering_duplication.sh, which fails against the current
code. It pins two defects in the graph-to-file path, both of which
corrupt a file while reporting complete success:
1. Independent changes merge as a conflict. One change prepends a
line, another appends one — disjoint regions that commute — and the
result is a conflict between untouched base content and the second
change's line, with that line relocated into the middle of the file.
2. Content after a conflict is emitted once per side. The conflict
region renders correctly, but every line below it is written twice,
scaling with the length of the tail.
Both are silent: `atomic insert` prints success with no conflict warning
and `atomic status` then reports a clean tree, so a later `record` bakes
the duplicated text into history as though it were authored.
Root cause is in recording, not output. `rewrite_shifted_equals_for_graph`
(atomic-core/src/record/workflow/record/mod.rs) converts unchanged lines
into Replace ops whenever content inserted above shifts their line
numbers, so a change re-records everything from its edit point to EOF as
fresh content it owns. Inserting one line into a five-line file records
5 vertices / 28 bytes instead of 1 / 4, growing linearly with distance
from EOF. Invisible in a single change, since the new copy supersedes the
old — but on merge both changes carry their own copy of the shared tail
and both stay alive.
The test is committed failing, deliberately. Deleting that rewrite makes
all 13 assertions pass and shrinks a one-line insertion back to 1 vertex
/ 4 bytes, but it drops content in
test_{switch_view,insert_change}_preserves_harness_08_sequence_through_v10,
where an edit interleaving several insert and replace regions loses
whole lines. The rewrite is load-bearing: it compensates for the graph
being unable to re-anchor unchanged regions that shift. Repairing that
anchoring is the actual fix, and is not attempted here — shipping the
naive removal would trade duplicated content for lost content.
…nes via SCC order Recording a prepend or middle insert rewrote every downstream unchanged line into a Replace, so each view carried private copies of shared text. Merging then produced spurious conflicts and duplicated everything after a genuine conflict once per side (harness 26). Removing the rewrite exposed the real bug it masked: hunk line indices were mapped to vertices by a linear walker that drops vertices at diamond structures, letting a Delete hunk fall back to delete-all-content. The mapping now uses the same retrieve_graph + compute_order pipeline that materializes file content, with a new RetrieveOptions::deletions_final mode so aliveness matches the view-filtered materialization.
… with 26_vault_intent_inherit
graywolf336
force-pushed
the
fix/merge-ordering-duplication
branch
from
August 7, 2026 19:49
11d6828 to
6168e9a
Compare
vinceblock99
reviewed
Aug 7, 2026
| let options = RetrieveOptions::new().deletions_final(true); | ||
| let mut result = match retrieve_graph(txn, inode_pos, options) { | ||
| Ok(r) => r, | ||
| Err(_) => return Ok(Vec::new()), |
Contributor
There was a problem hiding this comment.
Should we just return error and end the record if there is an error?
martian56
approved these changes
Aug 10, 2026
graywolf336
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
4 participants
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.
Record-layer fix (harness 27)
Fixes the two data-integrity bugs in
tests/harness/27_merge_ordering_duplication.sh: independent changes merged as a conflict, and content after a genuine conflict duplicated once per side. Root cause was in record, not merge:rewrite_shifted_equals_for_graphturned unchanged lines below an insertion into Replaces, so each view carried private copies of shared text. Removed it, and replaced the linear line→vertex walker (which dropped vertices at diamond structures and could escalate a one-line delete to delete-all-content) with the sameretrieve_graph+compute_orderpipeline that materializes content, plus a newRetrieveOptions::deletions_finalmode so aliveness matches the view-filtered materialization.Merge rubric + conflict surfacing
docs/MERGE-CONFLICT-RUBRIC.md: enumerates the merge/duplication scenario space (edit relationship × convergence pathway) so gaps are known instead of discovered via corruption.atomic statusreportsConflicted, newatomic conflictscommand lists kind/line/contending changes, andatomic doctor checkverifies materialization drift and conflict honesty (markers ⇔ status ⇔ conflict list).28_merge_rubric.sh(rubric cells) and29_rename.sh, sharedmerge_helpers.sh, and property/surface/delete-propagation/rename test suites.Remote fixes
Effective-history computation for remotes, remote type/query updates, and clone/pull/push/view-list fixes.
🤖 Generated with Claude Code