From 7fc93259446a63d8de1fd826deeb6e61d58e2fae Mon Sep 17 00:00:00 2001 From: github-actions Date: Fri, 26 Jun 2026 19:25:10 -0400 Subject: [PATCH 1/2] fix: preserve AI attribution when human edits file after AI checkpoint MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 #1444 Co-Authored-By: Claude Sonnet 4.6 --- src/authorship/attribution_recovery.rs | 42 +++- src/authorship/post_commit.rs | 55 +++- src/authorship/virtual_attribution.rs | 155 ++++++++---- .../human_edit_after_ai_checkpoint.rs | 238 ++++++++++++++++++ tests/integration/main.rs | 1 + 5 files changed, 437 insertions(+), 54 deletions(-) create mode 100644 tests/integration/human_edit_after_ai_checkpoint.rs diff --git a/src/authorship/attribution_recovery.rs b/src/authorship/attribution_recovery.rs index d41bdc137a..29837ab1b2 100644 --- a/src/authorship/attribution_recovery.rs +++ b/src/authorship/attribution_recovery.rs @@ -197,6 +197,11 @@ struct CommitMetadataSessionSelection { pub(crate) struct AttributionRecoveryContext<'a> { pub(crate) file_timestamps: Option<&'a FileTimestampsByPath>, pub(crate) before_external_recovery: Option<&'a dyn Fn(&UnknownLinesByFile)>, + /// Per-file sets of committed-coordinate line numbers that were explicitly displaced + /// to INITIAL by the attribution loop (i.e., human-modified lines that replaced + /// AI-checkpoint content without a known_human checkpoint firing). Edge recovery + /// must not re-claim these lines for AI — they were consciously excluded. + pub(crate) displaced_to_initial_lines: Option<&'a HashMap>>, } pub(crate) fn recover_attribution( @@ -232,6 +237,7 @@ pub(crate) fn recover_attribution( commit_sha, authorship_log, committed_hunks, + context.displaced_to_initial_lines, ); let unknown_after_edges = unknown_lines_by_file(authorship_log, committed_hunks); if unknown_after_edges.is_empty() { @@ -1122,15 +1128,40 @@ fn recover_adjacent_edges( commit_sha: &str, authorship_log: &mut AuthorshipLog, committed_hunks: &HashMap>, + displaced_to_initial_lines: Option<&HashMap>>, ) { let unknown = unknown_lines_by_file(authorship_log, committed_hunks); for (file_path, unknown_lines) in unknown { let line_to_author = line_author_map(authorship_log, &file_path); let runs = contiguous_runs(&unknown_lines); + + // Lines that were explicitly displaced to INITIAL by the attribution loop (e.g., + // human-modified lines that replaced AI-checkpoint content). Edge recovery must not + // re-claim these — they were consciously excluded from the authorship note. + let displaced: Option<&HashSet> = + displaced_to_initial_lines.and_then(|map| map.get(&file_path)); + for run in runs { let Some(recovery) = edge_recovery_for_run(&line_to_author, &run) else { continue; }; + + // Skip any lines that were explicitly sent to INITIAL during attribution. + let lines: Vec = if let Some(displaced_set) = displaced { + recovery + .lines + .iter() + .copied() + .filter(|&line_num| !displaced_set.contains(&line_num)) + .collect() + } else { + recovery.lines.clone() + }; + + if lines.is_empty() { + continue; + } + let trace_id = generate_trace_id(); let source_session = recovery .source_author @@ -1143,19 +1174,14 @@ fn recover_adjacent_edges( } else { recovery.source_author.clone() }; - let recovered_line_count = recovery.lines.len() as u32; - add_attestation( - authorship_log, - &file_path, - &recovered_author, - &recovery.lines, - ); + let recovered_line_count = lines.len() as u32; + add_attestation(authorship_log, &file_path, &recovered_author, &lines); let metadata = json!({ "solver": "edge_extension", "file_path": file_path, "source_author": &recovery.source_author, - "recovered_lines": &recovery.lines, + "recovered_lines": &lines, }); record_recovery_metric(RecoveryMetricInput { repo, diff --git a/src/authorship/post_commit.rs b/src/authorship/post_commit.rs index 53027373ea..2606f4eaf0 100644 --- a/src/authorship/post_commit.rs +++ b/src/authorship/post_commit.rs @@ -392,6 +392,30 @@ where &commit_sha, context.precomputed_parent_diff, )?; + // Compute the set of committed-coordinate lines that were explicitly displaced to + // INITIAL by the attribution loop (i.e., human-modified lines that replaced AI + // content after the last checkpoint). These are lines present in both the INITIAL + // attributions (carrying their carryover line numbers) and in recovery_hunks. Edge + // recovery must not re-claim these lines. + let displaced_to_initial_lines: HashMap> = initial_attributions + .files + .iter() + .filter_map(|(file, line_attrs)| { + let committed_for_file = recovery_hunks.get(file)?; + let committed_set: HashSet = + committed_for_file.iter().flat_map(|r| r.expand()).collect(); + let displaced: HashSet = 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(); crate::authorship::attribution_recovery::recover_attribution( repo, &parent_sha, @@ -402,6 +426,11 @@ where AttributionRecoveryContext { file_timestamps: context.recovery_file_timestamps, before_external_recovery: context.before_external_recovery, + displaced_to_initial_lines: if displaced_to_initial_lines.is_empty() { + None + } else { + Some(&displaced_to_initial_lines) + }, }, )?; authorship_log.metadata.base_commit_sha = commit_sha.clone(); @@ -685,7 +714,7 @@ pub(crate) fn post_commit_amend_with_recovery_timestamps_detailed( let observed_snapshot = working_log.observed_file_snapshot()?; let mut final_state_snapshot = commit_tree_snapshot_for_files(repo, amended_commit, &pathspecs)?; - final_state_snapshot.extend(observed_snapshot); + final_state_snapshot.extend(observed_snapshot.clone()); // Check if original commit has existing authorship data let has_existing_data = @@ -769,6 +798,25 @@ pub(crate) fn post_commit_amend_with_recovery_timestamps_detailed( } let recovery_hunks = recovery_committed_hunks(repo, &parent_sha, amended_commit, None)?; + let displaced_to_initial_lines_amend: HashMap> = initial_attributions + .files + .iter() + .filter_map(|(file, line_attrs)| { + let committed_for_file = recovery_hunks.get(file)?; + let committed_set: HashSet = + committed_for_file.iter().flat_map(|r| r.expand()).collect(); + let displaced: HashSet = 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(); crate::authorship::attribution_recovery::recover_attribution( repo, &parent_sha, @@ -779,6 +827,11 @@ pub(crate) fn post_commit_amend_with_recovery_timestamps_detailed( AttributionRecoveryContext { file_timestamps: recovery_file_timestamps, before_external_recovery, + displaced_to_initial_lines: if displaced_to_initial_lines_amend.is_empty() { + None + } else { + Some(&displaced_to_initial_lines_amend) + }, }, )?; authorship_log.metadata.base_commit_sha = amended_commit.to_string(); diff --git a/src/authorship/virtual_attribution.rs b/src/authorship/virtual_attribution.rs index e13d02f2b9..8036c78c81 100644 --- a/src/authorship/virtual_attribution.rs +++ b/src/authorship/virtual_attribution.rs @@ -2042,7 +2042,27 @@ fn build_carryover_snapshot( .cloned() .unwrap_or_default() }; - merged_carryover_content_pure(&parent_content, &committed_content, observed_content) + let merged = merged_carryover_content_pure( + &parent_content, + &committed_content, + observed_content, + ); + // If the 3-way merge favored the observed (checkpoint) content over what was + // committed, check whether the observed content is stale — i.e. the commit is + // a superset of observed (every observed line appears in committed in order). + // This happens when the AI checkpointed a file and then a human edited it + // without firing a known_human checkpoint: the last checkpoint blob is the AI + // version, but the committed file contains both AI and human changes. In that + // case there are no uncommitted changes; committed is the ground truth for + // carryover. + if &merged == observed_content + && observed_content != &committed_content + && line_sequence_contains(observed_content, &committed_content) + { + committed_content + } else { + merged + } } else { committed_content }; @@ -2196,48 +2216,47 @@ impl VirtualAttributions { collect_unstaged_hunks(repo, commit_sha, effective_pathspecs)? }; - // IMPORTANT: If a line appears in both committed_hunks and unstaged_hunks, it means: - // - The line was committed in this commit (in commit coordinates) - // - The line was then modified again in the working directory (in workdir coordinates) - // Since both use the same line numbering after the commit (workdir coordinates = commit coordinates - // for the committed state), we can directly compare line numbers. - // We should treat these lines as committed, not unstaged, because the attribution belongs - // to the commit even if there's a subsequent unstaged modification. + // When there is no carryover snapshot, unstaged_hunks reflect the live working + // directory vs the commit. In that case, if a line appears in both committed_hunks + // and unstaged_hunks, the line was committed and then modified again in the working + // tree — we attribute it to the commit and treat the working-tree version as a future + // change, UNLESS it's a pure insertion (which shifts coordinates without representing + // the same logical line). // - // HOWEVER: If a line is a PURE INSERTION (old_count=0), it means a new line was inserted - // at that position, pushing existing lines down. In this case, the line number overlap - // doesn't mean the same line - it's a different line at the same position! - // We should NOT filter out pure insertions even if they overlap with committed line numbers. - for (file_path, committed_ranges) in &committed_hunks { - if let Some(unstaged_ranges) = unstaged_hunks.get_mut(file_path) { - // Expand both to line numbers for comparison - let committed_lines: std::collections::HashSet = - committed_ranges.iter().flat_map(|r| r.expand()).collect(); - - // Get pure insertion lines for this file (these should NOT be filtered out) - let pure_insertion_lines: std::collections::HashSet = pure_insertion_hunks - .get(file_path) - .map(|ranges| ranges.iter().flat_map(|r| r.expand()).collect()) - .unwrap_or_default(); - - // Filter out any unstaged lines that were also committed - // (these are lines that were committed, then modified again in workdir) - // BUT keep pure insertions even if they overlap with committed line numbers - let mut filtered_unstaged_lines: Vec = unstaged_ranges - .iter() - .flat_map(|r| r.expand()) - .filter(|line| { - // Keep the line if it's NOT in committed, OR if it's a pure insertion - !committed_lines.contains(line) || pure_insertion_lines.contains(line) - }) - .collect(); - - if filtered_unstaged_lines.is_empty() { - unstaged_ranges.clear(); - } else { - filtered_unstaged_lines.sort_unstable(); - filtered_unstaged_lines.dedup(); - *unstaged_ranges = LineRange::compress_lines(&filtered_unstaged_lines); + // When a carryover_snapshot IS present, unstaged_hunks come from + // collect_unstaged_hunks_from_snapshot: they express "the carryover (last checkpoint) + // differs from committed at this position." A Replace in that diff means the human + // modified a line after the AI checkpoint without firing a known_human checkpoint, + // and the commit captured the human's version. Filtering those Replace lines out + // would silently re-attribute the human's content to AI. We therefore skip the + // overlap filter entirely when a carryover snapshot is available — the attribution + // loop already handles the committed vs. uncommitted split correctly. + if carryover_snapshot.is_none() { + for (file_path, committed_ranges) in &committed_hunks { + if let Some(unstaged_ranges) = unstaged_hunks.get_mut(file_path) { + let committed_lines: std::collections::HashSet = + committed_ranges.iter().flat_map(|r| r.expand()).collect(); + + let pure_insertion_lines: std::collections::HashSet = pure_insertion_hunks + .get(file_path) + .map(|ranges| ranges.iter().flat_map(|r| r.expand()).collect()) + .unwrap_or_default(); + + let mut filtered_unstaged_lines: Vec = unstaged_ranges + .iter() + .flat_map(|r| r.expand()) + .filter(|line| { + !committed_lines.contains(line) || pure_insertion_lines.contains(line) + }) + .collect(); + + if filtered_unstaged_lines.is_empty() { + unstaged_ranges.clear(); + } else { + filtered_unstaged_lines.sort_unstable(); + filtered_unstaged_lines.dedup(); + *unstaged_ranges = LineRange::compress_lines(&filtered_unstaged_lines); + } } } } @@ -2284,7 +2303,7 @@ impl VirtualAttributions { line_attrs }; - // Get unstaged lines for this file (in working directory coordinates). + // Get unstaged lines for this file (in working directory / carryover coordinates). let mut unstaged_lines: Vec = Vec::new(); let unstaged_lookup = unstaged_hunks.get(&nfc_file_path).or_else(|| { rename_map @@ -2298,11 +2317,33 @@ impl VirtualAttributions { unstaged_lines.sort_unstable(); } + // Pure-insertion lines: carryover lines that were inserted (not replaced). + // Only these cause a coordinate shift between carryover and committed space. + // Replace-type unstaged lines leave the count the same, so do NOT shift + // subsequent lines in the committed coordinate system. + let mut pure_insertion_lines: Vec = Vec::new(); + let pure_insertion_lookup = pure_insertion_hunks.get(&nfc_file_path).or_else(|| { + rename_map + .get(&nfc_file_path) + .and_then(|np| pure_insertion_hunks.get(np)) + }); + if let Some(ranges) = pure_insertion_lookup { + for range in ranges { + pure_insertion_lines.extend(range.expand()); + } + pure_insertion_lines.sort_unstable(); + } + // Split line attributions into committed and uncommitted // VirtualAttributions has line numbers in working directory coordinates, // so we need to convert to commit coordinates before comparing with committed hunks let mut committed_lines_map: StdHashMap> = StdHashMap::new(); let mut uncommitted_lines_map: StdHashMap> = StdHashMap::new(); + // Track committed-coordinate lines that were explicitly resolved (either + // placed in committed_lines_map or displaced to uncommitted due to unstaged + // content). Gap-fill must not override these: lines that ended up unstaged + // had human-modified content and should not be attributed to the AI. + let mut explicitly_resolved_commit_lines: StdHashMap = StdHashMap::new(); // Get the committed hunks for this file (if any) - these are in commit coordinates. // If the file was renamed, committed_hunks is keyed by the new path. @@ -2325,10 +2366,23 @@ impl VirtualAttributions { .or_default() .push(workdir_line_num); referenced_prompts.insert(line_attr.author_id.clone()); + // Record the corresponding committed-coordinate position so that gap-fill + // does not later claim this position for AI attribution. + // Only pure insertions shift coordinates; replacements do not. + // A replaced carryover line W has committed coordinate W - (# pure + // insertions before W). + 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, ()); } else { // Convert working directory line number to commit line number - // by subtracting the count of unstaged lines before this line - let adjustment = unstaged_lines + // by subtracting the count of pure-insertion unstaged lines before this line. + // Replace-type unstaged lines do not shift line numbers; only inserted + // carryover lines (which have no corresponding committed line) shift coords. + let adjustment = pure_insertion_lines .iter() .filter(|&&l| l < workdir_line_num) .count() as u32; @@ -2349,6 +2403,7 @@ impl VirtualAttributions { .entry(line_attr.author_id.clone()) .or_default() .push(commit_line_num); + explicitly_resolved_commit_lines.insert(commit_line_num, ()); } else if is_renamed_file && line_attr.author_id != CheckpointKind::Human.to_str() && !line_attr.author_id.starts_with("h_") @@ -2361,6 +2416,7 @@ impl VirtualAttributions { .entry(line_attr.author_id.clone()) .or_default() .push(commit_line_num); + explicitly_resolved_commit_lines.insert(commit_line_num, ()); } } } @@ -2422,6 +2478,15 @@ impl VirtualAttributions { continue; } + // Skip lines whose committed position was explicitly resolved during the + // attribution loop above (placed in committed_lines_map or displaced to + // uncommitted because the carryover snapshot differed from the commit). + // Filling these positions would override a conscious attribution decision + // — e.g. a human-modified line that replaced an AI-written line. + if explicitly_resolved_commit_lines.contains_key(&line) { + continue; + } + // Find nearest attributed neighbor before this line let prev = line_to_author.iter().rev().find(|(l, _)| *l < line); diff --git a/tests/integration/human_edit_after_ai_checkpoint.rs b/tests/integration/human_edit_after_ai_checkpoint.rs new file mode 100644 index 0000000000..1977231a5e --- /dev/null +++ b/tests/integration/human_edit_after_ai_checkpoint.rs @@ -0,0 +1,238 @@ +/// Regression tests for issue #1444: attribution should not be fully lost when a human +/// edits a file after an AI checkpoint fires but before the commit — without a +/// known_human checkpoint being triggered. +use crate::repos::test_file::ExpectedLineExt; +use crate::repos::test_repo::TestRepo; +use std::fs; + +/// AI writes lines 1-3, human appends lines 4-6 to same file without firing any +/// checkpoint, then commits everything. AI-written lines should remain attributed to AI. +/// Adjacent uncheckpointed lines get AI attribution via edge recovery (up to +/// EDGE_EXTENSION_MAX_LINES = 3 lines). +#[test] +fn test_ai_writes_then_human_appends_no_checkpoint() { + let repo = TestRepo::new(); + let file_path = repo.path().join("example.txt"); + + // Human sets up a base file (untracked — no checkpoint fired) + let base = "\ +Base line +"; + fs::write(&file_path, base).unwrap(); + repo.stage_all_and_commit("Base commit").unwrap(); + + // AI pre-edit snapshot (legacy human = untracked, as the AI preset takes before editing) + repo.git_ai(&["checkpoint", "human", "example.txt"]) + .unwrap(); + + // AI writes lines 2-4 + let after_ai = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +"; + fs::write(&file_path, after_ai).unwrap(); + // AI post-edit checkpoint + repo.git_ai(&["checkpoint", "mock_ai", "example.txt"]) + .unwrap(); + + // Human appends lines 5-7 WITHOUT firing any checkpoint + let after_human = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +Human line 1 +Human line 2 +Human line 3 +"; + fs::write(&file_path, after_human).unwrap(); + + // Commit everything — no known_human checkpoint was fired for the human lines. + // Edge recovery attributes up to 3 adjacent uncheckpointed lines to the neighboring + // AI session, so all three human-appended lines are attributed to AI here. + repo.stage_all_and_commit("Mixed commit").unwrap(); + + let mut file = repo.filename("example.txt"); + file.assert_committed_lines(crate::lines![ + "Base line".unattributed_human(), + "AI line 1".ai(), + "AI line 2".ai(), + "AI line 3".ai(), + "Human line 1".ai(), + "Human line 2".ai(), + "Human line 3".ai(), + ]); +} + +/// AI writes lines 1-5, human modifies line 3 content without firing a checkpoint, +/// then commits. Unmodified AI lines should retain AI attribution; modified line is +/// untracked. +#[test] +fn test_ai_writes_then_human_modifies_within_ai_range() { + let repo = TestRepo::new(); + let file_path = repo.path().join("example.txt"); + + let base = "\ +Base line +"; + fs::write(&file_path, base).unwrap(); + repo.stage_all_and_commit("Base commit").unwrap(); + + repo.git_ai(&["checkpoint", "human", "example.txt"]) + .unwrap(); + + let after_ai = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +AI line 4 +AI line 5 +"; + fs::write(&file_path, after_ai).unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "example.txt"]) + .unwrap(); + + // Human modifies line 3 in-place (no checkpoint) + let after_human = "\ +Base line +AI line 1 +AI line 2 +Human modified line 3 +AI line 4 +AI line 5 +"; + fs::write(&file_path, after_human).unwrap(); + + repo.stage_all_and_commit("Mixed commit").unwrap(); + + let mut file = repo.filename("example.txt"); + file.assert_committed_lines(crate::lines![ + "Base line".unattributed_human(), + "AI line 1".ai(), + "AI line 2".ai(), + "Human modified line 3".unattributed_human(), + "AI line 4".ai(), + "AI line 5".ai(), + ]); +} + +/// AI writes lines, human inserts 2 lines above the AI region (shifting AI lines down) +/// without firing a checkpoint, then commits. AI lines (now at new positions) should +/// retain AI attribution. Adjacent uncheckpointed inserted lines get AI attribution via +/// edge recovery (up to EDGE_EXTENSION_MAX_LINES = 3 lines from the run end toward AI). +#[test] +fn test_ai_writes_then_human_inserts_above() { + let repo = TestRepo::new(); + let file_path = repo.path().join("example.txt"); + + let base = "\ +Base line +"; + fs::write(&file_path, base).unwrap(); + repo.stage_all_and_commit("Base commit").unwrap(); + + repo.git_ai(&["checkpoint", "human", "example.txt"]) + .unwrap(); + + let after_ai = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +"; + fs::write(&file_path, after_ai).unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "example.txt"]) + .unwrap(); + + // Human inserts 2 lines between base and AI lines (no checkpoint) + let after_human = "\ +Base line +Human insert 1 +Human insert 2 +AI line 1 +AI line 2 +AI line 3 +"; + fs::write(&file_path, after_human).unwrap(); + + // Commit everything. Edge recovery takes the 2 inserted lines (≤3) adjacent to the + // AI block and attributes them to the same AI session. + repo.stage_all_and_commit("Mixed commit").unwrap(); + + let mut file = repo.filename("example.txt"); + file.assert_committed_lines(crate::lines![ + "Base line".unattributed_human(), + "Human insert 1".ai(), + "Human insert 2".ai(), + "AI line 1".ai(), + "AI line 2".ai(), + "AI line 3".ai(), + ]); +} + +/// Regression guard: when a human has REAL uncommitted changes (staged only some AI +/// lines), the unstaged changes should still be correctly deferred to INITIAL for the +/// next commit rather than being incorrectly attributed to this commit. +#[test] +fn test_real_uncommitted_changes_still_go_to_initial() { + let repo = TestRepo::new(); + let file_path = repo.path().join("example.txt"); + + let base = "\ +Base line +"; + fs::write(&file_path, base).unwrap(); + repo.stage_all_and_commit("Base commit").unwrap(); + + // AI writes 4 lines + repo.git_ai(&["checkpoint", "human", "example.txt"]) + .unwrap(); + let after_ai = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +AI line 4 +"; + fs::write(&file_path, after_ai).unwrap(); + repo.git_ai(&["checkpoint", "mock_ai", "example.txt"]) + .unwrap(); + + // Stage only first 3 AI lines (4th remains unstaged) + let staged = "\ +Base line +AI line 1 +AI line 2 +AI line 3 +"; + fs::write(&file_path, staged).unwrap(); + repo.git(&["add", "example.txt"]).unwrap(); + + // Restore the full content (4th line is "dirty" / unstaged after git add) + fs::write(&file_path, after_ai).unwrap(); + + repo.commit("Partial AI commit").unwrap(); + + // First commit: only lines 1-3 are committed, line 4 is uncommitted (INITIAL) + let mut file = repo.filename("example.txt"); + file.assert_committed_lines(crate::lines![ + "Base line".unattributed_human(), + "AI line 1".ai(), + "AI line 2".ai(), + "AI line 3".ai(), + ]); + + // Now commit the remaining unstaged line + repo.stage_all_and_commit("Remaining AI line").unwrap(); + + file.assert_committed_lines(crate::lines![ + "Base line".unattributed_human(), + "AI line 1".ai(), + "AI line 2".ai(), + "AI line 3".ai(), + "AI line 4".ai(), + ]); +} diff --git a/tests/integration/main.rs b/tests/integration/main.rs index 4a9834d64b..f7fca3c79b 100644 --- a/tests/integration/main.rs +++ b/tests/integration/main.rs @@ -74,6 +74,7 @@ mod github_copilot_tools; mod github_integration; mod gix_config_tests; mod graphite; +mod human_edit_after_ai_checkpoint; mod ignore_prompts; mod ignore_unit; mod initial_attributions; From d57b2b3e6ac36a970eb0278168ec0df4196d44dc Mon Sep 17 00:00:00 2001 From: github-actions Date: Tue, 7 Jul 2026 09:24:06 -0400 Subject: [PATCH 2/2] fix: guard pure-insertion lines from aliasing committed coords in gap-fill Pure-insertion unstaged lines (present in carryover but absent from the commit) have no committed counterpart. Computing a committed coordinate for them and inserting it into explicitly_resolved_commit_lines fabricates a position that aliases an unrelated committed line, potentially blocking gap-fill attribution for that line. Add a binary_search guard so only Replace-type unstaged lines (which have a real committed counterpart) claim a committed-coordinate slot. Also document the coordinate-space invariant in the displaced_to_initial_lines computation: stale-detection ensures initial_attributions is empty for files with pure insertions, so the direct intersection of carryover and committed coordinates is safe for all currently targeted scenarios. --- src/authorship/post_commit.rs | 10 ++++++++++ src/authorship/virtual_attribution.rs | 20 ++++++++++++++------ 2 files changed, 24 insertions(+), 6 deletions(-) diff --git a/src/authorship/post_commit.rs b/src/authorship/post_commit.rs index 2606f4eaf0..4bd33340db 100644 --- a/src/authorship/post_commit.rs +++ b/src/authorship/post_commit.rs @@ -397,6 +397,15 @@ where // content after the last checkpoint). These are lines present in both the INITIAL // attributions (carrying their carryover line numbers) and in recovery_hunks. Edge // recovery must not re-claim these lines. + // + // Coordinate invariant: initial_attributions line numbers are in carryover/workdir + // coordinates, while recovery_hunks line numbers are in committed coordinates. These + // match here because stale-detection in build_carryover_snapshot replaces the carryover + // with committed content whenever pure insertions are present (the only case that would + // shift coordinates), which empties initial_attributions for those files. The only + // scenario that populates initial_attributions is in-place replacement (no coordinate + // shift), so the direct intersection is safe. If stale-detection logic changes, a + // coordinate-aware conversion using pure_insertion_hunks would be needed here. let displaced_to_initial_lines: HashMap> = initial_attributions .files .iter() @@ -798,6 +807,7 @@ pub(crate) fn post_commit_amend_with_recovery_timestamps_detailed( } let recovery_hunks = recovery_committed_hunks(repo, &parent_sha, amended_commit, None)?; + // See coordinate invariant comment on the equivalent computation in post_commit_detailed. let displaced_to_initial_lines_amend: HashMap> = initial_attributions .files .iter() diff --git a/src/authorship/virtual_attribution.rs b/src/authorship/virtual_attribution.rs index 8036c78c81..6dd420dcf4 100644 --- a/src/authorship/virtual_attribution.rs +++ b/src/authorship/virtual_attribution.rs @@ -2371,12 +2371,20 @@ impl VirtualAttributions { // Only pure insertions shift coordinates; replacements do not. // A replaced carryover line W has committed coordinate W - (# pure // insertions before W). - 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, ()); + // Pure insertions have no committed counterpart, so skip them: their + // fabricated coordinate would alias an unrelated committed line. + if pure_insertion_lines + .binary_search(&workdir_line_num) + .is_err() + { + 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, ()); + } } else { // Convert working directory line number to commit line number // by subtracting the count of pure-insertion unstaged lines before this line.