perf(web): key PR file contents by commit set, memoize loader

apps/server/src/pullRequest/GitHubPullRequestCli.test.ts

@@ -2560,12 +2560,43 @@ layer("GitHubPullRequestCli.layer", (it) => {
         newPath: "src/root.ts",
       });
 
-      expect(contents).toEqual({ oldContents: "", newContents: "root contents\n" });
+      expect(contents).toEqual({
+        oldContents: "",
+        newContents: "root contents\n",
+        // No parent to echo: a root commit's new file leaves `baseSha` absent.
+        headSha: "a1b2c3d",
+      });
       assert.strictEqual(mockedExecute.mock.calls.length, 2);
       expect(callAt(1).args.join(" ")).toContain("contents/src/root.ts?ref=a1b2c3d");
     }),
   );
 
+  it.effect("echoes the revisions the host actually read", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output("a1b2c3d\tb1c2d3e\n")));
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output("before\n")));
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output("after\n")));
+      const cli = yield* GitHubPullRequestCli.GitHubPullRequestCli;
+
+      const contents = yield* cli.getPullRequestDiffFileContents({
+        cwd: "/w",
+        repository: "acme/web",
+        host: "github.com",
+        number: 7,
+        changeType: "change",
+        oldPath: "src/a.ts",
+        newPath: "src/a.ts",
+      });
+
+      expect(contents).toEqual({
+        oldContents: "before\n",
+        newContents: "after\n",
+        baseSha: "a1b2c3d",
+        headSha: "b1c2d3e",
+      });
+    }),
+  );
+
   it.effect("reports unusable diff revisions as a structured error", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(Effect.succeed(output("not-a-sha\tstill-not-a-sha\n")));

apps/server/src/pullRequest/GitHubPullRequestCli.ts

@@ -1517,7 +1517,15 @@ export const make = Effect.gen(function* () {
           ],
           { concurrency: 2 },
         );
-        return { oldContents, newContents };
+        // Echo the revisions actually read so the caller can invalidate its memo when the
+        // comparison moved under it. A root commit's new file has no parent: `baseRef` is
+        // the empty jq field there, and the echo stays absent rather than naming nothing.
+        return {
+          oldContents,
+          newContents,
+          ...(baseRef.length > 0 ? { baseSha: baseRef } : {}),
+          headSha: headRef,
+        };
       });
 
   const readLegacyDetail = (

apps/server/src/pullRequest/GitLabPullRequestCli.test.ts

@@ -670,11 +670,54 @@ layer("GitLabPullRequestCli.layer", (it) => {
         newPath: "src/first.ts",
       });
 
-      expect(contents).toEqual({ oldContents: "", newContents: "first contents\n" });
+      expect(contents).toEqual({
+        oldContents: "",
+        newContents: "first contents\n",
+        // No parent to echo: a root commit's new file leaves `baseSha` absent.
+        headSha: "a1b2c3d",
+      });
       expect(argsOfCall(1)[1]).toContain("raw?ref=a1b2c3d");
     }),
   );
 
+  it.effect("echoes the revisions the host actually read", () =>
+    Effect.gen(function* () {
+      mockedExecute.mockReturnValueOnce(
+        Effect.succeed(
+          output(
+            // @effect-diagnostics-next-line preferSchemaOverJson:off
+            JSON.stringify({
+              diff_refs: {
+                base_sha: "a1b2c3d",
+                head_sha: "b1c2d3e",
+                start_sha: "a1b2c3d",
+              },
+            }),
+          ),
+        ),
+      );
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output("before\n")));
+      mockedExecute.mockReturnValueOnce(Effect.succeed(output("after\n")));
+      const cli = yield* GitLabPullRequestCli.GitLabPullRequestCli;
+
+      const contents = yield* cli.getMergeRequestDiffFileContents({
+        cwd: "/w",
+        repository: "acme/web",
+        number: 7,
+        changeType: "change",
+        oldPath: "src/a.ts",
+        newPath: "src/a.ts",
+      });
+
+      expect(contents).toEqual({
+        oldContents: "before\n",
+        newContents: "after\n",
+        baseSha: "a1b2c3d",
+        headSha: "b1c2d3e",
+      });
+    }),
+  );
+
   it.effect("reports an oversized diff file with its path and reason", () =>
     Effect.gen(function* () {
       mockedExecute.mockReturnValueOnce(

apps/server/src/pullRequest/GitLabPullRequestCli.ts

@@ -1295,7 +1295,14 @@ export const make = Effect.gen(function* () {
           ],
           { concurrency: 2 },
         );
-        return { oldContents, newContents };
+        // Echo the revisions actually read (see PullRequestDiffFileContentsResult). A root
+        // commit's new file has no parent (`baseSha === ""`), which stays absent.
+        return {
+          oldContents,
+          newContents,
+          ...(refs.baseSha.length > 0 ? { baseSha: refs.baseSha } : {}),
+          headSha: refs.headSha,
+        };
       }),
 
     getProjectMergeCapabilities: (input) =>

apps/server/src/pullRequest/PullRequestProvider.ts

@@ -258,6 +258,9 @@ export interface ProviderDiffSlice {
 export interface ProviderDiffFileContents {
   readonly oldContents: string;
   readonly newContents: string;
+  /** Echoed read revisions (see PullRequestDiffFileContentsResult); absent means unknown. */
+  readonly baseSha?: string | undefined;
+  readonly headSha?: string | undefined;
 }
 
 export interface ProviderFilesViewed {

apps/web/src/components/pullRequest/PullRequestCodeTab.tsx

@@ -33,7 +33,11 @@ import { useLocalStorage } from "~/hooks/useLocalStorage";
 import { useClientSettings, useUpdateClientSettings } from "~/hooks/useSettings";
 import { useTheme } from "~/hooks/useTheme";
 import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse";
-import { pullRequestFindingKey, type PullRequestFinding } from "./pullRequestDetail.logic";
+import {
+  pullRequestFileContentsRevisionKey,
+  pullRequestFindingKey,
+  type PullRequestFinding,
+} from "./pullRequestDetail.logic";
 import { canEditPullRequestComment } from "./pullRequestEditing.logic";
 import { orderDiffFiles } from "./pullRequestFileOrder.logic";
 import {
@@ -355,15 +359,37 @@ function PullRequestCodeTab({
     reportFailure: false,
   });
   const getDiffFileContents = useAtomCommand(pullRequestEnvironment.diffFileContents);
+  // Revision-scoped, not update-scoped: `updatedAt` moves on comments, labels, and reviews
+  // while the files stay identical, and carrying it into the cache key throws away every file
+  // Pierre holds plus the loader's own memo. The commit set only moves when the code does, so
+  // the loader — and its memo — survives metadata touches and is rebuilt on a push. A
+  // base-branch advance without a head commit moves `behindBy` instead, so it rides along in
+  // the key where the host counted it. The key is the coarse gate: the loader also busts
+  // its memo when a served read echoes new revisions (same-count base replacement, or a
+  // refreshed diff outrunning lagging activity), so files expanded after the move read the
+  // code the host actually served. Already-displayed expansions re-render only when the
+  // key rebuilds the loader. Empty (activity not yet loaded) keeps the previous
+  // conservative key rather than sharing one across revisions that cannot be told apart.
+  // Preservation is unit-covered in pullRequestDetail.logic.test.ts rather than by a
+  // component render here.
+  const fileContentsRevisionKey = useMemo(
+    () =>
+      pullRequestFileContentsRevisionKey({
+        commits: detail.commits,
+        commit,
+        behindBy: detail.behindBy,
+      }) ?? `updated:${detail.updatedAt}`,
+    [commit, detail.behindBy, detail.commits, detail.updatedAt],
+  );
   const loadDiffFiles = useMemo(
     () =>
       createPullRequestDiffFileContentsLoader(getDiffFileContents, {
         environmentId,
         reference,
         commit,
-        cacheKey: `pull-request:${referenceKey}:${detail.updatedAt}:${commit ?? "all"}`,
+        cacheKey: `pull-request:${referenceKey}:${fileContentsRevisionKey}`,
       }),
-    [commit, detail.updatedAt, environmentId, getDiffFileContents, reference, referenceKey],
+    [commit, environmentId, fileContentsRevisionKey, getDiffFileContents, reference, referenceKey],
   );
 
   // What is offered is the intersection of two different questions: what this host can do at

apps/web/src/components/pullRequest/pullRequestDetail.logic.test.ts

@@ -35,6 +35,7 @@ import {
   pullRequestActionMenuHasGroup,
   pullRequestActionNeedsHostRefresh,
   pullRequestCheckoutCommand,
+  pullRequestFileContentsRevisionKey,
   pullRequestFindingKey,
   pullRequestReviewOutcome,
   readableFailure,
@@ -162,6 +163,100 @@ describe("pull request activity refresh", () => {
     ).toBe(false);
   });
 });
+
+describe("pull request file-contents revision", () => {
+  const commits = [{ oid: "aaa" }, { oid: "bbb" }];
+
+  it("keys one commit's own comparison by its oid alone", () => {
+    expect(pullRequestFileContentsRevisionKey({ commits, commit: "bbb" })).toBe("commit:bbb");
+    // Independent of the whole-PR commit set: the commit itself is immutable.
+    expect(pullRequestFileContentsRevisionKey({ commits: [{ oid: "zzz" }], commit: "bbb" })).toBe(
+      "commit:bbb",
+    );
+  });
+
+  it("holds still while only metadata moves", () => {
+    // Same commit set as after an unrelated updatedAt bump: identical key, so the loader and
+    // Pierre keep every expanded file instead of re-reading them.
+    expect(pullRequestFileContentsRevisionKey({ commits, commit: null })).toBe(
+      pullRequestFileContentsRevisionKey({
+        commits: [{ oid: "bbb" }, { oid: "aaa" }],
+        commit: null,
+      }),
+    );
+  });
+
+  it("busts on a push and on a same-length force-push", () => {
+    const before = pullRequestFileContentsRevisionKey({ commits, commit: null });
+    expect(
+      pullRequestFileContentsRevisionKey({ commits: [...commits, { oid: "ccc" }], commit: null }),
+    ).not.toBe(before);
+    expect(
+      pullRequestFileContentsRevisionKey({
+        commits: [{ oid: "aaa" }, { oid: "ccc" }],
+        commit: null,
+      }),
+    ).not.toBe(before);
+  });
+
+  it("tells apart oid sets that only differ by a separator boundary", () => {
+    expect(
+      pullRequestFileContentsRevisionKey({ commits: [{ oid: "ab" }, { oid: "c" }], commit: null }),
+    ).not.toBe(
+      pullRequestFileContentsRevisionKey({ commits: [{ oid: "a" }, { oid: "bc" }], commit: null }),
+    );
+  });
+
+  it("reports unknown while the activity has not loaded", () => {
+    expect(pullRequestFileContentsRevisionKey({ commits: [], commit: null })).toBe(null);
+    expect(pullRequestFileContentsRevisionKey({ commits: [{ oid: "" }], commit: null })).toBe(null);
+  });
+
+  it("keeps the CodeTab cache key still across updatedAt bumps while the revision is known", () => {
+    // Mirrors PullRequestCodeTab's `revisionKey ?? updated:${updatedAt}`: the loader's memo
+    // survives comments/labels/reviews, and only the unknown-revision fallback tracks updatedAt.
+    const keyFor = (oids: ReadonlyArray<string>, updatedAt: string) =>
+      pullRequestFileContentsRevisionKey({
+        commits: oids.map((oid) => ({ oid })),
+        commit: null,
+      }) ?? `updated:${updatedAt}`;
+    expect(keyFor(["aaa", "bbb"], "2026-08-13T13:00:00Z")).toBe(
+      keyFor(["aaa", "bbb"], "2026-08-13T13:01:00Z"),
+    );
+    expect(keyFor([], "2026-08-13T13:00:00Z")).not.toBe(keyFor([], "2026-08-13T13:01:00Z"));
+  });
+
+  it("busts on a base-branch advance without a head commit, where the host counted it", () => {
+    const before = pullRequestFileContentsRevisionKey({ commits, commit: null, behindBy: 2 });
+    const after = pullRequestFileContentsRevisionKey({ commits, commit: null, behindBy: 5 });
+    expect(before).not.toBeNull();
+    expect(after).not.toBe(before);
+    expect(after).toBe(`${before!.split(":behind")[0]}:behind5`);
+  });
+
+  it("keeps the old key shape where behindBy is absent", () => {
+    const without = pullRequestFileContentsRevisionKey({ commits, commit: null });
+    expect(without).toMatch(/^commits:2:[0-9a-z]+$/);
+    expect(pullRequestFileContentsRevisionKey({ commits, commit: null, behindBy: undefined })).toBe(
+      without,
+    );
+    expect(pullRequestFileContentsRevisionKey({ commits, commit: null, behindBy: null })).toBe(
+      without,
+    );
+  });
+
+  it("keeps one commit's own comparison keyed by its oid alone, whatever the base did", () => {
+    expect(pullRequestFileContentsRevisionKey({ commits, commit: "bbb", behindBy: 5 })).toBe(
+      "commit:bbb",
+    );
+  });
+
+  it("stays unknown while the activity has not loaded, even where the base count is known", () => {
+    expect(
+      pullRequestFileContentsRevisionKey({ commits: [], commit: null, behindBy: 5 }),
+    ).toBeNull();
+  });
+});
 describe("review thread comment pages", () => {
   it("appends new comments once and keeps refreshed base comments", () => {
     expect(

apps/web/src/components/pullRequest/pullRequestDetail.logic.ts

@@ -33,6 +33,7 @@ import {
 import { inferReviewCommentFenceLanguage, type ReviewCommentContext } from "~/reviewCommentContext";
 import { reviewCommentContextId } from "~/lib/composerContextRecords";
 import { removeInlineContextReference } from "~/lib/composerContextReferences";
+import { fnv1a32 } from "~/lib/diffRendering";
 
 export const PULL_REQUEST_MERGE_METHOD_LABELS: Record<PullRequestMergeMethod, string> = {
   merge: "Merge",
@@ -147,6 +148,56 @@ export function shouldRefreshPullRequestActivity(
 ): boolean {
   return previous !== null && previous.key === next.key && previous.updatedAt !== next.updatedAt;
 }
+
+/**
+ * The revision a pull request's file contents belong to, for scoping the hunk-expansion cache.
+ * `updatedAt` moves on comments, labels, and reviews while the files stay identical, so keying
+ * the cache on it throws every expanded file away on each of those. The commit set only moves
+ * when the code does — a push, a force-push, a rebase — so it busts exactly when the contents
+ * may have changed. One commit's own comparison is immutable and keyed by its oid directly.
+ *
+ * Null where the revision cannot be known yet (activity not loaded): the caller falls back to
+ * `updatedAt` rather than sharing one key across revisions it cannot tell apart.
+ *
+ * A base-branch advance without a new head commit leaves the commit set alone but moves
+ * `behindBy`, so the count rides along as a `:behindN` suffix and busts the key where the
+ * host counted it. Absent (a host that cannot compare, or an older caller) keeps the
+ * commit-set key exactly as before. An explicit refresh re-reads the patch but keeps
+ * expanded files: threading its token into this key would also bust on every metadata
+ * touch, which is the thrash revision-scoping exists to avoid.
+ *
+ * This key is the coarse gate, not the whole correctness story: a base replacement at
+ * the same `behindBy` count moves neither arm of it, and the commits behind it ride the
+ * activity query while the diff rides its own, so a refreshed diff can outrun a lagging
+ * activity read. The loader closes both windows from the other side — the server echoes
+ * the revisions each file read actually served, and the first read that lands on new
+ * revisions busts the loader memo. Residual: entries settled before the move are served
+ * until the next file read observes it; with no new read, only the key above protects.
+ * Already-expanded files on screen likewise re-render only when the key rebuilds the
+ * loader — the bust covers subsequently expanded files.
+ */
+export function pullRequestFileContentsRevisionKey(input: {
+  readonly commits: ReadonlyArray<{ readonly oid: string }>;
+  readonly commit: string | null;
+  readonly behindBy?: number | null | undefined;
+}): string | null {
+  if (input.commit !== null) return `commit:${input.commit}`;
+  const oids = input.commits
+    .map((commit) => commit.oid)
+    .filter((oid) => oid.length > 0)
+    .sort();
+  if (oids.length === 0) return null;
+  // The whole set, not just the head: a force-push can keep the length while replacing every
+  // oid, and a faked committer date can hide a new head from a newest-by-date read. NUL joins
+  // so ["ab", "c"] and ["a", "bc"] hash differently. Two independent FNV-1a passes make a
+  // 64-bit fingerprint, since a collision here serves another revision's files as this one's.
+  const joined = oids.join("\u0000");
+  const low = fnv1a32(joined);
+  const high = fnv1a32(joined, 0x9e3779b9, 0x85ebca6b);
+  const base = `commits:${oids.length}:${low.toString(36)}${high.toString(36)}`;
+  return input.behindBy == null ? base : `${base}:behind${input.behindBy}`;
+}
+
 /** Appends fetched pages without replacing fresher comments already in the activity response. */
 export function mergePullRequestThreadComments<T extends { readonly id: string }>(
   base: ReadonlyArray<T>,