Content roundtrip fidelity tests failing: duplication and truncation in record pipeline #41

Activity

1. leefaus

   added

   bugSomething isn't workingSomething isn't working

   on Apr 9, 2026
2. leefaus commented on Apr 9, 2026

   

   leefaus

   on Apr 9, 2026

   ContributorAuthor

   More actions

   Update: 5 of 6 failures fixed

   Root cause for 5 tests: The INODE_GRAPH fast path in find_content_vertices bypassed ViewGraph's change filter. When record() uses ViewGraph wrapping, the INODE_GRAPH returned vertices from ALL changes regardless of view visibility, causing content duplication.

   Fix: Always use retrieve_graph (which respects whatever GraphTxnT impl is provided) for content vertex discovery. Commit: 0a2d376.

   Remaining failure: test_three_sequential_modifications

   This test records 3 versions of a file:

   • v1: line1\nline2\nline3\n
   • v2: line1\nmodified\nline3\n (middle line changed)
   • v3: line1\nmodified\nline3\nextra\n (line appended)

   Then calls assert_content_after_change(&repo, "data.txt", &h3, v3) which fails:

   • Got: line1\nmodified\nline3\n (21 bytes — v2's content, missing the appended line)
   • Expected: line1\nmodified\nline3\nextra\n (27 bytes — v3's content)

   This test passes on the old branch (origin/chore/roundtirp-fidelity-test), so it's a regression specific to dev's architecture.

   The issue is likely in get_file_content_after_change() and how it builds the change filter for state-based content retrieval. The change filter may not include h3's change, so the graph traversal returns v2's state instead of v3's.
3. leefaus commented on Apr 9, 2026

   

   leefaus

   on Apr 9, 2026

   ContributorAuthor

   More actions

   Update: narrowed to 1 remaining failure

   Commit 0a2d376 fixed 5 of 6 tests by disabling the INODE_GRAPH fast path in find_content_vertices (it bypassed ViewGraph's change filter).

   Remaining: test_three_sequential_modifications

   Diagnosis complete — the issue is NOT in recording. Debug confirms:

   • h3's record produces 1 hunk (old=21 bytes, new=27 bytes, Insert)
   • h3 IS in the view's change log (seq=2)
   • h3's change is saved and inserted successfully

   The issue is in content retrieval:

   • get_file_content("data.txt") returns 21 bytes (v2 content) even though h3 (with the append) is in the view
   • get_file_content_after_change(h3) also returns 21 bytes
   • h1 and h2 content retrieval is correct

   Root cause: h3's Insert hunk creates a new vertex for "extra\n" that should be connected after the last content vertex. But retrieve_graph doesn't find it — the vertex either wasn't written to GRAPH correctly by insert_change, or the edge connecting it to the existing graph is missing/wrong.

   This is a graph-write or edge-connectivity bug in the apply pipeline, not a recording or content retrieval bug. The old branch's apply pipeline (apply_change) produced correct edges; dev's insert_change → write_change_to_graph does not for this specific case (append after a Replace).
4. leefaus commented on Apr 9, 2026

   

   leefaus

   on Apr 9, 2026

   ContributorAuthor

   More actions

   Deep dive on last failure — hit dead end

   Exhaustive comparison of the code paths between old branch and dev shows identical logic for:

   • record_modified_file (consolidation, content position calc)
   • globalize_hunk / classify_insert / find_content_vertices
   • pipeline.rs content routing
   • is_vertex_alive / create_alive_vertex
   • iter_parents flag ranges
   • retrieve_graph DFS

   Key finding: After h2's Replace, classify_insert finds 2 content vertices (h1's old vertex AND h2's new vertex) on dev. On the old branch it should find only 1 (h2's). This means h1's old content vertex is considered alive on dev but dead on the old branch.

   Both branches have the same is_vertex_alive logic (check for non-deleted PARENT edges). Both use iter_adjacent with the same flag range to exclude DELETED. The old branch's OverlayTxn passes through to bare ReadTxn for shared stacks.

   Hypothesis: The difference is in how h2's Replace hunk is globalized — the deletion edges it produces might differ between the two branches, causing h1's vertex to retain an alive parent edge on dev but not on the old branch. The globalize pipeline code LOOKS identical, but the restructuring into subdirectories may have introduced a subtle difference in how content bytes are passed through the pipeline.

   Recommended next step: Add a graph edge dump test (iterate all GRAPH entries) on both branches after recording h1+h2, and diff the actual B-tree contents. This will show exactly which edges are present and whether h1's vertex has the expected DELETED parent on dev.
5. leefaus commented on Apr 9, 2026

   

   leefaus

   on Apr 9, 2026

   ContributorAuthor

   More actions

   Root cause found

   The apply pipeline's vertex resolution is different between the old branch and dev.

   Old branch (atomic-core/src/apply/insertion.rs):

   • resolve_context_vertex_for_target(txn, pos, is_up, target)
   • resolve_vertex_for_target(txn, pos, target, FindBlockMode)
   • These use overlay::find_block_in_stack_graph for target-aware resolution

   Dev (atomic-core/src/apply/insertion.rs):

   • resolve_context_vertex(txn, pos, is_up)
   • resolve_vertex(txn, pos, is_up)
   • Simpler resolution without target awareness

   When a second Replace targets a file that was already Replaced by a prior change, the deletion edges need to find the CORRECT content vertex (the one created by the first Replace). The old branch's target-aware resolution finds it correctly. Dev's simpler resolution can resolve to the wrong vertex (the original vertex from the first change), causing the deletion to mark the wrong edges — leaving the first Replace's content vertex appearing alive alongside the second Replace's new content.

   This affects both:

   • test_three_sequential_modifications (text: append after replace)
   • Suite 13 "Multiple Binary Modifications" (binary: replace after replace)
   • Any pattern with 3+ sequential modifications to the same file

   Fix: Port the target-aware vertex resolution from the old branch, or adjust resolve_vertex/resolve_context_vertex in dev's position.rs to handle the multi-vertex ambiguity correctly. The key functions to compare:

   • Old: atomic-core/src/apply/insertion.rs lines 117, 136, 155-165, 178-188
   • Dev: atomic-core/src/apply/insertion.rs lines 109, 128, 147-148, 161-162
   • Old: atomic-core/src/apply/edge.rs — resolve_vertex_for_target
   • Old: atomic-core/src/pristine/overlay.rs — find_block_in_stack_graph
6. leefaus

   added 2 commits that reference this issue on Apr 9, 2026

   fix: restore del_edge_with_reverse in apply pipeline

   ...

   This issue will close once commit e78a7da is merged into the 'dev' branch.

   e78a7da

   Improve roundtrip fidelity and optimize Git import process (#40)

   ...

   This issue will close once commit c9b14c0 is merged into the 'dev' branch.

   c9b14c0
7. leefaus

   closed this as completedon Apr 9, 2026
8. leefaus

   added a commit that references this issue on May 5, 2026

   Enhance Git import performance and roundtrip fidelity (#44)

   ...

   This issue will close once commit 961df86 is merged into the 'dev' branch.

   961df86