Merge pull request #270 from acunniffe/fix/ai-line-attr-imara-bugs

Fix subtle intra-line deletion tracking bug

Author: svarlamov

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

tests/simple_additions.rs

@@ -1008,3 +1008,113 @@ This project is open source and available for use.
         "This project is open source and available for use.".ai(),
     ]);
 }
+
+#[test]
+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:
+    // - 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();
+
+    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(),
+    ]);
+}
+
+#[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(),
+    ]);
+}