fix(web): keep loading diffs collapsed after collapse all

apps/web/src/components/DiffPanel.tsx

@@ -43,7 +43,13 @@ import {
   resolveFileDiffPath,
 } from "../lib/diffRendering";
 import { PREFERRED_HIGHLIGHTER } from "../lib/syntaxHighlighting";
-import { areAllDiffFilesCollapsed, toggleAllDiffFiles } from "../lib/diffCollapse";
+import {
+  areAllDiffFilesCollapsed,
+  isFileDiffCollapsed,
+  setDiffFileCollapsed,
+  toggleAllDiffFiles,
+  type DiffFilesCollapseState,
+} from "../lib/diffCollapse";
 import { useTurnDiffSummaries } from "../hooks/useTurnDiffSummaries";
 import { useWorkspaceMutationRefresh } from "../hooks/useWorkspaceMutationRefresh";
 import { useProject, useThreadProjection, useThreadShell } from "../state/entities";
@@ -112,9 +118,8 @@ function getCachedFileEntry(fileDiff: FileDiffMetadata) {
   return entry;
 }
 
-interface CollapsedDiffFilesState {
+interface CollapsedDiffFilesState extends DiffFilesCollapseState {
   readonly scopeKey: string | null;
-  readonly fileKeys: ReadonlySet<string>;
 }
 
 const EMPTY_COLLAPSED_DIFF_FILE_KEYS: ReadonlySet<string> = new Set();
@@ -212,7 +217,8 @@ export default function DiffPanel({
   const [baseRefQuery, setBaseRefQuery] = useState("");
   const [collapsedDiffFiles, setCollapsedDiffFiles] = useState<CollapsedDiffFilesState>(() => ({
     scopeKey: null,
-    fileKeys: EMPTY_COLLAPSED_DIFF_FILE_KEYS,
+    foldOverride: null,
+    toggledFileKeys: EMPTY_COLLAPSED_DIFF_FILE_KEYS,
   }));
   const [codeViewRevision, setCodeViewRevision] = useState(0);
   const [codeView, setCodeView] = useState<AnnotatableCodeViewHandle | null>(null);
@@ -508,20 +514,41 @@ export default function DiffPanel({
 
   const isRefreshingDiff = branchDiffPreview.isPending || areFilePatchesPending;
   const renderableFileEntries = useMemo(
-    () => renderableFiles.map(getCachedFileEntry),
-    [renderableFiles],
-  );
-  const defaultCollapsedDiffFileKeys = useMemo(
     () =>
-      settings.diffFilesCollapsed
-        ? new Set(renderableFileEntries.map((file) => file.fileKey))
-        : EMPTY_COLLAPSED_DIFF_FILE_KEYS,
-    [renderableFileEntries, settings.diffFilesCollapsed],
+      renderableFiles.map((fileDiff) => {
+        const entry = getCachedFileEntry(fileDiff);
+        return lazySource
+          ? { ...entry, fileKey: buildFileDiffIdentityKey(fileDiff, "file") }
+          : entry;
+      }),
+    [renderableFiles, lazySource],
+  );
+  const defaultCollapsedDiffFiles = useMemo<DiffFilesCollapseState>(
+    () => ({
+      foldOverride: settings.diffFilesCollapsed ? "folded" : "expanded",
+      toggledFileKeys: EMPTY_COLLAPSED_DIFF_FILE_KEYS,
+    }),
+    [settings.diffFilesCollapsed],
   );
-  const collapsedDiffFileKeys =
+  const effectiveCollapsedDiffFiles =
     collapsedDiffFiles.scopeKey === collapseScopeKey
-      ? collapsedDiffFiles.fileKeys
-      : defaultCollapsedDiffFileKeys;
+      ? collapsedDiffFiles
+      : defaultCollapsedDiffFiles;
+  const collapsedDiffFileKeys = useMemo(
+    () =>
+      new Set(
+        renderableFileEntries
+          .filter(({ fileKey }) =>
+            isFileDiffCollapsed(
+              fileKey,
+              effectiveCollapsedDiffFiles.foldOverride,
+              effectiveCollapsedDiffFiles.toggledFileKeys,
+            ),
+          )
+          .map(({ fileKey }) => fileKey),
+      ),
+    [renderableFileEntries, effectiveCollapsedDiffFiles],
+  );
   const renderLoadingBoundary = useCallback(
     () =>
       settledFileCount < renderableFiles.length ? (
@@ -594,11 +621,12 @@ export default function DiffPanel({
       const file = renderableFileEntries[index];
       if (!file) return;
       setCollapsedDiffFiles((current) => {
-        const next = new Set(
-          current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys,
+        const next = setDiffFileCollapsed(
+          file.fileKey,
+          current.scopeKey === collapseScopeKey ? current : defaultCollapsedDiffFiles,
+          false,
         );
-        next.delete(file.fileKey);
-        return { scopeKey: collapseScopeKey, fileKeys: next };
+        return { ...next, scopeKey: collapseScopeKey };
       });
       if (lazySource && index >= settledFileCount) {
         requestFile(index);
@@ -608,7 +636,7 @@ export default function DiffPanel({
     [
       renderableFileEntries,
       collapseScopeKey,
-      defaultCollapsedDiffFileKeys,
+      defaultCollapsedDiffFiles,
       requestTreeReveal,
       lazySource,
       settledFileCount,
@@ -657,49 +685,44 @@ export default function DiffPanel({
     },
     [activeCwd, activeRepositoryRoot, openInPreferredEditor, routeThreadRef],
   );
-  const collapseDefaultsRef = useRef({ collapseScopeKey, defaultCollapsedDiffFileKeys });
+  const collapseDefaultsRef = useRef({ collapseScopeKey, defaultCollapsedDiffFiles });
   useLayoutEffect(() => {
-    collapseDefaultsRef.current = { collapseScopeKey, defaultCollapsedDiffFileKeys };
-  }, [collapseScopeKey, defaultCollapsedDiffFileKeys]);
+    collapseDefaultsRef.current = { collapseScopeKey, defaultCollapsedDiffFiles };
+  }, [collapseScopeKey, defaultCollapsedDiffFiles]);
   const toggleDiffFileCollapsed = useCallback((fileKey: string) => {
-    const { collapseScopeKey, defaultCollapsedDiffFileKeys } = collapseDefaultsRef.current;
+    const { collapseScopeKey, defaultCollapsedDiffFiles } = collapseDefaultsRef.current;
     setCollapsedDiffFiles((current) => {
-      const next = new Set(
-        current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys,
+      const state = current.scopeKey === collapseScopeKey ? current : defaultCollapsedDiffFiles;
+      const next = setDiffFileCollapsed(
+        fileKey,
+        state,
+        !isFileDiffCollapsed(fileKey, state.foldOverride, state.toggledFileKeys),
       );
-      if (next.has(fileKey)) {
-        next.delete(fileKey);
-      } else {
-        next.add(fileKey);
-      }
-      return { scopeKey: collapseScopeKey, fileKeys: next };
+      return { ...next, scopeKey: collapseScopeKey };
     });
   }, []);
   // Find can ask again before the unfolded file reaches the viewer, so this must never fold.
   const unfoldDiffFile = useCallback((fileKey: string) => {
-    const { collapseScopeKey, defaultCollapsedDiffFileKeys } = collapseDefaultsRef.current;
+    const { collapseScopeKey, defaultCollapsedDiffFiles } = collapseDefaultsRef.current;
     setCollapsedDiffFiles((current) => {
-      const fileKeys =
-        current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys;
-      if (!fileKeys.has(fileKey)) return current;
-      const next = new Set(fileKeys);
-      next.delete(fileKey);
-      return { scopeKey: collapseScopeKey, fileKeys: next };
+      const state = current.scopeKey === collapseScopeKey ? current : defaultCollapsedDiffFiles;
+      const next = setDiffFileCollapsed(fileKey, state, false);
+      if (next === state) return current;
+      return { ...next, scopeKey: collapseScopeKey };
     });
   }, []);
 
   const toggleDiffFileCollapse = useCallback(() => {
     setCodeViewRevision((current) => current + 1);
     setCollapsedDiffFiles((current) => {
-      const currentKeys =
-        current.scopeKey === collapseScopeKey ? current.fileKeys : defaultCollapsedDiffFileKeys;
+      const state = current.scopeKey === collapseScopeKey ? current : defaultCollapsedDiffFiles;
 
       return {
+        ...toggleAllDiffFiles(diffFileKeys, state),
         scopeKey: collapseScopeKey,
-        fileKeys: toggleAllDiffFiles(diffFileKeys, currentKeys),
       };
     });
-  }, [collapseScopeKey, defaultCollapsedDiffFileKeys, diffFileKeys]);
+  }, [collapseScopeKey, defaultCollapsedDiffFiles, diffFileKeys]);
 
   const selectTurn = (runId: RunId) => {
     if (!routeThreadRef) return;

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

@@ -31,7 +31,11 @@ import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } fro
 import { useLocalStorage } from "~/hooks/useLocalStorage";
 import { useClientSettings, useUpdateClientSettings } from "~/hooks/useSettings";
 import { useTheme } from "~/hooks/useTheme";
-import { areAllDiffFilesCollapsed } from "~/lib/diffCollapse";
+import {
+  areAllDiffFilesCollapsed,
+  isFileDiffCollapsed,
+  type DiffFoldOverride,
+} from "~/lib/diffCollapse";
 import { pullRequestFindingKey, type PullRequestFinding } from "./pullRequestDetail.logic";
 import { canEditPullRequestComment, canResolvePullRequestThread } from "./pullRequestEditing.logic";
 import { orderDiffFiles } from "./pullRequestFileOrder.logic";
@@ -87,12 +91,7 @@ import {
   ReviewThreadCard,
   toViewerSide,
 } from "./PullRequestReviewAnnotation";
-import {
-  isFileDiffCollapsed,
-  isLineInFileDiff,
-  toggleFileDiffFoldForViewed,
-  type DiffFoldOverride,
-} from "./pullRequestDiff.logic";
+import { isLineInFileDiff, toggleFileDiffFoldForViewed } from "./pullRequestDiff.logic";
 import { PullRequestDiffStat, PullRequestMetaLine } from "./pullRequestPresentation";
 import { usePullRequestFilesViewed } from "./usePullRequestFilesViewed";
 import {

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

@@ -1,11 +1,8 @@
 import type { FileDiffMetadata } from "@pierre/diffs";
 import { describe, expect, it } from "vite-plus/test";
+import { isFileDiffCollapsed } from "~/lib/diffCollapse";
 
-import {
-  isFileDiffCollapsed,
-  isLineInFileDiff,
-  toggleFileDiffFoldForViewed,
-} from "./pullRequestDiff.logic";
+import { isLineInFileDiff, toggleFileDiffFoldForViewed } from "./pullRequestDiff.logic";
 
 /** Only the hunk ranges matter here; the viewer fills the rest in when it renders. */
 function fileWithHunks(

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

@@ -1,5 +1,6 @@
 import type { FileDiffMetadata } from "@pierre/diffs";
 import type { PullRequestDiffSide } from "@t3tools/contracts";
+import { isFileDiffCollapsed, type DiffFoldOverride } from "~/lib/diffCollapse";
 
 /**
  * Whether a conversation's line is really in this file's hunks.
@@ -21,27 +22,6 @@ export function isLineInFileDiff(
   );
 }
 
-/** What the toolbar last asked of every file at once, null being the reader asking nothing yet. */
-export type DiffFoldOverride = "expanded" | "folded" | null;
-
-/**
- * Whether a file is drawn folded.
- *
- * A diff arrives a slice at a time, so the reader's own choices are kept as the difference from
- * what the toolbar last said rather than as the set of folded files: a file that has not loaded
- * yet cannot be in a set, and would otherwise land expanded moments after the reader folded
- * everything. The caller supplies the saved default until the toolbar overrides it; individual
- * files can still be toggled independently.
- */
-export function isFileDiffCollapsed(
-  fileKey: string,
-  foldOverride: DiffFoldOverride,
-  toggledFileKeys: ReadonlySet<string>,
-): boolean {
-  const foldedByDefault = foldOverride === "folded";
-  return toggledFileKeys.has(fileKey) ? !foldedByDefault : foldedByDefault;
-}
-
 /**
  * The reader's fold choices after a file was ticked off, or put back.
  *

apps/web/src/lib/diffCollapse.test.ts

@@ -1,9 +1,24 @@
 import { describe, expect, it } from "vite-plus/test";
 
-import { areAllDiffFilesCollapsed, toggleAllDiffFiles } from "./diffCollapse";
+import {
+  areAllDiffFilesCollapsed,
+  isFileDiffCollapsed,
+  setDiffFileCollapsed,
+  toggleAllDiffFiles,
+  type DiffFilesCollapseState,
+} from "./diffCollapse";
+import { buildFileDiffIdentityKey, getRenderablePatch } from "./diffRendering";
 
 const FILE_KEYS = ["src/app.ts", "src/index.ts"];
 const FIRST_FILE_KEY = FILE_KEYS[0]!;
+const EXPANDED: DiffFilesCollapseState = { foldOverride: "expanded", toggledFileKeys: new Set() };
+const FOLDED: DiffFilesCollapseState = { foldOverride: "folded", toggledFileKeys: new Set() };
+
+function collapsedKeys(fileKeys: ReadonlyArray<string>, state: DiffFilesCollapseState) {
+  return new Set(
+    fileKeys.filter((key) => isFileDiffCollapsed(key, state.foldOverride, state.toggledFileKeys)),
+  );
+}
 
 describe("diff collapse controls", () => {
   it("reports whether every rendered file is collapsed", () => {
@@ -13,10 +28,72 @@ describe("diff collapse controls", () => {
   });
 
   it("collapses all files when any rendered file is expanded", () => {
-    expect(toggleAllDiffFiles(FILE_KEYS, new Set([FIRST_FILE_KEY]))).toEqual(new Set(FILE_KEYS));
+    const partial = setDiffFileCollapsed(FIRST_FILE_KEY, EXPANDED, true);
+    const collapsed = toggleAllDiffFiles(FILE_KEYS, partial);
+    expect(areAllDiffFilesCollapsed(FILE_KEYS, collapsedKeys(FILE_KEYS, collapsed))).toBe(true);
   });
 
   it("expands all files when every rendered file is collapsed", () => {
-    expect(toggleAllDiffFiles(FILE_KEYS, new Set(FILE_KEYS))).toEqual(new Set());
+    const expanded = toggleAllDiffFiles(FILE_KEYS, FOLDED);
+    expect(collapsedKeys(FILE_KEYS, expanded)).toEqual(new Set());
+  });
+
+  it("keeps a newly added file collapsed when its pending patch finishes loading", () => {
+    const pendingFileKey = "new.ts\u0000new.ts\u0000change";
+    const loadedFileKey = "new.ts\u0000new.ts\u0000new";
+    const collapsed = toggleAllDiffFiles([FIRST_FILE_KEY, pendingFileKey], EXPANDED);
+
+    const loadedKeys = [FIRST_FILE_KEY, loadedFileKey];
+    expect(areAllDiffFilesCollapsed(loadedKeys, collapsedKeys(loadedKeys, collapsed))).toBe(true);
+  });
+
+  it("applies collapse and expand all to files arriving on later pages", () => {
+    const collapsed = toggleAllDiffFiles([FIRST_FILE_KEY], EXPANDED);
+    const nextPage = [...FILE_KEYS, "src/next-page.ts"];
+    expect(areAllDiffFilesCollapsed(nextPage, collapsedKeys(nextPage, collapsed))).toBe(true);
+
+    const expanded = toggleAllDiffFiles(FILE_KEYS, collapsed);
+    expect(collapsedKeys(nextPage, expanded)).toEqual(new Set());
+  });
+
+  it("keeps individual file choices when later pages arrive", () => {
+    const collapsed = toggleAllDiffFiles(FILE_KEYS, EXPANDED);
+    const revealed = setDiffFileCollapsed(FIRST_FILE_KEY, collapsed, false);
+    const nextPage = [...FILE_KEYS, "src/next-page.ts"];
+    expect(collapsedKeys(nextPage, revealed)).toEqual(new Set(nextPage.slice(1)));
+
+    const foldedAgain = setDiffFileCollapsed(FIRST_FILE_KEY, revealed, true);
+    expect(areAllDiffFilesCollapsed(nextPage, collapsedKeys(nextPage, foldedAgain))).toBe(true);
+  });
+
+  it("keeps a revealed lazy file expanded when its pending patch becomes a new file", () => {
+    const patch = getRenderablePatch(
+      [
+        "diff --git a/new.ts b/new.ts",
+        "new file mode 100644",
+        "--- /dev/null",
+        "+++ b/new.ts",
+        "@@ -0,0 +1 @@",
+        "+export const value = 1;",
+      ].join("\n"),
+    );
+    if (patch?.kind !== "files" || !patch.files[0]) throw new Error("Expected a new file diff");
+    const loaded = patch.files[0];
+    const pending = { ...loaded, type: "change" as const };
+    const pendingKey = buildFileDiffIdentityKey(pending, "file");
+    const loadedKey = buildFileDiffIdentityKey(loaded, "file");
+    const collapsed = toggleAllDiffFiles([FIRST_FILE_KEY, pendingKey], EXPANDED);
+    const revealed = setDiffFileCollapsed(pendingKey, collapsed, false);
+
+    expect(collapsedKeys([FIRST_FILE_KEY, loadedKey], revealed)).toEqual(new Set([FIRST_FILE_KEY]));
+  });
+
+  it("keeps repeated file reveals expanded and clears exceptions on the next bulk collapse", () => {
+    const revealed = setDiffFileCollapsed(FIRST_FILE_KEY, FOLDED, false);
+    const revealedAgain = setDiffFileCollapsed(FIRST_FILE_KEY, revealed, false);
+    expect(collapsedKeys(FILE_KEYS, revealedAgain)).toEqual(new Set([FILE_KEYS[1]]));
+
+    const collapsed = toggleAllDiffFiles(FILE_KEYS, revealedAgain);
+    expect(areAllDiffFilesCollapsed(FILE_KEYS, collapsedKeys(FILE_KEYS, collapsed))).toBe(true);
   });
 });