fix(server): detect file renames in review diffs - #8086
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Want fixes drafted automatically? Bugbot Autofix can create code changes for findings. A team admin can enable Autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4234e41. Configure here.
ApprovabilityVerdict: Not approved Macroscope's review found this PR not approvable — The server-side review-diff pipeline is substantially reworked for worktrees with untracked files, including temporary Git-index handling, staged-deletion filtering, fallback logic, and new rename rendering. Although the scope is confined to review previews and includes regression coverage, the runtime behavior and implementation complexity merit human review. You can add or adjust custom eligibility rules. Learn more. |
4234e41 to
49a8e8e
Compare

Supersedes #3916 with a smaller implementation rebased onto the current UI and server architecture.
Problem
A filesystem rename appears as an unrelated deletion and addition in the Working tree review because tracked and untracked patches are generated separately.
Fix
git diff --find-renamesonce across tracked and untracked changes.The temporary index is necessary because Git cannot correlate a tracked deletion with an untracked destination unless both paths participate in the same diff. The user's real index is never modified, and the temporary file is removed when the scoped operation finishes.
Before
The current UI renders an exact filesystem rename as a deletion plus an addition:
After
The same fixture renders as one rename:
Rename with a minor edit
Rename detection is preserved when the destination also changes—in this fixture, one line is edited after the file is moved:
Performance
The normal path with no untracked files still uses two Git processes. When untracked files exist, the previous implementation used
N + 2processes (onediff --no-indexper untracked file); this implementation uses a fixed five, with the independent staged-deletion and index-path lookups run concurrently. That is two extra processes for one untracked file, equal at three, and fewer for four or more.On a tiny exact-rename fixture over 100 measured runs after 10 warmups, the one-file worst case increased from 16.6 ms median / 19.7 ms p95 to 33.9 ms median / 38.4 ms p95. In exchange, the cost no longer grows one Git process per untracked file and Git can detect renames correctly.
Validation
vp test run apps/server/src/vcs/GitVcsDriverCore.test.ts(54 tests)Generated with GPT-5.6-Sol via T3 Code's Codex harness.
Note
Fix file rename detection in
GitVcsDriverCorereview diffsgetReviewDiffPreviewwith a unified diff pipeline that adds--find-renamesto all trackedgit diffcallsreadUnifiedWorkingTreeReviewDiffstages untracked files with--intent-to-addinto a temp copy of the index (viaGIT_INDEX_FILE) so the real index is never mutated, and filters untracked paths that have staged deletionsreadWorkingTreeReviewDiffprefers the unified path and falls back to the old concatenated approach on failure; truncation state is propagated through all pathsreadUnifiedWorkingTreeReviewDiffsetsGIT_INDEX_FILEto a temp file; if cleanup fails, subsequent git commands in the process could target a stale index. The scoped context should restore the env var, but reviewers should confirm the temp file is removed on all exit pathsMacroscope summarized 49a8e8e.
Note
Medium Risk
Touches Git review-diff generation and uses a copied index plus
GIT_INDEX_FILE/intent-to-add. The real index is not modified and failures fall back to the old path, but incorrect path filtering could still mis-render staged deletions or untracked files.Overview
Working-tree review diffs now treat filesystem renames as a single rename (including edits) instead of an unrelated delete plus add.
When untracked files exist, the driver copies the Git index into a scoped temp file,
intent-to-adds those paths there, and runs onegit diff --find-renamesagainstHEAD. The real index is never written. Staged deletions whose paths still exist on disk are excluded from that add so they stay deletions. If temp-index setup fails, it falls back to the previous separate tracked/untracked patches.Tests cover rename-with-edit without mutating a split index, staged deletions, and untracked files before the first commit.
Reviewed by Cursor Bugbot for commit 49a8e8e. Bugbot is set up for automated code reviews on this repo. Configure here.