fix(mobile): simplify PR review file scrolling

apps/mobile/src/components/pr-review/diff/pr-diff-file-list-render.tsx

@@ -22,18 +22,16 @@ import { collapseOnMarkViewed } from '@/lib/pr-review/diff/collapse-on-mark-view
 import { type ExpandSeparatorItem, type ListItem } from '@/lib/pr-review/diff/pr-diff-list-items';
 import { type ParsedHunk } from '@/lib/pr-review/diff/parse-patch';
 import { sideForDiffLineType } from '@/lib/pr-review/diff-selection';
-import {
-  type FetchToCompletionResult,
-  type UsePrReviewFileListQueryResult,
-} from '@/lib/pr-review/diff/pr-review-file-list-state';
 
 type UseDiffRenderItemArgs = {
   viewed: {
     isViewed: (path: string) => boolean;
     toggle: (path: string) => Promise<void>;
   };
-  query: UsePrReviewFileListQueryResult['query'];
-  fetchToCompletion: FetchToCompletionResult;
+  /** Retries the failed next page (pagination row). */
+  onRetryPage: () => void;
+  /** Drives the query to completion ("Load all"). */
+  onFetchAll: () => void;
   handleLoadContext: (item: ExpandSeparatorItem, windowSize: number) => void;
   setExpanded: React.Dispatch<React.SetStateAction<Record<string, boolean>>>;
   /** Producer-side tap handler. Receives the parsed data needed to run
@@ -65,8 +63,8 @@ export type LineTapArgs = {
 
 export function useDiffRenderItem({
   viewed,
-  query,
-  fetchToCompletion,
+  onRetryPage,
+  onFetchAll,
   handleLoadContext,
   setExpanded,
   onLineTap,
@@ -170,12 +168,8 @@ export function useDiffRenderItem({
               state={item.state}
               loadedFiles={item.loadedFiles}
               totalFiles={item.totalFiles}
-              onRetry={() => {
-                void query.fetchNextPage();
-              }}
-              onFetchAll={() => {
-                void fetchToCompletion.run();
-              }}
+              onRetry={onRetryPage}
+              onFetchAll={onFetchAll}
             />
           );
         }
@@ -184,6 +178,6 @@ export function useDiffRenderItem({
         }
       }
     },
-    [viewed, query, fetchToCompletion, handleLoadContext, setExpanded, onLineTap, selection]
+    [viewed, onRetryPage, onFetchAll, handleLoadContext, setExpanded, onLineTap, selection]
   );
 }

apps/mobile/src/components/pr-review/diff/pr-diff-file-list.tsx

@@ -4,10 +4,11 @@
 // Architecture:
 //   * A single FlashList with mixed item kinds (see `pr-diff-list-items`)
 //   * `usePrReviewFileListQuery` drives a tRPC infinite query for `listFiles`
+//     and produces a deduped `files` array via `flattenFilePages`
 //   * `usePrReviewViewedFiles` reads + toggles the per-PR viewed set
-//   * `useFetchToCompletion` lets S6c's navigator drive the query to its end
+//   * `useFetchToCompletion` lets the navigator drive the query to its end
 //   * `subscribeFileNavigatorRequest` is consumed here so a "scroll to file"
-//     request (emitted by S6c) snaps the list to the right section
+//     request from the navigator sheet snaps the list to the right section
 //   * S7a adds diff-line selection: tapping a line runs the pure
 //     `selectLine` reducer; the result is mirrored into the
 //     `diff-selection-bridge` (so the comment composer can read it on
@@ -23,7 +24,7 @@
 // makes cold match warm.
 
 import { FlashList, type FlashListRef } from '@shopify/flash-list';
-import { useEffect, useMemo, useRef, useState } from 'react';
+import { useCallback, useEffect, useMemo, useRef, useState } from 'react';
 import { View } from 'react-native';
 
 import { QueryError } from '@/components/query-error';
@@ -45,8 +46,7 @@ import {
   LIST_CONTENT_STYLE,
   TabStateMessage,
 } from '@/components/pr-review/diff/pr-diff-rows';
-import { dedupeFilesByPath } from '@/lib/pr-review/diff/dedupe-file-pages';
-import { buildItems } from '@/lib/pr-review/diff/pr-diff-list-builder';
+import { buildFileItems, buildPaginationItem } from '@/lib/pr-review/diff/pr-diff-list-builder';
 import { itemTypeFor, type ListItem } from '@/lib/pr-review/diff/pr-diff-list-items';
 import { stickyFileHeaderIndices } from '@/lib/pr-review/diff/sticky-file-headers';
 import { usePrDiffContextLoader } from '@/lib/pr-review/diff/use-pr-diff-context-loader';
@@ -55,7 +55,6 @@ import {
   usePrReviewFileListQuery,
   usePrReviewViewedFiles,
 } from '@/lib/pr-review/diff/pr-review-file-list-state';
-import { type PrReviewFile } from '@/lib/pr-review/diff/pr-review-file-types';
 import { usePrDiffListScroll } from '@/lib/pr-review/diff/use-pr-diff-list-scroll';
 import { clearDiffSelection } from '@/lib/pr-review/diff-selection-bridge';
 import { useIsTablet } from '@/lib/hooks/use-is-tablet';
@@ -85,7 +84,7 @@ export function PrReviewFileList({
   // diff rows receive the live scale and resize to fit.
   const diffFontMetrics = useBoundedDiffFontMetrics();
 
-  const { query, firstPageErrorState } = usePrReviewFileListQuery({
+  const { query, files, firstPageErrorState } = usePrReviewFileListQuery({
     owner,
     repo,
     number,
@@ -116,18 +115,6 @@ export function PrReviewFileList({
   // list always starts with no selection.
   useEffect(() => clearDiffSelection, []);
 
-  const files = useMemo(() => {
-    const all: PrReviewFile[] = [];
-    for (const page of query.data?.pages ?? []) {
-      for (const f of page.files) {
-        all.push(f);
-      }
-    }
-    // Dedupe by path (first wins) so retry/refetch races cannot emit
-    // duplicate `file-header:<path>` keys into FlashList.
-    return dedupeFilesByPath(all);
-  }, [query.data]);
-
   const viewedCount = useMemo(() => {
     let count = 0;
     for (const file of files) {
@@ -138,9 +125,11 @@ export function PrReviewFileList({
     return count;
   }, [files, viewed]);
 
-  const items = useMemo(
+  const effectiveViewMode = isTablet ? viewMode : 'unified';
+
+  const fileItems = useMemo(
     () =>
-      buildItems({
+      buildFileItems({
         files,
         expanded,
         expandedContext,
@@ -150,14 +139,15 @@ export function PrReviewFileList({
         repo,
         number,
         changedFiles,
-        isLoading: query.isLoading,
-        isFetchingNextPage: query.isFetchingNextPage,
-        hasNextPage: query.hasNextPage,
-        laterPageError: query.isError && files.length > 0,
-        fetchToCompletionRunning: fetchToCompletion.isRunning,
-        fetchToCompletionLoaded: fetchToCompletion.loadedFiles,
-        totalFiles: changedFiles,
-        viewMode: isTablet ? viewMode : 'unified',
+        viewMode: effectiveViewMode,
+        // Pagination fields — required by BuildItemsArgs but unused by buildFileItems.
+        isLoading: false,
+        isFetchingNextPage: false,
+        hasNextPage: false,
+        laterPageError: false,
+        fetchToCompletionRunning: false,
+        fetchToCompletionLoaded: 0,
+        totalFiles: null,
       }),
     [
       files,
@@ -169,33 +159,76 @@ export function PrReviewFileList({
       repo,
       number,
       changedFiles,
+      effectiveViewMode,
+    ]
+  );
+
+  const paginationItem = useMemo(
+    () =>
+      buildPaginationItem({
+        files,
+        expanded,
+        expandedContext,
+        viewed: viewed.isViewed,
+        headSha,
+        owner,
+        repo,
+        number,
+        changedFiles,
+        viewMode: effectiveViewMode,
+        isLoading: query.isLoading,
+        isFetchingNextPage: query.isFetchingNextPage,
+        hasNextPage: query.hasNextPage,
+        laterPageError: query.isError && files.length > 0,
+        fetchToCompletionRunning: fetchToCompletion.isRunning,
+        fetchToCompletionLoaded: fetchToCompletion.loadedFiles,
+        totalFiles: changedFiles,
+      }),
+    // buildPaginationItem only reads the pagination-specific fields below;
+    // the other fields are required by BuildItemsArgs but unused by this
+    // builder so they must not force a rebuild when file-level state changes.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+    [
       query.isLoading,
       query.isFetchingNextPage,
       query.hasNextPage,
       query.isError,
+      files.length,
       fetchToCompletion.isRunning,
       fetchToCompletion.loadedFiles,
-      viewMode,
-      isTablet,
+      changedFiles,
     ]
   );
 
+  const items = useMemo(() => [...fileItems, paginationItem], [fileItems, paginationItem]);
+
   const stickyHeaderIndices = useMemo(() => stickyFileHeaderIndices(items), [items]);
 
-  const { handleContentSizeChange } = usePrDiffListScroll({
+  usePrDiffListScroll({
     owner,
     repo,
     number,
-    filesLength: files.length,
     items,
     listRef,
     setExpanded,
   });
 
+  const onFetchAll = useCallback(() => {
+    void fetchToCompletion.run();
+    // fetchToCompletion.run is a stable useCallback reference; the full
+    // fetchToCompletion object would change every render.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
+  }, [fetchToCompletion.run]);
+
   const renderItem = useDiffRenderItem({
     viewed,
-    query,
-    fetchToCompletion,
+    onRetryPage: useCallback(() => {
+      void query.fetchNextPage();
+      // query.fetchNextPage is a stable reference in React Query v5;
+      // depending on `query` would make this callback new on every render.
+      // eslint-disable-next-line react-hooks/exhaustive-deps
+    }, [query.fetchNextPage]),
+    onFetchAll,
     handleLoadContext,
     setExpanded,
     onLineTap: handleLineTap,
@@ -246,7 +279,6 @@ export function PrReviewFileList({
   }
 
   const isTruncated = query.hasNextPage || Boolean(fetchToCompletion.error);
-  const effectiveViewMode = isTablet ? viewMode : 'unified';
   // First page still in flight: keep FlashList unmounted. A list that first
   // lays out a single loading pagination-row and later receives the real file
   // rows can measure full content height without mounting cells until scroll.
@@ -281,7 +313,6 @@ export function PrReviewFileList({
             maintainVisibleContentPosition={{ disabled: true }}
             // Re-measure rows when the bounded font scale changes.
             extraData={diffFontMetrics.scale}
-            onContentSizeChange={handleContentSizeChange}
             onEndReached={() => {
               if (query.hasNextPage && !query.isFetchingNextPage) {
                 void query.fetchNextPage();

apps/mobile/src/components/pr-review/diff/pr-diff-file-navigator.tsx

@@ -79,7 +79,7 @@ export function PrDiffFileNavigator({
   const [searchVersion, setSearchVersion] = useState(0);
   const inputRef = useRef<TextInput | null>(null);
 
-  const { query, firstPageErrorState } = usePrReviewFileListQuery({
+  const { query, files, firstPageErrorState } = usePrReviewFileListQuery({
     owner,
     repo,
     number,
@@ -101,16 +101,6 @@ export function PrDiffFileNavigator({
     }
   }, [query.isFetching, query.hasNextPage, fetchAll.isRunning, fetchAll.error]);
 
-  const files = useMemo(() => {
-    const all: PrReviewFile[] = [];
-    for (const page of query.data?.pages ?? []) {
-      for (const f of page.files) {
-        all.push(f);
-      }
-    }
-    return all;
-  }, [query.data]);
-
   const filtered = useMemo(
     () => filterFiles(files, searchRef.current),
     // `searchVersion` is the only thing that signals "the ref changed",

apps/mobile/src/lib/pr-review/diff/dedupe-file-pages.test.ts

@@ -1,6 +1,6 @@
 import { describe, expect, it } from 'vitest';
 
-import { dedupeFilesByPath } from './dedupe-file-pages';
+import { dedupeFilesByPath, flattenFilePages } from './dedupe-file-pages';
 
 describe('dedupeFilesByPath', () => {
   it('returns the same order when there are no duplicates', () => {
@@ -35,3 +35,59 @@ describe('dedupeFilesByPath', () => {
     expect(dedupeFilesByPath([])).toEqual([]);
   });
 });
+
+describe('flattenFilePages', () => {
+  it('returns [] for undefined pages', () => {
+    expect(flattenFilePages(undefined)).toEqual([]);
+  });
+
+  it('returns [] for empty pages array', () => {
+    expect(flattenFilePages([])).toEqual([]);
+  });
+
+  it('concatenates non-overlapping pages in page order', () => {
+    const pages = [{ files: [{ path: 'a.ts' }, { path: 'b.ts' }] }, { files: [{ path: 'c.ts' }] }];
+    expect(flattenFilePages(pages)).toEqual([{ path: 'a.ts' }, { path: 'b.ts' }, { path: 'c.ts' }]);
+  });
+
+  it('dedupes a path repeated across page boundaries, keeping first occurrence', () => {
+    const first = { path: 'a.ts', page: 1 };
+    const second = { path: 'a.ts', page: 2 };
+    const pages = [{ files: [first] }, { files: [second] }];
+    expect(flattenFilePages(pages)).toEqual([first]);
+  });
+
+  it('works with a single page', () => {
+    const pages = [{ files: [{ path: 'x.ts' }, { path: 'y.ts' }] }];
+    expect(flattenFilePages(pages)).toEqual([{ path: 'x.ts' }, { path: 'y.ts' }]);
+  });
+
+  it('dedupes within and across pages', () => {
+    const pages = [
+      {
+        files: [
+          { path: 'a.ts', n: 1 },
+          { path: 'b.ts', n: 1 },
+        ],
+      },
+      {
+        files: [
+          { path: 'a.ts', n: 2 },
+          { path: 'c.ts', n: 2 },
+        ],
+      },
+      {
+        files: [
+          { path: 'b.ts', n: 3 },
+          { path: 'd.ts', n: 3 },
+        ],
+      },
+    ];
+    expect(flattenFilePages(pages)).toEqual([
+      { path: 'a.ts', n: 1 },
+      { path: 'b.ts', n: 1 },
+      { path: 'c.ts', n: 2 },
+      { path: 'd.ts', n: 3 },
+    ]);
+  });
+});

apps/mobile/src/lib/pr-review/diff/dedupe-file-pages.ts

@@ -13,3 +13,16 @@ export function dedupeFilesByPath<T extends { readonly path: string }>(files: re
   }
   return result;
 }
+
+/** Flatten infinite-query pages into a single deduped file list (first occurrence wins). */
+export function flattenFilePages<T extends { readonly path: string }>(
+  pages: readonly { readonly files: readonly T[] }[] | undefined
+): T[] {
+  const all: T[] = [];
+  for (const page of pages ?? []) {
+    for (const file of page.files) {
+      all.push(file);
+    }
+  }
+  return dedupeFilesByPath(all);
+}