fix: diff a renamed pr file against its previous path

.demo/demo.tape

@@ -27,6 +27,7 @@ Output .demo/demo.mp4
 
 Set Shell "zsh"
 Set FontFamily "JetBrainsMono Nerd Font Mono"
+Set Padding 0
 Set FontSize 13
 Set Width 1280
 Set Height 760

.demo/fake-sidecar/fixtures.go

@@ -21,22 +21,21 @@ const (
 	other   = "octocat"  // the third user with a pre-existing comment
 )
 
-// the two changed files, base and head. theme.lua flips monokai -> dracula, widens
-// context, and defaults the theme in setup() (a second, separate hunk to land a fresh
-// review comment on); palette.lua gains the accent colour the theme references.
-var fileVersions = map[string][2]string{
-	"lua/theme.lua": {
-		// base
-		"local M = {}\n\nM.theme = \"monokai\"\nM.context = 10\n\nfunction M.setup()\n  return M.theme\nend\n\nreturn M\n",
-		// head
-		"local M = {}\n\nM.theme = \"dracula\"\nM.context = 20\nM.accent = \"#bd93f9\"\n\nfunction M.setup()\n  M.theme = M.theme or \"dracula\"\n  return M.theme\nend\n\nreturn M\n",
-	},
-	"lua/palette.lua": {
-		// base
-		"local M = {}\n\nM.colours = {\n  red = \"#ff5555\",\n  green = \"#50fa7b\",\n}\n\nreturn M\n",
-		// head
-		"local M = {}\n\nM.colours = {\n  red = \"#ff5555\",\n  green = \"#50fa7b\",\n  purple = \"#bd93f9\",\n}\n\nreturn M\n",
-	},
+// the changed files as they exist at each ref, keyed by their path at that ref. theme.lua
+// flips monokai -> dracula, widens context, and defaults the theme in setup() (a second,
+// separate hunk to land a fresh review comment on); palette.lua gains the accent colour
+// the theme references. monokai.lua -> dracula.lua is the rename: it exists under one
+// name at the base and the other at the head, which is all github records of a move.
+var baseBlobs = map[string]string{
+	"lua/theme.lua":   "local M = {}\n\nM.theme = \"monokai\"\nM.context = 10\n\nfunction M.setup()\n  return M.theme\nend\n\nreturn M\n",
+	"lua/palette.lua": "local M = {}\n\nM.colours = {\n  red = \"#ff5555\",\n  green = \"#50fa7b\",\n}\n\nreturn M\n",
+	"lua/monokai.lua": "local M = {}\n\nM.name = \"monokai\"\nM.bg = \"#272822\"\nM.fg = \"#f8f8f2\"\nM.comment = \"#75715e\"\n\nreturn M\n",
+}
+
+var headBlobs = map[string]string{
+	"lua/theme.lua":   "local M = {}\n\nM.theme = \"dracula\"\nM.context = 20\nM.accent = \"#bd93f9\"\n\nfunction M.setup()\n  M.theme = M.theme or \"dracula\"\n  return M.theme\nend\n\nreturn M\n",
+	"lua/palette.lua": "local M = {}\n\nM.colours = {\n  red = \"#ff5555\",\n  green = \"#50fa7b\",\n  purple = \"#bd93f9\",\n}\n\nreturn M\n",
+	"lua/dracula.lua": "local M = {}\n\nM.name = \"dracula\"\nM.bg = \"#282a36\"\nM.fg = \"#f8f8f2\"\nM.comment = \"#6272a4\"\n\nreturn M\n",
 }
 
 // fixture is the in-memory github stand-in. it satisfies handlers.API and keeps the
@@ -108,6 +107,7 @@ func (f *fixture) GetPR(_ context.Context, _, _ string, _ int) (*github.PRDetail
 		Files: []github.PRFile{
 			{Path: "lua/theme.lua", Status: "modified", Additions: 4, Deletions: 2, ViewedState: f.viewedState("lua/theme.lua")},
 			{Path: "lua/palette.lua", Status: "modified", Additions: 1, Deletions: 0, ViewedState: f.viewedState("lua/palette.lua")},
+			{Path: "lua/dracula.lua", Status: "renamed", PreviousPath: "lua/monokai.lua", Additions: 3, Deletions: 3, ViewedState: f.viewedState("lua/dracula.lua")},
 		},
 	}, nil
 }
@@ -120,17 +120,26 @@ func (f *fixture) viewedState(path string) string {
 	return "UNVIEWED"
 }
 
-func (f *fixture) GetFileVersions(_ context.Context, _, _ string, _ int, path, _, _ string) (*github.FileVersions, error) {
-	v, ok := fileVersions[path]
-	if !ok {
-		return &github.FileVersions{Base: github.FileBlob{Missing: true}, Head: github.FileBlob{Missing: true}}, nil
+func (f *fixture) GetFileVersions(_ context.Context, _, _ string, _ int, path, basePath, _, _ string) (*github.FileVersions, error) {
+	if basePath == "" {
+		basePath = path
 	}
 	return &github.FileVersions{
-		Base: github.FileBlob{Content: v[0]},
-		Head: github.FileBlob{Content: v[1]},
+		Base: blobAt(baseBlobs, basePath),
+		Head: blobAt(headBlobs, path),
 	}, nil
 }
 
+// blobAt stands in for the Contents API: a path absent at that ref reads as missing,
+// which is how an add, a delete and a rename's two half-present paths all render.
+func blobAt(blobs map[string]string, path string) github.FileBlob {
+	content, ok := blobs[path]
+	if !ok {
+		return github.FileBlob{Missing: true}
+	}
+	return github.FileBlob{Content: content}
+}
+
 func (f *fixture) HeadSHA(_ context.Context, _, _ string, _ int) (string, error) {
 	return headSHA, nil
 }

CHANGELOG.md

@@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).
 
 ## [Unreleased]
 
+### Fixed
+
+- A renamed file in a PR review showed every line as new, which wasn't very useful. It now diffs against the file's old path, showing only what actually changed
+
+## [0.1.40] — 2026-09-06
+
 ### Added
 
 - `:checkhealth differ` warns when `status.renames` and `diff.renames` disagree, which is what leaves the file panel listing a rename as an add and a delete where a rev-pair diff lists it as a rename

internal/github/cache_test.go

@@ -27,7 +27,7 @@ func TestBlobCacheServesContents(t *testing.T) {
 		return nil, nil
 	})
 	for range 2 {
-		if _, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "", ""); err != nil {
+		if _, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "", "", ""); err != nil {
 			t.Fatal(err)
 		}
 	}
@@ -49,11 +49,11 @@ func TestClearCacheFlushesBlobs(t *testing.T) {
 		return resp(200, `{"base":{"sha":"BASE"},"head":{"sha":"HEAD"}}`, nil), nil
 	})
 	ctx := context.Background()
-	if _, err := c.GetFileVersions(ctx, "o", "r", 3, "a.go", "", ""); err != nil {
+	if _, err := c.GetFileVersions(ctx, "o", "r", 3, "a.go", "", "", ""); err != nil {
 		t.Fatal(err)
 	}
 	c.ClearCache()
-	if _, err := c.GetFileVersions(ctx, "o", "r", 3, "a.go", "", ""); err != nil {
+	if _, err := c.GetFileVersions(ctx, "o", "r", 3, "a.go", "", "", ""); err != nil {
 		t.Fatal(err)
 	}
 	if contentCalls.Load() != 4 {

internal/github/files.go

@@ -10,13 +10,14 @@ import (
 	"github.com/undont/differ.nvim/internal/protocol"
 )
 
-// GetFileVersions returns the full base and head blobs for one path in a PR. the
-// caller passes the pinned base/head shas (from get_pr) so the prRefs round-trip is
-// skipped and the exact session blobs are fetched; an empty sha falls back to
-// resolving it. the two sides are fetched concurrently (independent immutable blobs,
-// cache is mutex-guarded for fan-out). a 404 on a side marks it missing (an added
-// file has no base, a deleted file has no head) rather than failing the call.
-func (c *Client) GetFileVersions(ctx context.Context, owner, repo string, number int, path, base, head string) (*FileVersions, error) {
+// GetFileVersions returns the full base and head blobs for one file in a PR. basePath is
+// the file's path at the base sha: a rename's previous_path from the REST file list, else
+// path. base and head are the caller's pinned shas (from get_pr), which skip the prRefs
+// round-trip and pin the exact session blobs; empty resolves them.
+func (c *Client) GetFileVersions(ctx context.Context, owner, repo string, number int, path, basePath, base, head string) (*FileVersions, error) {
+	if basePath == "" {
+		basePath = path
+	}
 	if base == "" || head == "" {
 		rbase, rhead, err := c.prRefs(ctx, owner, repo, number)
 		if err != nil {
@@ -31,7 +32,7 @@ func (c *Client) GetFileVersions(ctx context.Context, owner, repo string, number
 	}
 	baseCh := make(chan blobResult, 1)
 	go func() {
-		b, err := c.rawBlob(ctx, owner, repo, path, base)
+		b, err := c.rawBlob(ctx, owner, repo, basePath, base)
 		baseCh <- blobResult{b, err}
 	}()
 	headBlob, headErr := c.rawBlob(ctx, owner, repo, path, head)

internal/github/read_test.go

@@ -28,7 +28,7 @@ func TestGetFileVersions(t *testing.T) {
 		t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path)
 		return nil, nil
 	})
-	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "dir/a.go", "", "")
+	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "dir/a.go", "", "", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -71,7 +71,7 @@ func TestGetFileVersionsPinnedRefsSkipPrRefs(t *testing.T) {
 		t.Fatalf("unexpected request %s %s", r.Method, r.URL.Path)
 		return nil, nil
 	})
-	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "pinbase", "pinhead")
+	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "", "pinbase", "pinhead")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -94,7 +94,7 @@ func TestGetFileVersionsAddedFileMissingBase(t *testing.T) {
 		t.Fatalf("unexpected request %s", r.URL.Path)
 		return nil, nil
 	})
-	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "new.go", "", "")
+	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "new.go", "", "", "")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -106,14 +106,49 @@ func TestGetFileVersionsAddedFileMissingBase(t *testing.T) {
 	}
 }
 
+// a rename's two sides live at different paths: the base blob is fetched at basePath
+// (the REST file list's previous_path), the head blob at path. asking for either path
+// at the other side's ref is the bug this guards.
+func TestGetFileVersionsRenameFetchesBothPaths(t *testing.T) {
+	c := newClient(func(r *http.Request) (*http.Response, error) {
+		ref := r.URL.Query().Get("ref")
+		switch {
+		case strings.HasSuffix(r.URL.Path, "/pulls/3"):
+			return resp(200, prRefsREST, nil), nil
+		case strings.Contains(r.URL.Path, "/contents/old/a.go"):
+			if ref != "basesha" {
+				t.Errorf("old path should only be asked for at the base, got ref %q", ref)
+			}
+			return resp(200, "old content", nil), nil
+		case strings.Contains(r.URL.Path, "/contents/new/b.go"):
+			if ref != "headsha" {
+				t.Errorf("new path should only be asked for at the head, got ref %q", ref)
+			}
+			return resp(200, "new content", nil), nil
+		}
+		t.Fatalf("unexpected request %s", r.URL.Path)
+		return nil, nil
+	})
+	fv, err := c.GetFileVersions(context.Background(), "o", "r", 3, "new/b.go", "old/a.go", "", "")
+	if err != nil {
+		t.Fatal(err)
+	}
+	if fv.Base.Missing || fv.Base.Content != "old content" {
+		t.Errorf("base should come from the previous path, not be missing: %+v", fv.Base)
+	}
+	if fv.Head.Missing || fv.Head.Content != "new content" {
+		t.Errorf("head should come from the new path: %+v", fv.Head)
+	}
+}
+
 func TestGetFileVersionsPropagatesError(t *testing.T) {
 	c := newClient(func(r *http.Request) (*http.Response, error) {
 		if strings.HasSuffix(r.URL.Path, "/pulls/3") {
 			return resp(200, prRefsREST, nil), nil
 		}
 		return resp(401, `{"message":"Bad credentials"}`, nil), nil
 	})
-	_, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "", "")
+	_, err := c.GetFileVersions(context.Background(), "o", "r", 3, "a.go", "", "", "")
 	if codeOf(t, err) != protocol.CodeAuth {
 		t.Fatalf("want auth, got %v", err)
 	}

internal/handlers/handlers.go

@@ -25,7 +25,7 @@ type Registry map[string]Handler
 type API interface {
 	ListPRs(ctx context.Context, owner, repo, filter string) ([]github.PR, error)
 	GetPR(ctx context.Context, owner, repo string, number int) (*github.PRDetail, error)
-	GetFileVersions(ctx context.Context, owner, repo string, number int, path, base, head string) (*github.FileVersions, error)
+	GetFileVersions(ctx context.Context, owner, repo string, number int, path, basePath, base, head string) (*github.FileVersions, error)
 	HeadSHA(ctx context.Context, owner, repo string, number int) (string, error)
 	GetThreads(ctx context.Context, owner, repo string, number int) ([]github.Thread, error)
 	GetTimeline(ctx context.Context, owner, repo string, number int) (*github.Timeline, error)

internal/handlers/handlers_test.go

@@ -22,6 +22,7 @@ type mockAPI struct {
 	gotFilter    string
 	gotNumber    int
 	gotPath      string
+	gotBasePath  string
 	gotBase      string
 	gotHead      string
 	gotReviewID  string
@@ -54,10 +55,11 @@ func (m *mockAPI) GetPR(_ context.Context, _, _ string, number int) (*github.PRD
 	return m.detail, nil
 }
 
-func (m *mockAPI) GetFileVersions(_ context.Context, _, _ string, number int, path, base, head string) (*github.FileVersions, error) {
+func (m *mockAPI) GetFileVersions(_ context.Context, _, _ string, number int, path, basePath, base, head string) (*github.FileVersions, error) {
 	m.called = true
 	m.gotNumber = number
 	m.gotPath = path
+	m.gotBasePath = basePath
 	m.gotBase = base
 	m.gotHead = head
 	return &github.FileVersions{}, nil
@@ -236,13 +238,16 @@ func TestMalformedParams(t *testing.T) {
 
 func TestGetFileVersionsRoutes(t *testing.T) {
 	m := &mockAPI{}
-	_, err := deps(m).getFileVersions(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":7,"path":"a.go","base":"B","head":"H"}`))
+	_, err := deps(m).getFileVersions(context.Background(), json.RawMessage(`{"owner":"o","repo":"r","number":7,"path":"a.go","previous_path":"old.go","base":"B","head":"H"}`))
 	if err != nil {
 		t.Fatal(err)
 	}
 	if m.gotNumber != 7 || m.gotPath != "a.go" {
 		t.Errorf("params not forwarded: number=%d path=%q", m.gotNumber, m.gotPath)
 	}
+	if m.gotBasePath != "old.go" {
+		t.Errorf("rename base path not forwarded: %q", m.gotBasePath)
+	}
 	if m.gotBase != "B" || m.gotHead != "H" {
 		t.Errorf("pinned refs not forwarded: base=%q head=%q", m.gotBase, m.gotHead)
 	}

internal/handlers/read_files.go

@@ -10,6 +10,8 @@ import (
 type getFileVersionsParams struct {
 	prParams
 	Path string `json:"path"`
+	// the file's path at the base sha: a rename's previous_path, else absent
+	PreviousPath string `json:"previous_path"`
 	// optional pinned refs: when the client already holds the PR's base/head shas
 	// (from get_pr), it sends them so the sidecar skips the prRefs round-trip and
 	// fetches the exact blobs the session is reviewing. empty falls back to prRefs
@@ -29,5 +31,5 @@ func (d Deps) getFileVersions(ctx context.Context, params json.RawMessage) (any,
 	if p.Path == "" {
 		return nil, protocol.NewError(protocol.CodeBadRequest, "path is required")
 	}
-	return d.GH.GetFileVersions(ctx, p.Owner, p.Repo, p.Number, p.Path, p.Base, p.Head)
+	return d.GH.GetFileVersions(ctx, p.Owner, p.Repo, p.Number, p.Path, p.PreviousPath, p.Base, p.Head)
 }

lua/differ/pr/client.lua

@@ -30,20 +30,22 @@ function M.get_pr(pr, cb)
 end
 
 -- get_file_versions result: {base: {content, missing?}, head: {content, missing?},
--- truncated?}. the sidecar fetches `path` at both the base and head refs. pass the
--- pinned `refs` ({base, head} from get_pr) so the sidecar skips the prRefs round-trip
--- and fetches the exact session blobs; omit to let the sidecar resolve them
+-- truncated?}. the sidecar fetches `path` at the head ref and `previous_path` (a
+-- rename's old path, else `path`) at the base ref. pass the pinned `refs` ({base, head}
+-- from get_pr) so the sidecar skips the prRefs round-trip and fetches the exact session
+-- blobs; omit to let the sidecar resolve them
 ---@param pr { owner: string, repo: string, number: integer }
----@param path string
+---@param file { path: string, previous_path?: string } -- only the path(s) are read
 ---@param refs { base?: string, head?: string }|nil
 ---@param cb fun(err: table|nil, result: any)
-function M.get_file_versions(pr, path, refs, cb)
+function M.get_file_versions(pr, file, refs, cb)
     refs = refs or {}
     sidecar.request("get_file_versions", {
         owner = pr.owner,
         repo = pr.repo,
         number = pr.number,
-        path = path,
+        path = file.path,
+        previous_path = file.previous_path,
         base = refs.base,
         head = refs.head,
     }, cb)

lua/differ/pr/init.lua

@@ -174,7 +174,7 @@ local function prefetch_around(entry)
                 local path = nb.path
                 s.prefetching[path] = true
                 local refs = { base = s.pr_meta.base_sha, head = s.pr_meta.head_sha }
-                client.get_file_versions(s.pr, path, refs, function(err, vers)
+                client.get_file_versions(s.pr, nb, refs, function(err, vers)
                     if not guard.owns(s) then
                         return -- session torn down (or replaced) while it was in flight
                     end
@@ -258,12 +258,10 @@ local function show_file(entry, focus_line)
     if cached then
         return render(cached)
     end
-    -- pass entry.path, not previous_path: the sidecar fetches one path at both refs,
-    -- so a rename shows its head content as an add (true rename diffing is a later
-    -- sidecar concern; this keeps open-and-navigate correct for the common cases).
-    -- the pinned shas skip the sidecar's prRefs round-trip (latency discipline)
+    -- the pinned shas skip the sidecar's prRefs round-trip (latency discipline); the
+    -- entry rides along whole so a rename's base side is read from its previous_path
     local refs = { base = session.pr_meta.base_sha, head = session.pr_meta.head_sha }
-    client.get_file_versions(session.pr, entry.path, refs, function(err, vers)
+    client.get_file_versions(session.pr, entry, refs, function(err, vers)
         if not guard.owns(s) then
             return -- session torn down (or replaced) while the blob was in flight
         end