fix: normalize rename-only diff paths (#194)

Author: benvinegar

src/core/diffPaths.ts

@@ -0,0 +1,22 @@
+import type { FileDiffMetadata } from "@pierre/diffs";
+
+/** Remove parser-added CR/LF suffixes from diff paths without touching meaningful spaces. */
+export function normalizeDiffPath(path: string | undefined) {
+  return path?.replace(/[\r\n]+$/u, "");
+}
+
+/** Sanitize parsed diff metadata path fields before the UI or loaders consume them. */
+export function normalizeDiffMetadataPaths(metadata: FileDiffMetadata): FileDiffMetadata {
+  const name = normalizeDiffPath(metadata.name) ?? metadata.name;
+  const prevName = normalizeDiffPath(metadata.prevName);
+
+  if (name === metadata.name && prevName === metadata.prevName) {
+    return metadata;
+  }
+
+  return {
+    ...metadata,
+    name,
+    prevName,
+  };
+}

src/core/loaders.test.ts

@@ -720,6 +720,30 @@ describe("loadAppBootstrap", () => {
     ).rejects.toThrow("`hunk stash show stash@{99}` could not resolve stash entry `stash@{99}`.");
   });
 
+  test("strips parser-added line endings from rename-only paths", async () => {
+    const bootstrap = await loadAppBootstrap({
+      kind: "patch",
+      text: [
+        "diff --git a/pi/extensions/loop.ts b/agents/pi/extensions/notify.ts",
+        "similarity index 100%",
+        "rename from pi/extensions/loop.ts",
+        "rename to agents/pi/extensions/notify.ts",
+      ].join("\n"),
+      options: { mode: "auto" },
+    });
+
+    expect(bootstrap.changeset.files).toHaveLength(1);
+    expect(bootstrap.changeset.files[0]).toMatchObject({
+      path: "agents/pi/extensions/notify.ts",
+      previousPath: "pi/extensions/loop.ts",
+      metadata: {
+        name: "agents/pi/extensions/notify.ts",
+        prevName: "pi/extensions/loop.ts",
+        type: "rename-pure",
+      },
+    });
+  });
+
   test("treats malformed inline patch text as an empty review instead of throwing", async () => {
     const bootstrap = await loadAppBootstrap({
       kind: "patch",

src/core/loaders.ts

@@ -9,6 +9,7 @@ import { createTwoFilesPatch } from "diff";
 import { resolve as resolvePath } from "node:path";
 import { findAgentFileContext, loadAgentContext } from "./agent";
 import { createSkippedBinaryMetadata, isProbablyBinaryFile, patchLooksBinary } from "./binary";
+import { normalizeDiffMetadataPaths, normalizeDiffPath } from "./diffPaths";
 import {
   buildGitDiffArgs,
   buildGitShowArgs,
@@ -123,6 +124,7 @@ function findPatchChunk(metadata: FileDiffMetadata, chunks: string[], index: num
   return (
     chunks.find((chunk) =>
       [metadata.name, metadata.prevName]
+        .map(normalizeDiffPath)
         .filter((value): value is string => Boolean(value))
         .map(stripPrefixes)
         .some(
@@ -148,15 +150,19 @@ function buildDiffFile(
   agentContext: AgentContext | null,
   { isUntracked, previousPath, isBinary }: BuildDiffFileOptions = {},
 ): DiffFile {
+  const normalizedMetadata = normalizeDiffMetadataPaths(metadata);
+  const path = normalizedMetadata.name;
+  const resolvedPreviousPath = normalizeDiffPath(previousPath) ?? normalizedMetadata.prevName;
+
   return {
-    id: `${sourcePrefix}:${index}:${metadata.name}`,
-    path: metadata.name,
-    previousPath: previousPath ?? metadata.prevName,
+    id: `${sourcePrefix}:${index}:${path}`,
+    path,
+    previousPath: resolvedPreviousPath,
     patch,
-    language: getFiletypeFromFileName(metadata.name) ?? undefined,
-    stats: countDiffStats(metadata),
-    metadata,
-    agent: findAgentFileContext(agentContext, metadata.name, metadata.prevName),
+    language: getFiletypeFromFileName(path) ?? undefined,
+    stats: countDiffStats(normalizedMetadata),
+    metadata: normalizedMetadata,
+    agent: findAgentFileContext(agentContext, path, resolvedPreviousPath),
     isUntracked,
     isBinary: isBinary ?? patchLooksBinary(patch),
   };

src/opentui/HunkDiffView.tsx

@@ -1,5 +1,6 @@
 import { useMemo } from "react";
 import { patchLooksBinary } from "../core/binary";
+import { normalizeDiffMetadataPaths, normalizeDiffPath } from "../core/diffPaths";
 import type { DiffFile } from "../core/types";
 import { findMaxLineNumber } from "../ui/diff/codeColumns";
 import { buildSplitRows, buildStackRows } from "../ui/diff/pierre";
@@ -28,17 +29,19 @@ function countDiffStats(metadata: HunkDiffFile["metadata"]) {
 /** Adapt the public diff shape into Hunk's internal file model without exposing app-only fields. */
 function toInternalDiffFile(diff: HunkDiffFile): DiffFile {
   const patch = diff.patch ?? "";
+  const metadata = normalizeDiffMetadataPaths(diff.metadata);
+  const path = normalizeDiffPath(diff.path) ?? metadata.name;
 
   return {
     agent: null,
     id: diff.id,
     isBinary: patchLooksBinary(patch),
     language: diff.language,
-    metadata: diff.metadata,
+    metadata,
     patch,
-    path: diff.path ?? diff.metadata.name,
-    previousPath: diff.metadata.prevName,
-    stats: countDiffStats(diff.metadata),
+    path,
+    previousPath: metadata.prevName,
+    stats: countDiffStats(metadata),
   };
 }
 

src/ui/lib/files.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, test } from "bun:test";
 import { createTestDiffFile, lines } from "../../../test/helpers/diff-helpers";
-import { buildSidebarEntries } from "./files";
+import { buildSidebarEntries, fileLabelParts } from "./files";
 
 describe("files helpers", () => {
   test("buildSidebarEntries hides zero-value sidebar stats", () => {
@@ -60,4 +60,22 @@ describe("files helpers", () => {
       deletionsText: null,
     });
   });
+
+  test("fileLabelParts strips parser-added line endings from rename labels", () => {
+    const renamedAcrossDirectories = {
+      ...createTestDiffFile({
+        id: "rename-across-dirs",
+        path: "agents/pi/extensions/notify.ts",
+        previousPath: "pi/extensions/loop.ts\n",
+        before: lines("export const stable = true;"),
+        after: lines("export const stable = true;"),
+      }),
+      stats: { additions: 0, deletions: 0 },
+    };
+
+    expect(fileLabelParts(renamedAcrossDirectories)).toEqual({
+      filename: "pi/extensions/loop.ts -> agents/pi/extensions/notify.ts",
+      stateLabel: null,
+    });
+  });
 });

src/ui/lib/files.ts

@@ -1,5 +1,6 @@
 import { basename, dirname } from "node:path/posix";
 import type { FileDiffMetadata } from "@pierre/diffs";
+import { normalizeDiffPath } from "../../core/diffPaths";
 import type { AgentAnnotation, DiffFile } from "../../core/types";
 
 export interface FileListEntry {
@@ -22,12 +23,15 @@ export type SidebarEntry = FileListEntry | FileGroupEntry;
 
 /** Build the filename-first label shown inside one sidebar row. */
 function sidebarFileName(file: DiffFile) {
-  if (!file.previousPath || file.previousPath === file.path) {
-    return basename(file.path);
+  const path = normalizeDiffPath(file.path) ?? file.path;
+  const previousPath = normalizeDiffPath(file.previousPath);
+
+  if (!previousPath || previousPath === path) {
+    return basename(path);
   }
 
-  const previousName = basename(file.previousPath);
-  const nextName = basename(file.path);
+  const previousName = basename(previousPath);
+  const nextName = basename(path);
   return previousName === nextName ? nextName : `${previousName} -> ${nextName}`;
 }
 
@@ -91,7 +95,11 @@ export function filterReviewFiles(files: DiffFile[], query: string): DiffFile[]
   }
 
   return files.filter((file) => {
-    const haystack = [file.path, file.previousPath, file.agent?.summary]
+    const haystack = [
+      normalizeDiffPath(file.path),
+      normalizeDiffPath(file.previousPath),
+      file.agent?.summary,
+    ]
       .filter(Boolean)
       .join(" ")
       .toLowerCase();
@@ -105,7 +113,8 @@ export function buildSidebarEntries(files: DiffFile[]): SidebarEntry[] {
   let activeGroup: string | null = null;
 
   files.forEach((file, index) => {
-    const group = dirname(file.path);
+    const path = normalizeDiffPath(file.path) ?? file.path;
+    const group = dirname(path);
     const nextGroup = group === "." ? null : group;
 
     if (nextGroup !== activeGroup) {
@@ -148,10 +157,9 @@ export function fileLabelParts(file: DiffFile | undefined): {
     return { filename: "No file selected", stateLabel: null };
   }
 
-  const baseLabel =
-    file.previousPath && file.previousPath !== file.path
-      ? `${file.previousPath} -> ${file.path}`
-      : file.path;
+  const path = normalizeDiffPath(file.path) ?? file.path;
+  const previousPath = normalizeDiffPath(file.previousPath);
+  const baseLabel = previousPath && previousPath !== path ? `${previousPath} -> ${path}` : path;
 
   // Determine state label for special cases
   let stateLabel: string | null = null;