fix(ui): prevent highlight cache identity collisions

.changeset/fuzzy-diffs-refresh.md

@@ -0,0 +1,5 @@
+---
+"hunkdiff": patch
+---
+
+Prevent syntax-highlight cache collisions from displaying stale code after review reloads.

src/ui/diff/useHighlightedDiff.test.ts

@@ -14,6 +14,17 @@ function createLargeHighlightTestFile(id: string) {
   return createTestDiffFile({ after: `${lines}\n`, before: "", id });
 }
 
+/** Build equal-length patches whose only difference sits outside the former sampled regions. */
+function createAdversarialPatch(marker: string) {
+  return `${"x".repeat(96)}${marker}${"x".repeat(415)}`;
+}
+
+/** Reproduce the former sampled patch identity to prove the regression fixture collides there. */
+function sampledPatchFingerprintForTest(patch: string) {
+  const mid = Math.floor(patch.length / 2);
+  return `${patch.length}:${patch.slice(0, 64)}:${patch.slice(mid, mid + 64)}:${patch.slice(-64)}`;
+}
+
 /** Register a worker double that fails every request and reports how often a retry reaches it. */
 function registerFailingHighlightWorkerForTest() {
   const state = { calls: 0 };
@@ -34,6 +45,47 @@ function registerFailingHighlightWorkerForTest() {
 }
 
 describe("highlighted diff cache", () => {
+  test("does not reuse stale highlighted text for patches that collide under sampling", async () => {
+    const firstPatch = createAdversarialPatch("a");
+    const secondPatch = createAdversarialPatch("b");
+    const first = {
+      ...createTestDiffFile({
+        after: 'const marker = "one";\n',
+        before: 'const marker = "base";\n',
+        id: "adversarial-patch-cache",
+        path: "adversarial.ts",
+      }),
+      patch: firstPatch,
+    };
+    const second = {
+      ...createTestDiffFile({
+        after: 'const marker = "two";\n',
+        before: 'const marker = "base";\n',
+        id: "adversarial-patch-cache",
+        path: "adversarial.ts",
+      }),
+      patch: secondPatch,
+    };
+    const patchOnlyChange = { ...first, patch: secondPatch };
+    const theme = resolveTheme("github-dark-default", null);
+
+    expect(sampledPatchFingerprintForTest(firstPatch)).toBe(
+      sampledPatchFingerprintForTest(secondPatch),
+    );
+    // Keep metadata identical here so only the formerly unsampled patch content can change the key.
+    expect(highlightedDiffCacheKey(theme, first)).not.toBe(
+      highlightedDiffCacheKey(theme, patchOnlyChange),
+    );
+    expect(highlightedDiffCacheKey(theme, first)).not.toBe(highlightedDiffCacheKey(theme, second));
+
+    const firstHighlight = await prefetchHighlightedDiff({ file: first, theme });
+    const secondHighlight = await prefetchHighlightedDiff({ file: second, theme });
+    expect(secondHighlight).not.toBe(firstHighlight);
+    expect(JSON.stringify(firstHighlight.additionLines)).toContain("one");
+    expect(JSON.stringify(secondHighlight.additionLines)).toContain("two");
+    expect(JSON.stringify(secondHighlight.additionLines)).not.toContain("one");
+  });
+
   test("invalidates source-backed partial highlights when an unversioned provider changes", () => {
     const base = createTestDiffFile({ id: "cache", path: "cache.ts" });
     const firstFetcher = createTestSourceFetcher(() => "first source\n");

src/ui/diff/useHighlightedDiff.ts

@@ -1,3 +1,4 @@
+import { createHash } from "node:crypto";
 import { useLayoutEffect, useState } from "react";
 import type { DiffFile } from "../../core/changeset/model";
 import type { AppTheme } from "../themes";
@@ -7,58 +8,35 @@ import { syntaxHighlightThemeName } from "./syntaxHighlightTheme";
 
 const SHARED_HIGHLIGHTED_DIFF_CACHE = createHighlightedDiffCache();
 const SHARED_HIGHLIGHT_PROMISES = new Map<string, Promise<HighlightedDiffCode>>();
+const highlightedContentFingerprints = new WeakMap<
+  DiffFile,
+  { fingerprint: string; metadata: DiffFile["metadata"]; patch: string }
+>();
 const sourceFetcherIds = new WeakMap<NonNullable<DiffFile["sourceFetcher"]>, number>();
 let nextSourceFetcherId = 1;
 
-/** Summarize rendered diff lines without serializing whole arrays into the cache key. */
-function lineSetFingerprint(lines: string[] | undefined) {
-  let totalChars = 0;
-  let hash = 2166136261;
-
-  for (const line of lines ?? []) {
-    totalChars += line.length;
-
-    for (let index = 0; index < line.length; index += 1) {
-      hash ^= line.charCodeAt(index);
-      hash = Math.imul(hash, 16777619);
-    }
-
-    hash ^= 10;
-    hash = Math.imul(hash, 16777619);
+/** Hash every diff-content input that can change the rendered highlight result. */
+function highlightedContentFingerprint(file: DiffFile) {
+  const cached = highlightedContentFingerprints.get(file);
+  if (cached?.metadata === file.metadata && cached.patch === file.patch) {
+    return cached.fingerprint;
   }
 
-  return `${lines?.length ?? 0}:${totalChars}:${(hash >>> 0).toString(36)}`;
-}
-
-/** Build a fallback fingerprint from parsed metadata when raw patch text is unavailable. */
-function metadataFingerprint(file: DiffFile) {
-  const hunkSummary = file.metadata.hunks
-    .map(
-      (hunk) =>
-        `${hunk.hunkSpecs ?? ""}:${hunk.deletionStart}:${hunk.deletionCount}:${hunk.additionStart}:${hunk.additionCount}:${hunk.hunkContent.length}`,
-    )
-    .join("|");
-
-  return [
-    file.metadata.name,
-    file.metadata.prevName ?? "",
-    file.metadata.type,
-    lineSetFingerprint(file.metadata.deletionLines),
-    lineSetFingerprint(file.metadata.additionLines),
-    hunkSummary,
-  ].join(":");
-}
-
-/** Content fingerprint from the diff patch. Changes whenever the underlying diff
- *  changes, allowing per-file cache invalidation without a global flush. */
-function patchFingerprint(file: DiffFile) {
-  const { patch } = file;
-  if (patch.length === 0) {
-    return metadataFingerprint(file);
-  }
-
-  const mid = Math.floor(patch.length / 2);
-  return `${patch.length}:${patch.slice(0, 64)}:${patch.slice(mid, mid + 64)}:${patch.slice(-64)}`;
+  const metadata = JSON.stringify(file.metadata);
+  const fingerprint = createHash("sha256")
+    .update(`${file.patch.length}:`)
+    .update(file.patch)
+    .update(`${metadata.length}:`)
+    .update(metadata)
+    .digest("hex");
+  // Review reloads replace DiffFile snapshots rather than mutating them, so object identity safely
+  // avoids rehashing large patches during every render and viewport-prefetch pass.
+  highlightedContentFingerprints.set(file, {
+    fingerprint,
+    metadata: file.metadata,
+    patch: file.patch,
+  });
+  return fingerprint;
 }
 
 /** Identify the source snapshot provider used to recover grammar state for a partial diff. */
@@ -81,9 +59,9 @@ function sourceFetcherFingerprint(file: DiffFile) {
   return `source:${id}`;
 }
 
-/** Cache key that includes patch and source-provider identity so reloads cannot reuse stale grammar state. */
+/** Cache key that includes every content and source-provider input to highlighted rendering. */
 export function highlightedDiffCacheKey(theme: AppTheme, file: DiffFile) {
-  return `${theme.id}:${syntaxHighlightThemeName(theme)}:${file.id}:${patchFingerprint(file)}:${sourceFetcherFingerprint(file)}`;
+  return `${theme.id}:${syntaxHighlightThemeName(theme)}:${file.id}:${file.language ?? "text"}:${highlightedContentFingerprint(file)}:${sourceFetcherFingerprint(file)}`;
 }
 
 /**