fix(review): complete large diffs and improve review panel defaults

apps/desktop/src/settings/DesktopClientSettings.test.ts

@@ -42,6 +42,7 @@ const clientSettings: ClientSettings = {
   desktopNotifyOnFailure: true,
   composerCollapseOnScroll: true,
   dismissedProviderUpdateNotificationKeys: [],
+  diffFilesCollapsed: true,
   diffIgnoreWhitespace: true,
   diffLayout: "stacked",
   environmentIdentificationMode: "artwork",

apps/mobile/src/features/review/ReviewSheet.tsx

@@ -382,24 +382,40 @@ export function ReviewSheet(props: ReviewSheetProps) {
   useEffect(() => {
     showAuxiliaryPane("inspector");
   }, [environmentId, showAuxiliaryPane, threadId]);
-  const { error, reviewSections, selectedSection, refreshSelectedSection, selectSection } =
-    useReviewSections({
-      enabled: isEnvironmentReady,
-      environmentId,
-      threadId,
-      reviewCache,
-    });
+  const {
+    error,
+    reviewSections,
+    selectedSection,
+    refreshSelectedSection,
+    selectSection,
+    isSelectedSectionPending,
+    diffPreviewRevision,
+  } = useReviewSections({
+    enabled: isEnvironmentReady,
+    environmentId,
+    threadId,
+    reviewCache,
+  });
   useReviewDiffPrewarming({
     threadKey: reviewCache.threadKey,
     sections: reviewSections,
     selectedSectionId: selectedSection?.id ?? null,
   });
-  const { headerDiffSummary, nativeReviewDiffData, parsedDiff, pendingReviewCommentCount } =
-    useReviewDiffData({
-      threadKey: reviewCache.threadKey,
-      selectedSection,
-      draftMessage,
-    });
+  const {
+    headerDiffSummary,
+    nativeReviewDiffData,
+    parsedDiff,
+    pendingReviewCommentCount,
+    loadVisibleFile,
+    isPending: areFilePatchesPending,
+  } = useReviewDiffData({
+    threadKey: reviewCache.threadKey,
+    environmentId,
+    cwd: selectedThreadCwd,
+    selectedSection,
+    revision: diffPreviewRevision,
+    draftMessage,
+  });
   // Resolution returns null while Expo registers the native view (or forever
   // when the binary lacks it). Rendering a null component type crashes the
   // app, so callers must fall back — ThreadFeed's ReviewCommentCard does the
@@ -469,6 +485,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
 
   const handleSelectFile = useCallback(
     (fileId: string | null) => {
+      loadVisibleFile(fileId, true);
       commentSelection.clearSelection();
       if (fileId !== null && collapsedFileIds.includes(fileId)) {
         toggleExpandedFile(fileId);
@@ -481,13 +498,14 @@ export function ReviewSheet(props: ReviewSheetProps) {
         console.error("[review] Failed to navigate to diff file", error);
       });
     },
-    [collapsedFileIds, commentSelection, toggleExpandedFile],
+    [collapsedFileIds, commentSelection, toggleExpandedFile, loadVisibleFile],
   );
   const handleVisibleFileChange = useCallback(
     (event: NativeSyntheticEvent<{ readonly fileId?: string | null }>) => {
+      loadVisibleFile(event.nativeEvent.fileId ?? null);
       reviewFileNavigatorRef.current?.setVisibleFile(event.nativeEvent.fileId ?? null);
     },
-    [],
+    [loadVisibleFile],
   );
   const renderInspector = useCallback(
     () => (
@@ -508,10 +526,11 @@ export function ReviewSheet(props: ReviewSheetProps) {
     (event: NativeSyntheticEvent<{ readonly fileId?: string }>) => {
       const { fileId } = event.nativeEvent;
       if (fileId) {
+        loadVisibleFile(fileId, true);
         toggleExpandedFile(fileId);
       }
     },
-    [toggleExpandedFile],
+    [toggleExpandedFile, loadVisibleFile],
   );
 
   const handleNativeToggleViewedFile = useCallback(
@@ -574,12 +593,12 @@ export function ReviewSheet(props: ReviewSheetProps) {
     (event: { nativeEvent: { event: string } }) => {
       const id = event.nativeEvent.event;
       if (id === "refresh") {
-        void refreshSelectedSection();
+        void handlePullToRefresh();
       } else if (id.startsWith("section:")) {
         selectSection(id.slice("section:".length));
       }
     },
-    [refreshSelectedSection, selectSection],
+    [handlePullToRefresh, selectSection],
   );
   const handleRetryEnvironment = useCallback(() => {
     void retryEnvironment(environmentId);
@@ -818,7 +837,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
                 <NativeReviewDiffView
                   collapsable={false}
                   testID="review-native-diff-view"
-                  refreshing={isPullRefreshing}
+                  refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
                   onPullToRefresh={() => void handlePullToRefresh()}
                   style={StyleSheet.absoluteFill}
                   appearanceScheme={selectedTheme}
@@ -862,7 +881,7 @@ export function ReviewSheet(props: ReviewSheetProps) {
               // iOS has no other refresh affordance here (the explicit
               // "Refresh current diff" menu is Android-only).
               <RefreshControl
-                refreshing={isPullRefreshing}
+                refreshing={isPullRefreshing || isSelectedSectionPending || areFilePatchesPending}
                 onRefresh={() => void handlePullToRefresh()}
               />
             }

apps/mobile/src/features/review/nativeReviewDiffAdapter.test.ts

@@ -98,6 +98,29 @@ function appTheme(themeId: MobileThemeId, appearance: MobileThemeAppearance) {
 }
 
 describe("getCachedNativeReviewDiffData", () => {
+  it.each([true, false])(
+    "preserves available diff rows before a notice (has excerpt: %s)",
+    (hasExcerpt) => {
+      if (parsedDiff.kind !== "files") throw new Error("Expected a parsed file diff");
+      const notice = "This file preview was truncated.";
+      const result = buildNativeReviewDiffData({
+        parsedDiff: {
+          ...parsedDiff,
+          files: parsedDiff.files.map((file) => ({
+            ...file,
+            rows: hasExcerpt ? file.rows : [],
+            notice,
+          })),
+        },
+      });
+      const original = buildNativeReviewDiffData({ parsedDiff });
+      expect(result.rows.slice(0, -1)).toEqual(
+        hasExcerpt ? original.rows : original.rows.filter((row) => row.kind === "file"),
+      );
+      expect(result.rows.at(-1)).toMatchObject({ kind: "notice", text: notice });
+    },
+  );
+
   it("reuses the row model for equivalent empty comment arrays", () => {
     const first = getCachedNativeReviewDiffData(buildInput([]));
     const second = getCachedNativeReviewDiffData(buildInput([]));

apps/mobile/src/features/review/nativeReviewDiffAdapter.ts

@@ -125,6 +125,8 @@ interface PreparedNativeReviewFileRows {
   readonly filePath: string;
   readonly lineCount: number;
   readonly rows: ReadonlyArray<NativeReviewDiffRow>;
+  readonly commentTargetsByRowId: ReadonlyMap<string, NativeReviewDiffCommentTarget>;
+  readonly rowIdByCommentLineId: ReadonlyMap<string, string>;
   commentedRows: {
     readonly commentsKey: string;
     readonly rows: ReadonlyArray<NativeReviewDiffRow>;
@@ -136,6 +138,7 @@ interface PreparedNativeReviewDiffData extends Omit<NativeReviewDiffData, "rows"
 }
 
 const nativeReviewDiffDataCache = new WeakMap<ReviewParsedDiff, CachedNativeReviewDiffData>();
+const nativeReviewFileRowsCache = new WeakMap<ReviewRenderableFile, PreparedNativeReviewFileRows>();
 
 function buildReviewCommentsCacheKey(comments: ReadonlyArray<ReviewInlineComment>): string {
   if (comments.length === 0) {
@@ -261,6 +264,7 @@ function createNoticeRow(fileId: string, suffix: string, text: string): NativeRe
 }
 
 function noticeRowsForFile(file: ReviewRenderableFile): ReadonlyArray<NativeReviewDiffRow> {
+  if (file.notice) return [createNoticeRow(file.id, "loading", file.notice)];
   if (file.rows.length > 0) {
     return [];
   }
@@ -403,11 +407,11 @@ function mapLineRow(
   };
 }
 
-function prepareFileRows(
-  file: ReviewRenderableFile,
-  commentTargetsByRowId: Map<string, NativeReviewDiffCommentTarget>,
-  rowIdByCommentLineId: Map<string, string>,
-): PreparedNativeReviewFileRows {
+function prepareFileRows(file: ReviewRenderableFile): PreparedNativeReviewFileRows {
+  const cached = nativeReviewFileRowsCache.get(file);
+  if (cached) return cached;
+  const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
+  const rowIdByCommentLineId = new Map<string, string>();
   const rows: NativeReviewDiffRow[] = [
     {
       kind: "file",
@@ -446,14 +450,18 @@ function prepareFileRows(
   });
 
   rows.push(...noticeRowsForFile(file));
-  return {
+  const prepared: PreparedNativeReviewFileRows = {
     fileId: file.id,
     filePath: file.path,
     lineCount: lineRows.length,
     // Comments must not split the source deletion/addition runs used for word matching.
     rows: addNativeWordDiffRanges(rows),
+    commentTargetsByRowId,
+    rowIdByCommentLineId,
     commentedRows: null,
   };
+  nativeReviewFileRowsCache.set(file, prepared);
+  return prepared;
 }
 
 function insertFileComments(
@@ -523,9 +531,15 @@ function prepareNativeReviewDiffData(parsedDiff: ReviewParsedDiff): PreparedNati
   }));
   const commentTargetsByRowId = new Map<string, NativeReviewDiffCommentTarget>();
   const rowIdByCommentLineId = new Map<string, string>();
-  const fileRows = parsedDiff.files.map((file) =>
-    prepareFileRows(file, commentTargetsByRowId, rowIdByCommentLineId),
-  );
+  const fileRows = parsedDiff.files.map(prepareFileRows);
+  for (const file of fileRows) {
+    for (const [rowId, target] of file.commentTargetsByRowId) {
+      commentTargetsByRowId.set(rowId, target);
+    }
+    for (const [lineId, rowId] of file.rowIdByCommentLineId) {
+      rowIdByCommentLineId.set(lineId, rowId);
+    }
+  }
 
   return {
     fileRows,

apps/mobile/src/features/review/reviewModel.test.ts

@@ -8,6 +8,7 @@ import {
 } from "@t3tools/contracts";
 
 import {
+  applyReviewDiffMetadata,
   buildReviewParsedDiff,
   buildReviewSectionItems,
   getDefaultReviewSectionId,
@@ -135,6 +136,35 @@ describe("buildReviewSectionItems", () => {
 });
 
 describe("buildReviewParsedDiff", () => {
+  it.each([
+    ["a/example.ts", "a/example.ts"],
+    ["b/example.ts", "b/example.ts"],
+    ["a/before.ts", "b/after.ts"],
+  ])("preserves repository paths from %s to %s", (previousPath, path) => {
+    const parsed = buildReviewParsedDiff(
+      [
+        `diff --git a/${previousPath} b/${path}`,
+        ...(previousPath === path
+          ? []
+          : ["similarity index 50%", `rename from ${previousPath}`, `rename to ${path}`]),
+        `--- a/${previousPath}`,
+        `+++ b/${path}`,
+        "@@ -1 +1 @@",
+        "-before",
+        "+after",
+      ].join("\n"),
+      "repository-paths",
+    );
+    expect(parsed.kind).toBe("files");
+    if (parsed.kind !== "files") return;
+    expect(parsed.files[0]).toMatchObject({
+      path,
+      previousPath: previousPath === path ? null : previousPath,
+      additions: 1,
+      deletions: 1,
+    });
+  });
+
   it("builds renderable rows from a unified patch", () => {
     const parsed = buildReviewParsedDiff(
       [
@@ -271,3 +301,34 @@ describe("buildReviewParsedDiff", () => {
     });
   });
 });
+
+describe("applyReviewDiffMetadata", () => {
+  it("uses complete counts even when the preview contains only part of one file", () => {
+    const parsed = buildReviewParsedDiff(
+      [
+        "diff --git a/large.txt b/large.txt",
+        "--- a/large.txt",
+        "+++ b/large.txt",
+        "@@ -1 +1 @@",
+        "-before",
+        "+after",
+      ].join("\n"),
+      "partial",
+    );
+    const result = applyReviewDiffMetadata(parsed, {
+      truncated: true,
+      files: [
+        { path: "large.txt", previousPath: null, additions: 4000, deletions: 3000 },
+        { path: "unseen.txt", previousPath: null, additions: 100, deletions: 20 },
+      ],
+    });
+    expect(result.kind).toBe("files");
+    if (result.kind !== "files") return;
+    expect(result.fileCount).toBe(2);
+    expect(result.additions).toBe(4100);
+    expect(result.deletions).toBe(3020);
+    expect(result.files[0]?.additions).toBe(4000);
+    expect(result.notice).toContain("Counts include all changes");
+    expect(applyReviewDiffMetadata(parsed, null)).toEqual(parsed);
+  });
+});