Skip to content

Commit 630482f

Browse files
committed
fix subtle intra-line deletion attribution tracking bug
1 parent 38ec509 commit 630482f

File tree

2 files changed

+71
-5
lines changed

2 files changed

+71
-5
lines changed

src/authorship/attribution_tracker.rs

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -826,8 +826,17 @@ impl AttributionTracker {
826826
}
827827
}
828828
}
829+
} else if !data_is_whitespace(diff.data()) {
830+
// For non-move deletions of substantive content, create a zero-length
831+
// marker attribution at the deletion point. This ensures lines with
832+
// deletions get attributed to the deleting author.
833+
new_attributions.push(Attribution::new(
834+
new_pos,
835+
new_pos, // Zero-length marker
836+
current_author.to_string(),
837+
ts,
838+
));
829839
}
830-
// else: True deletion - attributions are lost
831840

832841
old_pos += len;
833842
deletion_idx += 1;
@@ -1497,7 +1506,10 @@ fn find_dominant_author_for_line(
14971506
..std::cmp::min(line_end, attribution.end)];
14981507
let attr_non_whitespace_count =
14991508
content_slice.chars().filter(|c| !c.is_whitespace()).count();
1500-
if attr_non_whitespace_count > 0 || is_line_empty {
1509+
// Zero-length attributions are deletion markers - they indicate the author
1510+
// deleted content at this position, so they should influence line attribution
1511+
let is_deletion_marker = attribution.start == attribution.end;
1512+
if attr_non_whitespace_count > 0 || is_line_empty || is_deletion_marker {
15011513
candidate_attrs.push(attribution.clone());
15021514
} else {
15031515
// If the attribution is only whitespace, discard it

tests/simple_additions.rs

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1010,7 +1010,7 @@ This project is open source and available for use.
10101010
}
10111011

10121012
#[test]
1013-
fn test_constructor_parameter_removal_attribution_bug() {
1013+
fn test_deletion_within_a_single_line_attribution() {
10141014
// Regression test for bug where removing a constructor parameter
10151015
// doesn't get attributed to AI when using mock_ai checkpoint
10161016
// This replicates the scenario where:
@@ -1043,8 +1043,7 @@ fn test_constructor_parameter_removal_attribution_bug() {
10431043
repo.git_ai(&["checkpoint", "mock_ai", "git-ai-integration-service.ts"])
10441044
.unwrap();
10451045

1046-
let commit = repo
1047-
.stage_all_and_commit("AI removes constructor parameter")
1046+
repo.stage_all_and_commit("AI removes constructor parameter")
10481047
.unwrap();
10491048

10501049
// Verify line-by-line attribution - the constructor line should be AI
@@ -1064,3 +1063,58 @@ fn test_constructor_parameter_removal_attribution_bug() {
10641063
"}".human(),
10651064
]);
10661065
}
1066+
1067+
#[test]
1068+
fn test_deletion_of_multiple_lines_by_ai() {
1069+
// Regression test for bug where removing a constructor parameter
1070+
// doesn't get attributed to AI when using mock_ai checkpoint
1071+
// This replicates the scenario where:
1072+
// - constructor(_config: Config, enabled: boolean = true) { [no-data]
1073+
// + constructor(enabled: boolean = true) { [no-data]
1074+
// The constructor line should be attributed to AI
1075+
use std::fs;
1076+
1077+
let repo = TestRepo::new();
1078+
let file_path = repo.path().join("git-ai-integration-service.ts");
1079+
1080+
// Initial commit: File with old constructor signature (all human)
1081+
fs::write(
1082+
&file_path,
1083+
"/**\n * Service for integrating git-ai hooks into the hook system.\n */\nexport class GitAiIntegrationService {\n private readonly commandPath: string;\n private registered = false;\n\n constructor(_config: Config, enabled: boolean = true) {\n this.enabled = enabled;\n this.commandPath = 'git-ai';\n }\n}\n",
1084+
)
1085+
.unwrap();
1086+
1087+
repo.git_ai(&["checkpoint"]).unwrap();
1088+
repo.stage_all_and_commit("Initial commit with old constructor").unwrap();
1089+
1090+
// Second commit: AI removes the _config parameter
1091+
fs::write(
1092+
&file_path,
1093+
"/**\n * Service for integrating git-ai hooks into the hook system.\n */\nexport class GitAiIntegrationService {\n private readonly commandPath: string;\n constructor(_config: Config, enabled: boolean = true) {\n this.commandPath = 'git-ai';\n }\n}\n",
1094+
)
1095+
.unwrap();
1096+
1097+
// Mark the change as AI-authored
1098+
repo.git_ai(&["checkpoint", "mock_ai", "git-ai-integration-service.ts"])
1099+
.unwrap();
1100+
1101+
repo.stage_all_and_commit("AI removes constructor parameter")
1102+
.unwrap();
1103+
1104+
// Verify line-by-line attribution - the constructor line should be AI
1105+
let mut file = repo.filename("git-ai-integration-service.ts");
1106+
file.assert_lines_and_blame(lines![
1107+
"/**".human(),
1108+
" * Service for integrating git-ai hooks into the hook system.".human(),
1109+
" */".human(),
1110+
"export class GitAiIntegrationService {".human(),
1111+
" private readonly commandPath: string;".human(),
1112+
// " private registered = false;".human(),
1113+
// "".human(),
1114+
" constructor(_config: Config, enabled: boolean = true) {".human(),
1115+
// " this.enabled = enabled;".human(),
1116+
" this.commandPath = 'git-ai';".human(),
1117+
" }".human(),
1118+
"}".human(),
1119+
]);
1120+
}

0 commit comments

Comments
 (0)