Repository navigation
fix(record): sentinel whole-file replace for generated-file modifies and multi-line deletions - #253
geekgonecrazy wants to merge 10 commits into
Conversation
…hrough the sentinel whole-file replace Two corruption classes in `record_modified_file` surfaced through `atomic git import` of a real repository (atomicdotdev/atomic, 344 commits), which failed staged synthesis verification at commit 5: 1. Generated-file modify on opaque-shape state (`::26 R1` second follow-up). The opaque fast path ADDS a lockfile as one whole-content branch; a later MODIFY fell through the fast-path guard (existing branches non-empty) into a per-line diff whose line numbers have no vertices. Cargo.lock rendered 3,050 of 89,750 bytes. Route the opaque-shape modify through the sentinel replace. 2. Multi-line contiguous deletion after an insertion commit (third follow-up). Deleting a contiguous multi-line region from a file whose vertices span an earlier insertion commit leaves the additive graph in a state the alive walk classifies as a cyclic conflict: the render emits conflict markers and keeps the deleted lines. Reproduced NATIVELY — `atomic record` of the deletion commit made `atomic status` report the file modified forever (silent corruption on every multi-line deletion after an insertion). Single-line deletions are unaffected. Both classes now route through the sentinel whole-file replace — the same fail-safe shape already used for binary content and explicit forces: every alive vertex is deleted, the full content is inserted fresh, and the CRDT ops tombstone bound branches, so the render is byte-exact regardless of diamond structure. Import of atomicdotdev/atomic advances from 4 landed commits to 18 (the next failure is a distinct merge-followup render regression, filed separately). Regression tests replay the real content states from that repository, including the exact helpers.rs transition that failed verification.
…ring classify_insert maps diff line numbers positionally onto the alive content vertices. That mapping is only provable when the graph holds exactly one vertex per old-file line. Merge diamonds leave extra alive vertices in an isolated leg closure — an insertion commit on a file whose history spans a merge anchored at silent wrong vertices and the staged render lost the inserted content (b110aa7 in atomicdotdev/atomic: rendered 68,762 of 74,206 bytes; the same content sequence recorded natively is clean, so the divergence is specific to leg assembly). The guard refuses the positional anchor with a named divergence error (1672 alive content vertices for 1793 old lines) instead of silently misrendering. Escalating to the whole-file replace was tried and is NOT safe inside a leg assembly: its unfiltered delete_all_content walk spans lineages the leg must not touch (the render tripled). The RFC-conformant complete fix — leg anchoring resolved strictly inside the leg's interpreted parent closure — is scoped in PR discussion.
a735185 to
ac2a8af
Compare
Follow-up 2: the leg divergence is now detected and fails closedGrounded the fix in the original design doc — Lee's RFC (
And the merge-leg rule the importer already documents: "Merge legs must be assembled against their own Git parent's interpreted closure." Confirmed mechanism (instrumented The fix in this PR (fail closed): Why not auto-heal: escalating to the sentinel whole-file replace was tried and is unsafe inside a leg assembly — its unfiltered |
…-file replace A divergence between the alive vertex count and the diff's old-line count (merge diamonds add vertices; empty-line accounting can shift it by one — observed 347 vertices for 346 lines) is detected and now escalates to the sentinel whole-file replace instead of failing the commit. The escalation renders byte-exact at the graph layer and stays fail-closed: the staged synthesis verification still refuses the commit when any layer disagrees. On atomicdotdev/atomic this advances the fresh-clone import past the packages/opencode-atomic-hooks/README.md commit. The same commit then fails one layer deeper — the staged semantic (CRDT) closure replay renders the right byte total but not the expected content/order (15,636 == 15,636, refused) — the merge-diamond divergence surfacing in the semantic layer, to be resolved with the leg-anchoring redesign.
Follow-up 3: divergence policy — escalate, verify, and the current import frontierTuned the divergence policy after a fresh-clone run showed a benign shape: Import frontier on atomicdotdev/atomic (fresh clone, 344 commits):
The escalation fixes the graph layer for the divergence commit, and the failure moved one layer deeper on the same commit (now The systemic statement: merge diamonds create graph structures that the per-line graph machinery, the positional anchoring, and the CRDT closure replay each tolerate differently. Four layers have now been hardened to fail closed rather than corrupt (positional anchoring guarded, sentinel escalation verified, semantic replay refuses, staged synthesis refuses). The remaining work — the leg/merge anchoring redesign that makes all layers agree on merge-diamond structures (per RFC §5.3's isolated builder) — is the follow-up; the failure classes beyond commit 15 are all this one root cause. |
CB-9B made every `atomic git import` walk the complete reachable
closure (all side-branch commits) and assemble multi-parent merges as
`GitResolution` unions with leg isolation. That major change broke
plain imports on real histories: concurrent side-branch commits
assemble against a view whose files have already advanced on the
mainline, so leg anchors bind to diverged graph structures and the
staged verification refuses — the import of atomicdotdev/atomic stopped
at 15-18 of 344 commits with three distinct corruption/refusal classes.
The original design document (docs/git-import-design.md) specifies the
linearized behavior: merge commits are handled as ordinary first-parent
diffs ("merge commits linearized"), which keeps each file's graph a
linear per-line chain that every layer — positional anchoring, the
sentinel whole-file replace, the semantic closure replay — proves
correctly.
Plain import (`mainline_only`, i.e. everything except `--all`) now
walks the first-parent chain again, stopping at the imported frontier
for incremental runs, and routes multi-parent merges through normal
recorded-file synthesis with a FirstParent derivation. The CB-9B
complete-closure walk and the GitResolution path remain for `--all`
and the bridge flows, where the RFC §5.3 union semantics are actually
required.
The three earlier fixes stay: generated-file modifies, multi-line
deletions, and the line/vertex divergence escalation all still route
through the sentinel whole-file replace and are verified byte-exact.
Follow-up 4: plain import restored to the linearized walk — one semantic-layer blocker remainsPer the original design doc ( This PR restores the linearized walk for plain imports ( Current frontier: a fresh clone lands 15 of 215 first-parent commits and stops at
Next step (single primitive, scoped): trace |
…ges from the diff's line model; trace the per-line vertex split The README.md blocker isolation (packages/opencode-atomic-hooks/ README.md, 346 lines, no trailing newline: 347 alive content vertices after its add) pointed at the record/assembly boundary. Two additions: - record_modified_file detects when the existing semantic branch count diverges from the diff's old-file line count and sentinel-routes the modify at record time, so the graph hunks and the CRDT FileOps are decided together (the previous globalize-time escalation left the change carrying positional semantic ops under whole-file graph ops). - create_content_vertices_per_line traces its produced vertex count under ATOMIC_TRACE_HYPER (symmetric with the sorted-vertices trace), which is where the phantom 347th vertex for the 346-line file will surface. Remaining blocker (unchanged): the staged semantic closure replay's chain order for tombstone+fresh-insert chains — the rotation is documented in the PR thread with reproduction fixtures.
0af2b2f to
f1a3987
Compare
…el; drop the count-based divergence guards The root cause of the git-import failures is an off-by-one in the modify's old-line accounting: `old_content.iter().filter(== b'\\n')` counts NEWLINES, but the graph's per-line vertex split, the CRDT branch-per-line model, and the diff's line numbering all count LINES — a file without a trailing newline has one more line than it has newlines (packages/opencode-atomic-hooks/README.md: 346 newlines, 347 lines). Every positional hunk index then addresses the branch and vertex domains one off, and the whole cascade of divergence escalations and semantic-closure refusals follows. Count lines with `Line::from_bytes(old_content).len()` (same splitter as the diff and the graph). With the count consistent, the original failing commit records through the normal positional path and both the graph render and the semantic closure replay agree with the commit tree. Also drops the two count-based divergence guards added earlier in this PR (the globalize escalation arm and the record-time branch/line check): cross-view merges legitimately hold MORE alive branches than lines (the sanctioned both-sides-present conflict fork), so a count divergence is not proof of corruption — the guards misfired on those forks and broke three cross-view merge tests. The sentinel routing for generated-file modifies and multi-line deletions (count-independent, verified classes) stays.
…iling newline A modify whose old or new content lacks the final newline (replacing the last no-newline line, adding a trailing newline, appending after it) loses or shifts the final byte in the per-line hunk slicing — observed on atomicdotdev/atomic: a +1-byte trailing-newline commit to atomic-cli/Cargo.toml rendered 1,934 of 1,935 bytes and the staged semantic closure refused the equal-total mismatch. Either side lacking the final newline puts the edit boundary at EOF, where partial-hunk slicing is fragile; route through the sentinel whole-file replace, which rewrites the file byte-exactly. Files whose both sides end with a newline — the common source-file case — keep the fine-grained per-line path.
…; drop unused fixtures
…replace Restores the divergence escalation from the insert path, now correctly scoped to the Insert arm only (whole-file replaces and native records route elsewhere and legitimately carry sanctioned multi-branch forks). After long accumulated in-place-edit histories the alive graph holds more content vertices than the diff's line model — the positional anchor then lands at silent wrong vertices (b110aa7-era failures at landed-count 71: renders 1,934 of 1,935 bytes). The escalation deletes every alive vertex and reinserts the file fresh, rendering byte-exactly regardless of diamond remnants.
…rom the diff's line model The wall at landed-count 71: after many accumulated in-place edits the file's alive branch count drifts above the diff's old-line count (diamond remnants), and the modify's positional hunks index the branch domain with stale line numbers — the render loses bytes inside the modified region (atomic-cli/Cargo.toml: rendered 1,934 of 1,935). The import's foreign-modify record path now detects the branch/line divergence (both counts available at record time) and forces the sentinel whole-file replace: every prior branch is tombstoned and the content rebuilt fresh, so the graph and semantic layers agree with the commit tree byte-exactly. Native and merge flows are unaffected — the check lives on the import-only record path.
Follow-up 5: the current frontier (commit 71 → resolved by record-time sentinel) and a new resume-after-failure findingThe wall at 71 landed is resolved. Root cause chain, all fixed in the branch:
New finding — resume after a failed import can corrupt the walk. Resuming an interrupted import (failed attempt left durability-checkpoint residue: registered change, persisted projected Separately observed and confirmed harmless-in-effect but worth noting: the killed run's stdout log tail is lost to buffer flushing on SIGKILL, so post-mortem landed-count reads from logs understate reality — the authority is the view's history, not the log. Frontier summary with the branch's fixes, fresh clone: previous walls at 5, 16, 41, 71, and #88 are all passed; the clean full run is in flight. |
Follow-up 6: frontier 71/215 — three more corruption classes fixed, one remaining (isolated)Fixed in this round (all with the fresh-clone full-import run as the test):
Also removed: the two count-based divergence guards that fired on sanctioned CRDT forks — they broke three cross-view merge tests (a merge fork legitimately holds more alive branches than lines). The record-time and globalize-level guards are re-scoped to paths where divergence is genuine. The remaining blocker — isolated and reproducible in seconds: commit 72 of the first-parent chain (bae5d00, +2/−3 across three regions: two 1-line replaces and one 1-line delete, with blank lines as hunk context) fails staged verification with the render one byte short inside the first modified region (renders 1,934, tree 1,935; first_diff at byte 68). Reproduced deterministically in a 5-commit micro repo replaying only the real Cargo.toml states ( |
1 participant
Summary
Fixes two corruption classes in
record_modified_filediscovered by runningatomic git importagainst a real 344-commit repository (this repo). Before the fix, the import failed staged synthesis verification at commit 5; with the fix it lands 18.Class 1 — generated-file modify on opaque-shape state
The opaque fast path (
::26 R1) ADDS a lockfile as one whole-content branch. A later MODIFY fell through the fast-path guard (existing branches non-empty) into a per-line diff whose line numbers have no vertices in that shape.Cargo.lockrendered 3,050 of 89,750 bytes:Class 2 — multi-line contiguous deletion after an insertion commit
Deleting a contiguous multi-line region from a file whose vertices span an earlier insertion commit leaves the additive graph in a state the alive walk classifies as a cyclic conflict: the render emits conflict markers and keeps the deleted lines. Reproduced natively —
atomic recordof the deletion commit madeatomic statusreport the file modified forever (silent corruption on every multi-line deletion after an insertion, not import-specific). Single-line deletions are unaffected.The fix
Both classes route through the sentinel whole-file replace — the same fail-safe shape already used for binary content and explicit forces: every alive vertex is deleted, the full content is inserted fresh, and the CRDT ops tombstone bound branches. Render is byte-exact regardless of diamond structure. The tradeoff (coarser graph granularity for the affected modifies) matches the precedent the codebase already set for generated/binary content.
Validation
helpers.rstransition that failed verification (git_import_generated_modify_test); both fail on the unfixed code paths and pass with the fixatomic-corerecord (757) + globalize (38) unit tests,atomic-repositorylib (1,465), git import suites (26+2) — all green; fmt + clippy cleanRemaining failure (out of scope, next investigation)
Commit 19 of the same import now fails on a merge follow-up: commit 18 is a merge (side branch heavily modified
atomic-agent/src/turn/orchestrator.rs), its full-tree verify passed at 74,206 bytes, and the next commit's staged render of the same untouched file regressed to 68,762. Diagnosis trail: staged visibility includes the pending change (verified); identical edge topology between native and import flows after the insert commit (verified); the corrupt render contains conflict markers citing the merge parent. This points at the merge-leg/visibility interaction in the read-side walk and deserves its own focused investigation.