feat: rename-aware diffs with old → new origin header#223
Merged
Conversation
Implementation plan for rename-aware diffs and old → new origin display. Related to #222
FileEntry gains OldPath, populated by ChangedFiles from git's rename pair (forced with -M so it works regardless of diff.renames config). FileDiff takes a FileDiffRequest carrying the rename origin; (*Git).FileDiff passes -M and both paths so a renamed-but-edited file shows only its real line changes instead of rendering as fully added, across --staged, single-ref, and two-ref modes. Non-git renderers ignore OldPath. Related to #222
The file tree gains an OldPath(path) accessor parallel to FileStatus; loadFileDiff threads the selected file's rename origin into the request and the diff-pane header shows "old → new" instead of just the new name. Related to #222
ComputeStats passes OldPath so a renamed-but-edited file contributes only its real +/- counts to the overlay footer, not all-lines-added. Related to #222
The preloader carries a new→old path map and passes OldPath into FileDiff so saved annotations on a renamed file map against the rename-aware diff's line set instead of the all-added version. Related to #222
README, docs.html, CLAUDE.md, and ARCHITECTURE.md describe the rename origin header and rename-aware diff behavior. Related to #222
There was a problem hiding this comment.
Pull request overview
This PR fixes rename handling in Git-backed reviews so renamed-and-edited files diff against their origin (instead of rendering as a full add), and the diff pane header shows the rename origin as old → new. It does this by preserving the old path from git diff --name-status, threading it through a new diff.FileDiffRequest, and making the Git renderer request rename-aware diffs by passing both paths with -M.
Changes:
- Preserve rename origin path (
OldPath) indiff.FileEntryand force rename detection in Git changed-file discovery. - Replace positional
FileDiffarguments across the codebase withdiff.FileDiffRequest, and make Git diffs rename-aware whenOldPathis provided. - Wire
OldPaththrough UI header, review stats, and annotation preloading; update docs and tests accordingly.
Reviewed changes
Copilot reviewed 44 out of 45 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| site/docs.html | Documents rename-aware behavior and Git-only scope. |
| README.md | Adds user-facing feature note for rename-aware diffs. |
| docs/plans/completed/20260606-rename-aware-diffs.md | Captures completed implementation plan and rationale for the change. |
| docs/ARCHITECTURE.md | Updates architecture docs for FileDiffRequest + rename-aware Git diff behavior. |
| CLAUDE.md | Adds repo gotcha notes for OldPath/FileDiffRequest threading and Git-only rename support. |
| app/ui/view.go | Renders old → new title in diff pane header when origin is known. |
| app/ui/view_test.go | Tests header rendering for rename vs non-rename cases. |
| app/ui/themeselect_test.go | Updates renderer mock signature usage to FileDiffRequest. |
| app/ui/sidepane/filetree.go | Stores and exposes rename origins via OldPath(path) accessor. |
| app/ui/sidepane/filetree_test.go | Tests FileTree.OldPath behavior and rebuild clearing/refresh. |
| app/ui/reviewinfo_test.go | Updates mocks/call sites to new FileDiffRequest signature. |
| app/ui/model.go | Updates ui.Renderer interface FileDiff signature; threads oldName via message/state. |
| app/ui/model_test.go | Updates renderer mocks and lookups to use FileDiffRequest. |
| app/ui/mocks/renderer.go | Regenerated/updated moq for new FileDiff(req) signature and call capture. |
| app/ui/loaders.go | Threads OldPath from file tree into diff load requests and loaded messages; staged fallback updated. |
| app/ui/loaders_test.go | Verifies loader threads rename origin into request/message/model. |
| app/ui/handlers_test.go | Updates renderer mocks to the new signature. |
| app/ui/diffview_test.go | Updates renderer mocks to the new signature. |
| app/ui/annotate_test.go | Updates renderer mocks to the new signature. |
| app/review/stats.go | Passes OldPath into FileDiffRequest for accurate rename-aware stats (incl. staged fallback). |
| app/review/stats_test.go | Adds tests ensuring OldPath is threaded and counts reflect minimal rename diffs. |
| app/renderer_setup_test.go | Updates renderer mocks to the new signature. |
| app/diff/stdin.go | Migrates stdin reader to FileDiffRequest (Path-based lookup). |
| app/diff/multistdin.go | Migrates multi-stdin reader to FileDiffRequest (Path-based lookup). |
| app/diff/multistdin_test.go | Updates tests for new FileDiffRequest signature. |
| app/diff/mocks/Renderer.go | Regenerated/updated moq for diff.Renderer new FileDiff(req) signature. |
| app/diff/jj.go | Migrates jj renderer to FileDiffRequest (uses req.Ref, req.Path, req.ContextLines). |
| app/diff/jj_test.go | Updates jj tests for new FileDiffRequest signature. |
| app/diff/include.go | Migrates include filter to pass through FileDiffRequest. |
| app/diff/include_test.go | Updates include filter tests for request-based signature and context threading. |
| app/diff/hg.go | Migrates hg renderer to FileDiffRequest. |
| app/diff/hg_test.go | Updates hg tests for new FileDiffRequest signature. |
| app/diff/hg_e2e_test.go | Updates hg/git e2e tests for new FileDiffRequest signature. |
| app/diff/fallback.go | Migrates fallback renderer + file reader to FileDiffRequest; uses req.Path for disk fallback. |
| app/diff/fallback_test.go | Updates fallback tests for new FileDiffRequest signature and context threading. |
| app/diff/exclude.go | Migrates exclude filter to pass through FileDiffRequest. |
| app/diff/exclude_test.go | Updates exclude filter tests for request-based signature and context threading. |
| app/diff/directory.go | Migrates directory reader to FileDiffRequest (Path-based disk read). |
| app/diff/directory_test.go | Updates directory reader tests for new FileDiffRequest signature. |
| app/diff/diff.go | Adds FileEntry.OldPath, introduces FileDiffRequest, forces -M in ChangedFiles, adds Git rename-aware pathArgs, updates old-line/binary probes. |
| app/diff/diff_test.go | Adds staged/refs rename reproduction tests and acceptance test for issue #222; updates existing tests to request-based API. |
| app/diff/compare.go | Migrates compare reader to FileDiffRequest (uses req.ContextLines). |
| app/diff/compare_test.go | Updates compare reader tests for new FileDiffRequest signature. |
| app/annotations_load.go | Threads rename origin into annotation preload diffs via renames map and FileDiffRequest.OldPath. |
| app/annotations_load_test.go | Adds tests ensuring rename-aware diff is used for annotation mapping; updates mocks to request-based signature. |
Files not reviewed (1)
- app/diff/mocks/Renderer.go: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
renamed files now diff against their origin instead of rendering as fully added, and the diff pane header shows
old → new.the bug:
ChangedFilesparsed git's rename pair but dropped the old path, andFileDiffrangit diff -- <newpath>. The single-path pathspec stripped the old side before git could pair the rename, so a renamed-but-edited file showed every line as added and the origin was never visible. Reproduced across--staged, single-ref, and two-ref modes.the fix, by layer:
FileEntrygainsOldPath, populated byChangedFiles(forced with-Mso it works regardless of the user'sdiff.renamesconfig)FileDifftakes aFileDiffRequestcarrying the rename origin;(*Git).FileDiffpasses-Mand both paths so only the real line changes showold → new, sourced from a newFileTree.OldPathaccessor parallel toFileStatus--annotationspreloader passOldPathtoo, so footer counts and saved-annotation line matching agree with the displayed diffnon-git renderers ignore
OldPath, so hg/jj/file/stdin/compare modes are unchanged. Rename-origin support for hg and jj is out of scope here, neither reports an origin today.tests cover the issue's exact reproduction across all three git modes, the
diff.renames=offedge, the header wiring, stats, and annotation mapping. Full suite green with-race, lint clean.Related to #222