fix: correctly attribute AI lines in merge commit stats#913
Open
fix: correctly attribute AI lines in merge commit stats#913
Conversation
Fixes #910 When AI resolves a merge conflict, `git ai blame` correctly attributes lines to the AI, but `git ai stats head` was incorrectly showing 100% human / 0% AI additions. Three root causes: 1. `accepted_lines_from_attestations()` had an early return that always returned (0, empty) for merge commits, skipping attestation matching. 2. `stats_for_commit_stats()` set `added_lines_by_file` to an empty HashMap for merge commits, so even without the early return there were no lines to match against attestations. 3. `git show --numstat` uses the combined-diff format for merge commits, which only shows files that differ from ALL parents (conflict resolutions). This misses cleanly-merged changes. The fix uses `git diff commit^1 commit --numstat` to diff against the first parent instead. Changes: - Remove `is_merge_commit` parameter and early return from `accepted_lines_from_attestations()` - Always compute `added_lines_by_file` by diffing against the first parent (works for both regular and merge commits) - Add `get_git_diff_stats_first_parent()` helper for merge commit numstat via `git diff commit^1 commit --numstat` - Extract `parse_numstat_output()` to share parsing logic between the two numstat code paths - Add two TestRepo-based integration tests that replicate the bug - Update unit tests to reflect the corrected behavior Co-Authored-By: Sasha Varlamov <sasha@sashavarlamov.com>
Contributor
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #910. When AI resolves a merge conflict,
git ai blamecorrectly attributes lines to the AI, butgit ai stats headwas showing 100% human / 0% AI. Three root causes:accepted_lines_from_attestations()early-returned(0, empty)for all merge commits, unconditionally skipping attestation matching.stats_for_commit_stats()setadded_lines_by_fileto an empty HashMap for merge commits, so there were no lines to match against attestations even if the early return were removed.git show --numstatuses combined-diff format for merge commits, which only shows files differing from ALL parents (conflict resolutions), missing cleanly-merged changes. Replaced withgit diff commit^1 commit --numstatfor merge commits.What changed
is_merge_commitparameter and early return fromaccepted_lines_from_attestations()added_lines_by_fileis now always computed by diffing against the first parent (works for both regular and merge commits)get_git_diff_stats_first_parent()helper usesgit diff commit^1..commit --numstatparse_numstat_output()to share parsing logic between the two numstat pathsReview & Testing Checklist for Human
git_diff_added_lines > 0for all changes vs first parent. Ifgit ai stats <range>sums across both the feature branch commits AND the merge commit, the same lines could be counted twice. Verify how range stats aggregation works and whether this is an issue.commit^1(first parent). Verify this is correct for octopus merges (3+ parents) — should it always be the first parent?human_additions=0, git_diff_added_lines=0. Now they show the actual first-parent diff (e.g.,human_additions=1, git_diff_added_lines=1). Confirm this is the desired behavior.git ai stats headand confirm the output now shows AI attribution correctly.Notes
git merge --no-commit -X theirs→git ai checkpoint mock_ai <file>→stage_all_and_commit. This mirrors what happens when an AI tool resolves conflicts and git-ai's checkpoint mechanism tracks it.Link to Devin session: https://app.devin.ai/sessions/6bd8df4f03d94a51a75575a7769f9b4f
Requested by: @svarlamov