From fc601013ef7d24a6075ceca1720f082ae18e7caa Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Sat, 6 Dec 2025 15:14:43 -0500 Subject: [PATCH 1/2] add failing regression test --- tests/simple_additions.rs | 56 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 56 insertions(+) diff --git a/tests/simple_additions.rs b/tests/simple_additions.rs index 291816d4..abf7585b 100644 --- a/tests/simple_additions.rs +++ b/tests/simple_additions.rs @@ -1008,3 +1008,59 @@ This project is open source and available for use. "This project is open source and available for use.".ai(), ]); } + +#[test] +fn test_constructor_parameter_removal_attribution_bug() { + // Regression test for bug where removing a constructor parameter + // doesn't get attributed to AI when using mock_ai checkpoint + // This replicates the scenario where: + // - constructor(_config: Config, enabled: boolean = true) { [no-data] + // + constructor(enabled: boolean = true) { [no-data] + // The constructor line should be attributed to AI + use std::fs; + + let repo = TestRepo::new(); + let file_path = repo.path().join("git-ai-integration-service.ts"); + + // Initial commit: File with old constructor signature (all human) + fs::write( + &file_path, + "/**\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", + ) + .unwrap(); + + repo.git_ai(&["checkpoint"]).unwrap(); + repo.stage_all_and_commit("Initial commit with old constructor").unwrap(); + + // Second commit: AI removes the _config parameter + fs::write( + &file_path, + "/**\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(enabled: boolean = true) {\n this.enabled = enabled;\n this.commandPath = 'git-ai';\n }\n}\n", + ) + .unwrap(); + + // Mark the change as AI-authored + repo.git_ai(&["checkpoint", "mock_ai", "git-ai-integration-service.ts"]) + .unwrap(); + + let commit = repo + .stage_all_and_commit("AI removes constructor parameter") + .unwrap(); + + // Verify line-by-line attribution - the constructor line should be AI + let mut file = repo.filename("git-ai-integration-service.ts"); + file.assert_lines_and_blame(lines![ + "/**".human(), + " * Service for integrating git-ai hooks into the hook system.".human(), + " */".human(), + "export class GitAiIntegrationService {".human(), + " private readonly commandPath: string;".human(), + " private registered = false;".human(), + "".human(), + " constructor(enabled: boolean = true) {".ai(), // Should be AI, not [no-data] + " this.enabled = enabled;".human(), + " this.commandPath = 'git-ai';".human(), + " }".human(), + "}".human(), + ]); +} From b37053bc893fb445f8a85b9027cb61a230cf1558 Mon Sep 17 00:00:00 2001 From: Sasha Varlamov Date: Sat, 6 Dec 2025 15:57:31 -0500 Subject: [PATCH 2/2] fix subtle intra-line deletion attribution tracking bug --- src/authorship/attribution_tracker.rs | 16 ++++++- tests/simple_additions.rs | 60 +++++++++++++++++++++++++-- 2 files changed, 71 insertions(+), 5 deletions(-) diff --git a/src/authorship/attribution_tracker.rs b/src/authorship/attribution_tracker.rs index 65ba3987..eb5da9ab 100644 --- a/src/authorship/attribution_tracker.rs +++ b/src/authorship/attribution_tracker.rs @@ -826,8 +826,17 @@ impl AttributionTracker { } } } + } else if !data_is_whitespace(diff.data()) { + // For non-move deletions of substantive content, create a zero-length + // marker attribution at the deletion point. This ensures lines with + // deletions get attributed to the deleting author. + new_attributions.push(Attribution::new( + new_pos, + new_pos, // Zero-length marker + current_author.to_string(), + ts, + )); } - // else: True deletion - attributions are lost old_pos += len; deletion_idx += 1; @@ -1497,7 +1506,10 @@ fn find_dominant_author_for_line( ..std::cmp::min(line_end, attribution.end)]; let attr_non_whitespace_count = content_slice.chars().filter(|c| !c.is_whitespace()).count(); - if attr_non_whitespace_count > 0 || is_line_empty { + // Zero-length attributions are deletion markers - they indicate the author + // deleted content at this position, so they should influence line attribution + let is_deletion_marker = attribution.start == attribution.end; + if attr_non_whitespace_count > 0 || is_line_empty || is_deletion_marker { candidate_attrs.push(attribution.clone()); } else { // If the attribution is only whitespace, discard it diff --git a/tests/simple_additions.rs b/tests/simple_additions.rs index abf7585b..8f767fcf 100644 --- a/tests/simple_additions.rs +++ b/tests/simple_additions.rs @@ -1010,7 +1010,7 @@ This project is open source and available for use. } #[test] -fn test_constructor_parameter_removal_attribution_bug() { +fn test_deletion_within_a_single_line_attribution() { // Regression test for bug where removing a constructor parameter // doesn't get attributed to AI when using mock_ai checkpoint // This replicates the scenario where: @@ -1043,8 +1043,7 @@ fn test_constructor_parameter_removal_attribution_bug() { repo.git_ai(&["checkpoint", "mock_ai", "git-ai-integration-service.ts"]) .unwrap(); - let commit = repo - .stage_all_and_commit("AI removes constructor parameter") + repo.stage_all_and_commit("AI removes constructor parameter") .unwrap(); // Verify line-by-line attribution - the constructor line should be AI @@ -1064,3 +1063,58 @@ fn test_constructor_parameter_removal_attribution_bug() { "}".human(), ]); } + +#[test] +fn test_deletion_of_multiple_lines_by_ai() { + // Regression test for bug where removing a constructor parameter + // doesn't get attributed to AI when using mock_ai checkpoint + // This replicates the scenario where: + // - constructor(_config: Config, enabled: boolean = true) { [no-data] + // + constructor(enabled: boolean = true) { [no-data] + // The constructor line should be attributed to AI + use std::fs; + + let repo = TestRepo::new(); + let file_path = repo.path().join("git-ai-integration-service.ts"); + + // Initial commit: File with old constructor signature (all human) + fs::write( + &file_path, + "/**\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", + ) + .unwrap(); + + repo.git_ai(&["checkpoint"]).unwrap(); + repo.stage_all_and_commit("Initial commit with old constructor").unwrap(); + + // Second commit: AI removes the _config parameter + fs::write( + &file_path, + "/**\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", + ) + .unwrap(); + + // Mark the change as AI-authored + repo.git_ai(&["checkpoint", "mock_ai", "git-ai-integration-service.ts"]) + .unwrap(); + + repo.stage_all_and_commit("AI removes constructor parameter") + .unwrap(); + + // Verify line-by-line attribution - the constructor line should be AI + let mut file = repo.filename("git-ai-integration-service.ts"); + file.assert_lines_and_blame(lines![ + "/**".human(), + " * Service for integrating git-ai hooks into the hook system.".human(), + " */".human(), + "export class GitAiIntegrationService {".human(), + " private readonly commandPath: string;".human(), + // " private registered = false;".human(), + // "".human(), + " constructor(_config: Config, enabled: boolean = true) {".human(), + // " this.enabled = enabled;".human(), + " this.commandPath = 'git-ai';".human(), + " }".human(), + "}".human(), + ]); +}