fix: preserve AI attribution when human edits file after AI checkpoint (fixes #1444)#1667
fix: preserve AI attribution when human edits file after AI checkpoint (fixes #1444)#1667IanMcNelly wants to merge 2 commits into
Conversation
When a human modifies a file after an AI checkpoint fires but before committing — without triggering a known_human checkpoint — the carryover snapshot could be stale (holding the AI version). This caused the overlap filter to incorrectly strip the human-modified line from unstaged_hunks, leaving it attributed to AI. Fix: skip the committed-vs-unstaged overlap filter entirely when a carryover snapshot is present. With a snapshot, Replace-type unstaged lines represent lines where committed content differs from the AI checkpoint — they should flow to INITIAL, not be re-attributed. Also replace the content-based guard in recover_adjacent_edges with a displaced_to_initial_lines set derived from initial_attributions ∩ recovery_hunks. This prevents edge recovery from re-claiming human-modified lines that were explicitly sent to INITIAL, while still allowing edge recovery for adjacent uncheckpointed lines (inserts/appends) that were never in the attribution loop at all. Fixes git-ai-project#1444 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
|
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| let pure_inserts_before = pure_insertion_lines | ||
| .iter() | ||
| .filter(|&&l| l < workdir_line_num) | ||
| .count() as u32; | ||
| explicitly_resolved_commit_lines | ||
| .insert(workdir_line_num - pure_inserts_before, ()); |
There was a problem hiding this comment.
🟡 Deleted AI lines incorrectly block gap-fill for unrelated committed lines
A line that was inserted into the carryover snapshot (but absent from the commit) is given a fabricated committed-coordinate position (workdir_line_num - pure_inserts_before at src/authorship/virtual_attribution.rs:2378-2379) and recorded as explicitly resolved, so gap-fill for the unrelated committed line at that position is silently skipped.
Impact: An AI-authored committed line could lose its attribution when a nearby carryover-only insertion happens to alias its position.
Coordinate-mapping logic for pure insertions in the unstaged branch
When a line is a pure insertion in carryover space (it exists in the last checkpoint but was removed before committing), it has no corresponding committed-space line. Despite this, the code at src/authorship/virtual_attribution.rs:2374-2379 computes workdir_line_num - pure_inserts_before and inserts the result into explicitly_resolved_commit_lines. That formula maps to a real but unrelated committed line.
For example, if committed content is [A, B, C] and carryover is [A, X, Y, C] (X replaces B, Y is a pure insertion), the pure insertion Y at carryover position 3 gets committed coordinate 3 - 0 = 3, which points to committed line C. If C is inside a committed hunk and needs gap-fill, the check at src/authorship/virtual_attribution.rs:2486 skips it.
The fix is to skip the explicitly_resolved_commit_lines insertion when the unstaged line is itself a pure insertion, since pure insertions have no committed counterpart:
if !pure_insertion_lines.binary_search(&workdir_line_num).is_ok() {
let pure_inserts_before = ...;
explicitly_resolved_commit_lines.insert(...);
}| let pure_inserts_before = pure_insertion_lines | |
| .iter() | |
| .filter(|&&l| l < workdir_line_num) | |
| .count() as u32; | |
| explicitly_resolved_commit_lines | |
| .insert(workdir_line_num - pure_inserts_before, ()); | |
| let is_pure_insertion = pure_insertion_lines.binary_search(&workdir_line_num).is_ok(); | |
| if !is_pure_insertion { | |
| let pure_inserts_before = pure_insertion_lines | |
| .iter() | |
| .filter(|&&l| l < workdir_line_num) | |
| .count() as u32; | |
| explicitly_resolved_commit_lines | |
| .insert(workdir_line_num - pure_inserts_before, ()); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
| let displaced_to_initial_lines: HashMap<String, HashSet<u32>> = initial_attributions | ||
| .files | ||
| .iter() | ||
| .filter_map(|(file, line_attrs)| { | ||
| let committed_for_file = recovery_hunks.get(file)?; | ||
| let committed_set: HashSet<u32> = | ||
| committed_for_file.iter().flat_map(|r| r.expand()).collect(); | ||
| let displaced: HashSet<u32> = line_attrs | ||
| .iter() | ||
| .flat_map(|la| la.start_line..=la.end_line) | ||
| .filter(|l| committed_set.contains(l)) | ||
| .collect(); | ||
| if displaced.is_empty() { | ||
| None | ||
| } else { | ||
| Some((file.clone(), displaced)) | ||
| } | ||
| }) | ||
| .collect(); |
There was a problem hiding this comment.
🚩 displaced_to_initial_lines assumes carryover and committed coordinates are identical
The displaced_to_initial_lines computation at src/authorship/post_commit.rs:400-418 intersects initial_attributions line numbers (carryover/working-directory coordinates) with recovery_hunks line numbers (committed coordinates). These coordinate spaces differ when there are pure insertions in carryover. However, I traced through the scenarios and found that when pure insertions exist (human appends lines after AI checkpoint), the stale-observed detection at src/authorship/virtual_attribution.rs:2058-2062 fires and sets carryover = committed, eliminating all unstaged hunks and producing empty initial_attributions. For replacements (human modifies a line), there are no pure insertions, so coordinates are identical. The assumption appears safe for the targeted scenarios, but if future changes alter the stale-detection logic, this intersection could silently produce incorrect results.
Was this helpful? React with 👍 or 👎 to provide feedback.
2 participants
Problem
When a human modifies a file after an AI checkpoint fires but before committing — without a
known_humancheckpoint being triggered — AI attribution for the AI's own lines was lost (shown as 100% untracked).Reported in #1444.
Root cause
The carryover snapshot stays stale (holds the AI checkpoint version). An overlap filter in the attribution loop was then stripping the human-modified position from
unstaged_hunksbecause it also appeared incommitted_hunks— incorrectly crediting that committed line to AI. For the modify case (human replaces an AI-written line), this meant the human's content was attributed to AI.Fix
virtual_attribution.rs: Skip the committed-vs-unstaged overlap filter when a carryover snapshot is present. With a snapshot, Replace-type unstaged lines represent content divergence between the AIcheckpoint and the commit — they should flow to INITIAL, not be silently re-attributed.
attribution_recovery.rs/post_commit.rs: Replace the content-based guard inrecover_adjacent_edgeswith adisplaced_to_initial_linesset (intersection ofinitial_attributionsandrecovery_hunks). This blocks edge recovery from re-claiming lines explicitly sent to INITIAL, while preserving edge recovery for genuinely adjacent uncheckpointed inserts/appends.Tests
Added
tests/integration/human_edit_after_ai_checkpoint.rswith four regression tests covering:All existing tests pass including the
delayed_checkout/switch_merge_trace_replaytests.Fixes #1444