perf(reset): skip wasted blame in post-reset working-log reconstruction (#1025)#1068
Open
perf(reset): skip wasted blame in post-reset working-log reconstruction (#1025)#1068
Conversation
When `git reset HEAD~` runs with `async_mode = false` (sync/wrapper mode), `reconstruct_working_log_after_reset` was building two VirtualAttributions: 1. old_head VA — blame `target..old_head` — correctly finds AI lines in the unwound commit(s). 2. target VA — blame `target..target` — always an empty range; every line is attributed to a "boundary" (pre-range) commit and mapped to human, so the VA had zero AI attributions. Running `git blame` for every file in the reset diff to produce zero output was O(files × file_size) wasted work. On a 500+ line commit in a large monorepo with many changed files this caused noticeable latency in the wrapper. Fix: replace Step 4 with an empty VirtualAttributions created directly, with no subprocess invocations. The merge step (`merge_attributions_favoring_first`) with an empty secondary VA is a no-op, so correctness is unchanged — all existing 79 reset tests continue to pass and the new large-commit test passes. Closes #1025 Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
The previous wrapper-daemon run hit a pre-existing race condition in test_cross_repo_checkpoint_preserves_local_repo_checkpoint (unrelated to this PR's changes — a dedicated fix branch already exists for it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Expand the Step 4 comment to explain both failure modes: 1. The blame range target..target is always empty (broken from day one) 2. old_head_va already covers pre-range AI lines via working log replay, making a correct target_va redundant regardless Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
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
1 participant
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
Root cause: After a
git reset HEAD~N,reconstruct_working_log_after_resetbuilds twoVirtualAttributionsobjects and merges them to recover AI line attribution. Step 4 built atarget_va— intended to capture AI lines that predate the reset range (lines AI-authored before the target commit, still present in the working directory). But the implementation passedblame_start_commit = target_commit_shaandbase_commit = target_commit_shatonew_for_base_commit, producinggit blame target..targetfor every changed file — an always-empty range where every line is a boundary commit (pre-range, attributed human).target_vaalways had zero AI attributions and never filled any gaps.Additionally,
old_head_vais built viafrom_working_log_for_commit, which replays existing working log entries on top of blame. Any AI lines predating the reset range that git-ai tracks are already carried intoold_head_vathrough that working log replay, so a correcttarget_vawould have been redundant anyway.Fix: Replace Step 4 with an empty
VirtualAttributionsconstructed directly. No subprocess calls. Merge result is identical to before becausetarget_vawas always empty. Verified: removingnew_for_base_commit(target, target)causes zero test regressions across all 269 reset tests.Affected flows:
git reset --soft/--mixed/--mergein both sync (async_mode = false, blocking) and async (daemon) modes — the daemon callsreconstruct_working_log_after_resetdirectly at three sites.Performance impact
Before: N
git blamesubprocess calls fortarget_va(N = files changed in the reset diff), each returning nothing useful — O(files × file_size) wasted work scaling directly with commit size.After: 0 subprocess calls for
target_va.For the scenario in #1025 (large monorepo, 500+ line commit touching many files,
async_mode = false), this halves the total blame work inpost_reset_hook.Test plan
test_reset_large_commit_preserves_attribution(wrapper + worktree modes)Fixes #1025