fix: stabilize diff panel rendering and hunk-safe truncation

apps/server/src/features/projects/diffs/snapshots/snapshot-service.ts

@@ -5,6 +5,7 @@
  */
 
 import { injectable, inject } from "tsyringe";
+import { truncateUnifiedDiff } from "@mcode/shared";
 import * as NodeFSPromises from "node:fs/promises";
 import * as NodePath from "node:path";
 import * as NodeCrypto from "node:crypto";
@@ -178,9 +179,7 @@ async function executeDiffBatches(
   return outputs;
 }
 
-function limitDiffLines(diff: string, maxLines: number | undefined): string {
-  return maxLines ? diff.split("\n").slice(0, maxLines).join("\n") : diff;
-}
+
 
 function collectDiffStats(
   outputs: readonly string[],
@@ -336,7 +335,7 @@ export class SnapshotService {
         refAfter,
         pathspecBatches,
       );
-      return limitDiffLines(outputs.join("\n"), maxLines);
+      return truncateUnifiedDiff(outputs.join("\n"), maxLines);
     } catch {
       return "";
     }

apps/server/src/features/projects/git/git-comparison-service.ts

@@ -1,5 +1,5 @@
 import { inject, injectable } from "tsyringe";
-import { logger } from "@mcode/shared";
+import { logger, truncateUnifiedDiff } from "@mcode/shared";
 import type {
   BranchComparison,
   GitCommit,
@@ -76,13 +76,13 @@ export class GitComparisonService {
       if (filePath) args.push("--", filePath);
       const { stdout } = await this.gitExecutor.exec(args, { timeout: 10_000 });
       const result = stdout.trim();
-      return truncate && maxLines ? result.split("\n").slice(0, maxLines).join("\n") : result;
+      return truncate ? truncateUnifiedDiff(result, maxLines) : result;
     };
     try {
       return await readDiff(`${sha}~1..${sha}`, true);
     } catch {
       try {
-        return await readDiff(`${EMPTY_TREE}..${sha}`, false);
+        return await readDiff(`${EMPTY_TREE}..${sha}`, true);
       } catch {
         return "";
       }
@@ -139,7 +139,7 @@ export class GitComparisonService {
     try {
       const { stdout } = await this.gitExecutor.exec(args, { timeout: 10_000 });
       const result = stdout.trim();
-      return maxLines ? result.split("\n").slice(0, maxLines).join("\n") : result;
+      return truncateUnifiedDiff(result, maxLines);
     } catch {
       return "";
     }
@@ -205,7 +205,7 @@ export class GitComparisonService {
     try {
       const { stdout } = await this.gitExecutor.exec(args, { timeout: 10_000 });
       const result = stdout.trim();
-      return maxLines ? result.split("\n").slice(0, maxLines).join("\n") : result;
+      return truncateUnifiedDiff(result, maxLines);
     } catch {
       return "";
     }

apps/web/src/components/diff/ReviewDiffView.tsx

@@ -1,6 +1,5 @@
 import { useCallback, useEffect, useMemo, useRef, useState } from "react";
 import {
-  parsePatchFiles,
   type DiffLineAnnotation,
   type FileContents,
   type FileDiffMetadata,
@@ -21,6 +20,7 @@ import {
 import type { ReviewFileChange } from "@mcode/contracts";
 import { getTransport } from "@/transport";
 import { loadFileDiff } from "@/lib/load-file-diff";
+import { FileDiffCache } from "@/lib/file-diff-cache";
 import { parseDiffLines, isMarkdownFile } from "@/lib/diff-parser";
 import { parseFirstHunkLine } from "@/lib/parse-first-hunk-line";
 import { useShikiTheme } from "@/hooks/useTheme";
@@ -298,18 +298,20 @@ export function ReviewDiffView({
     [expanded, editingAnnotation],
   );
 
+  // Reusing the parsed object keeps each CodeView item's render target
+  // identical across unrelated state changes; a fresh object under the same
+  // cacheKey makes pierre's virtualizer commit a diff its layout pass never
+  // prepared.
+  const diffCache = useRef(new FileDiffCache()).current;
   const fileDiffs = useMemo(() => {
     const out: Record<string, FileDiffMetadata> = {};
+    const scope = `${threadId}:${source}:${id}:${cacheVersion}`;
     for (const [path, patch] of Object.entries(patches)) {
-      const parsed = parsePatchFiles(patch).flatMap((p) => p.files);
-      if (parsed.length > 0) {
-        const fileDiff = parsed[0]!;
-        fileDiff.cacheKey = `${threadId}:${source}:${id}:${cacheVersion}:${path}:${patch.length}`;
-        out[path] = fileDiff;
-      }
+      const fileDiff = diffCache.get(scope, path, patch);
+      if (fileDiff) out[path] = fileDiff;
     }
     return out;
-  }, [patches, id, source, threadId, cacheVersion]);
+  }, [patches, id, source, threadId, cacheVersion, diffCache]);
 
   // Lazy-load the patch for every expanded file missing one.
   useEffect(() => {

apps/web/src/lib/__tests__/file-diff-cache.test.ts

@@ -0,0 +1,57 @@
+import { describe, it, expect } from "vitest";
+import { FileDiffCache } from "../file-diff-cache";
+
+const PATCH = `diff --git a/a.ts b/a.ts
+index 0000000..1111111 100644
+--- a/a.ts
++++ b/a.ts
+@@ -1,2 +1,2 @@
+ context
+-old
++new`;
+
+const PATCH_2 = `diff --git a/a.ts b/a.ts
+index 0000000..1111111 100644
+--- a/a.ts
++++ b/a.ts
+@@ -1,2 +1,2 @@
+ context
+-old
++newer`;
+
+describe("FileDiffCache", () => {
+  it("returns the same object while the patch is unchanged", () => {
+    const cache = new FileDiffCache();
+    const first = cache.get("s", "a.ts", PATCH)!;
+    expect(cache.get("s", "a.ts", PATCH)).toBe(first);
+  });
+
+  it("re-parses with a distinct cacheKey when the patch changes", () => {
+    const cache = new FileDiffCache();
+    const first = cache.get("s", "a.ts", PATCH)!;
+    const second = cache.get("s", "a.ts", PATCH_2)!;
+    expect(second).not.toBe(first);
+    expect(second.cacheKey).not.toBe(first.cacheKey);
+  });
+
+  it("re-parses with a distinct cacheKey when the scope changes", () => {
+    const cache = new FileDiffCache();
+    const first = cache.get("s1", "a.ts", PATCH)!;
+    const second = cache.get("s2", "a.ts", PATCH)!;
+    expect(second).not.toBe(first);
+    expect(second.cacheKey).not.toBe(first.cacheKey);
+  });
+
+  it("does not reuse entries across paths", () => {
+    const cache = new FileDiffCache();
+    const a = cache.get("s", "a.ts", PATCH)!;
+    const b = cache.get("s", "b.ts", PATCH)!;
+    expect(a).not.toBe(b);
+    expect(a.cacheKey).not.toBe(b.cacheKey);
+  });
+
+  it("returns undefined for a patch with no files", () => {
+    const cache = new FileDiffCache();
+    expect(cache.get("s", "a.ts", "not a diff")).toBeUndefined();
+  });
+});

apps/web/src/lib/file-diff-cache.ts

@@ -0,0 +1,26 @@
+import { parsePatchFiles, type FileDiffMetadata } from "@pierre/diffs";
+
+/**
+ * Parses per-file patches into pierre FileDiffMetadata with stable object
+ * identity. pierre's areDiffTargetsEqual treats two objects sharing a
+ * cacheKey as one render target: handing a CodeView item a re-parsed copy of
+ * the same patch under the same key lets its virtualizer commit a different
+ * object than the prepared layout, which throws inside render. Reuse the
+ * parsed object while the patch is unchanged, and give every fresh parse a
+ * unique key so object identity and render-target identity stay equivalent.
+ */
+export class FileDiffCache {
+  private readonly entries = new Map<string, { scope: string; patch: string; fileDiff: FileDiffMetadata }>();
+  private serial = 0;
+
+  /** Parse `patch` for `path`, reusing the previous result when nothing changed. */
+  get(scope: string, path: string, patch: string): FileDiffMetadata | undefined {
+    const cached = this.entries.get(path);
+    if (cached && cached.scope === scope && cached.patch === patch) return cached.fileDiff;
+    const fileDiff = parsePatchFiles(patch).flatMap((file) => file.files)[0];
+    if (!fileDiff) return undefined;
+    fileDiff.cacheKey = `${scope}:${path}:${++this.serial}`;
+    this.entries.set(path, { scope, patch, fileDiff });
+    return fileDiff;
+  }
+}

packages/shared/src/git/__tests__/truncate-patch.test.ts

@@ -0,0 +1,154 @@
+import { describe, expect, it } from "vitest";
+import { truncateUnifiedDiff } from "../truncate-patch.js";
+
+/** Assert every hunk header's declared counts match the body lines present. */
+function expectCompleteHunks(patch: string): void {
+  const lines = patch.split("\n");
+  for (let i = 0; i < lines.length; i++) {
+    const match = /^@@ -(\d+)(?:,(\d+))? \+(\d+)(?:,(\d+))? @@/.exec(lines[i]!);
+    if (!match) continue;
+    let old = Number(match[2] ?? 1);
+    let next = Number(match[4] ?? 1);
+    let j = i + 1;
+    while (old + next > 0) {
+      const prefix = lines[j]?.[0];
+      expect(prefix, `hunk at line ${i} ends early`).toMatch(/^[-+ ]$/);
+      if (prefix === " ") {
+        old--;
+        next--;
+      } else if (prefix === "-") old--;
+      else next--;
+      j++;
+      if (lines[j] === "\\ No newline at end of file") j++;
+    }
+  }
+}
+
+const PATCH = [
+  "diff --git a/a.txt b/a.txt",
+  "index 1111111..2222222 100644",
+  "--- a/a.txt",
+  "+++ b/a.txt",
+  "@@ -1,3 +1,4 @@",
+  " one",
+  "-two",
+  "+2",
+  "+2b",
+  " three",
+  "@@ -10,3 +11,4 @@",
+  " ten",
+  "-eleven",
+  "+11",
+  "+11b",
+  " twelve",
+  "diff --git a/b.txt b/b.txt",
+  "--- a/b.txt",
+  "+++ b/b.txt",
+  "@@ -1,3 +1,4 @@",
+  " a",
+  "-b",
+  "+c",
+  "+d",
+  " e",
+  "diff --git a/c.txt b/c.txt",
+  "--- a/c.txt",
+  "+++ b/c.txt",
+  "@@ -1,3 +1,4 @@",
+  " x",
+  "-y",
+  "+z",
+  "+w",
+  " tail",
+].join("\n") + "\n";
+
+describe("truncateUnifiedDiff", () => {
+  it("returns the input when maxLines is undefined or exceeds the length", () => {
+    expect(truncateUnifiedDiff(PATCH, undefined)).toBe(PATCH);
+    expect(truncateUnifiedDiff(PATCH, 0)).toBe(PATCH);
+    expect(truncateUnifiedDiff(PATCH, 10_000)).toBe(PATCH);
+  });
+
+  it("drops a hunk cut mid-body and keeps earlier complete hunks", () => {
+    // Cutting inside b.txt's only hunk drops its headers too.
+    const lines = PATCH.split("\n");
+    const hunkB = lines.indexOf("+++ b/b.txt") + 1;
+    const truncated = truncateUnifiedDiff(PATCH, hunkB + 3);
+    expect(truncated).not.toContain("b.txt");
+    expect(truncated).toContain("a/a.txt");
+    expectCompleteHunks(truncated);
+    expect(truncated.split("\n").length).toBeLessThanOrEqual(hunkB + 3);
+  });
+
+  it("keeps every line budget allows when cuts land on hunk boundaries", () => {
+    for (let maxLines = 1; maxLines < PATCH.split("\n").length; maxLines++) {
+      const truncated = truncateUnifiedDiff(PATCH, maxLines);
+      expect(truncated.split("\n").length).toBeLessThanOrEqual(maxLines);
+      expectCompleteHunks(truncated);
+    }
+  });
+
+  it("keeps the \\ No newline marker with its hunk", () => {
+    const noEof = [
+      "diff --git a/a.txt b/a.txt",
+      "--- a/a.txt",
+      "+++ b/a.txt",
+      "@@ -1,2 +1,2 @@",
+      " a",
+      "-b",
+      "+c",
+      "\\ No newline at end of file",
+      "diff --git a/d.txt b/d.txt",
+      "--- a/d.txt",
+      "+++ b/d.txt",
+      "@@ -1,1 +1,1 @@",
+      "-q",
+      "+r",
+    ].join("\n") + "\n";
+    // The marker is line index 7; cutting after it keeps a complete hunk.
+    const truncated = truncateUnifiedDiff(noEof, 9);
+    expect(truncated).toContain("\\ No newline at end of file");
+    expectCompleteHunks(truncated);
+    // Cutting before the marker drops the whole hunk and the trailing file.
+    const tighter = truncateUnifiedDiff(noEof, 7);
+    expect(tighter).not.toContain("a.txt");
+    expect(tighter).not.toContain("d.txt");
+  });
+
+  it("handles headers without counts and zero counts", () => {
+    const patch = [
+      "diff --git a/a.txt b/a.txt",
+      "--- a/a.txt",
+      "+++ b/a.txt",
+      "@@ -1 +1 @@",
+      "-old",
+      "+new",
+      "@@ -5,0 +5 @@",
+      "+appended",
+    ].join("\n") + "\n";
+    const truncated = truncateUnifiedDiff(patch, 7);
+    expect(truncated).toContain("@@ -1 +1 @@");
+    expect(truncated).not.toContain("@@ -5,0 +5 @@");
+    expectCompleteHunks(truncated);
+  });
+
+  it("counts bare empty lines as context (diff.suppressBlankEmpty)", () => {
+    const patch = [
+      "diff --git a/a.txt b/a.txt",
+      "--- a/a.txt",
+      "+++ b/a.txt",
+      "@@ -1,3 +1,3 @@",
+      " one",
+      "",
+      " three",
+      "diff --git a/b.txt b/b.txt",
+      "--- a/b.txt",
+      "+++ b/b.txt",
+      "@@ -1,1 +1,1 @@",
+      "-x",
+      "+y",
+    ].join("\n") + "\n";
+    const truncated = truncateUnifiedDiff(patch, 8);
+    expect(truncated).toContain("a/a.txt");
+    expect(truncated).not.toContain("b.txt");
+  });
+});