Server-side pagination for the file list (#476)#481
Conversation
FileList fetched a hard-capped `pageSize: 100` and rendered every row inside a fixed-height scroll box, so uploads beyond 100 were silently invisible — the last truncation case in #476. The backend GET /files already returns a proper 1-indexed PagedResponse (page/pageSize/sort/source), so this is a frontend fix. - Drive page/pageSize/total(Items|Pages) from the server response; reset to page 1 when the page size or monitor filter changes. - Map the "hide monitor files" toggle to `?source=ANALYSIS` (ANALYSIS is the only non-monitor source) instead of filtering a client-side array, so counts and pages stay authoritative. Removed the now-redundant client filter + scroll box. - Refetch the current page after delete / delete-page / merge (stepping back a page if the last row on a non-first page is removed) rather than mutating a local array. - Show the pager (with page-size selector) whenever the total exceeds the smallest page-size option, so bumping the size to one page can't strand the user — same fix as #479. - "Delete all" now honestly deletes only the current page (there is no bulk endpoint, and fetching all ids would reintroduce an unbounded fetch); the modal states the scope and how many files remain on other pages. Relabelled the trigger/modal to "Delete page" to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughFileList.tsx was updated to implement server-side pagination, replacing a single bulk fetch with paginated requests tracking page, pageSize, totalItems, and totalPages. Compare selection now persists filenames across pages, and delete operations (single-file and page-level) were changed to refetch data rather than mutate local state. ChangesFileList Pagination and Deletion
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant FileList
participant API
User->>FileList: toggle hideMonitor filter
FileList->>FileList: reset page to 1
FileList->>API: fetchFiles(page, pageSize, source)
API-->>FileList: files, totalItems, totalPages
FileList-->>User: render paged file list
User->>FileList: confirm "Delete Page"
FileList->>API: delete each file in current page (allSettled)
API-->>FileList: deletion results
FileList->>FileList: step back page or refetch current page
FileList->>API: fetchFiles(page, pageSize, source)
API-->>FileList: updated files, totals
FileList-->>User: render updated page
Possibly related issues
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces server-side pagination to the FileList component, updating file fetching, deletion, and filtering logic to support paginated results. Feedback highlights critical issues with this transition: first, using a Set of IDs for selected files breaks features like auto-merge name generation and monitor file deselection because the files array now only holds the current page's data; second, a useEffect hook resetting the page causes redundant API requests and race conditions, which should be resolved by batching state updates in event handlers; and finally, page deletion logic should only step back a page if the deleted page was the last page.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
- Drop the setPage(1) effect and reset the page directly in the monitor toggle and onPageSizeChange, so a filter/size change fires a single page-1 request instead of a stale-page fetch followed by a second reset fetch. - buildAutoMergeName now reads an accumulated id→filename map recorded at selection time, not `files` (current page only), so off-page selections still contribute to the merged name. Cleared with the selection after a merge. - Both delete paths step back a page only when the emptied page was the last one (page === totalPages); earlier pages backfill from below, so refetch in place. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
frontend/src/components/upload/FileList/FileList.tsx (1)
257-271: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick winMove
setPage/setSelectedForCompareout of thesetHideMonitorupdater.The function passed to
setHideMonitorshould be a pure state derivation, but it triggerssetPage(1)andsetSelectedForCompare(...)as side effects. Under React 19 Strict Mode, updater functions are intentionally double-invoked, so these nested setters run twice. They happen to be idempotent here, but this is a fragile anti-pattern — computenextand dispatch the setters from the handler body instead.Also note the monitor cleanup filters only the current-page
files, so monitor selections from other pages remain inselectedForCompareonce server-side paging is active.♻️ Proposed refactor
- onClick={() => setHideMonitor(v => { - const next = !v; - // New filter → back to page 1 so the current page can't fall out of range. - setPage(1); - if (next) { - setSelectedForCompare(prev => { - const monitorIds = new Set(files.filter(f => f.source === 'MONITOR').map(f => f.fileId)); - if ([...prev].every(id => !monitorIds.has(id))) return prev; - const next = new Set(prev); - monitorIds.forEach(id => next.delete(id)); - return next; - }); - } - return next; - })} + onClick={() => { + const next = !hideMonitor; + setHideMonitor(next); + // New filter → back to page 1 so the current page can't fall out of range. + setPage(1); + if (next) { + setSelectedForCompare(prev => { + const monitorIds = new Set(files.filter(f => f.source === 'MONITOR').map(f => f.fileId)); + if ([...prev].every(id => !monitorIds.has(id))) return prev; + const updated = new Set(prev); + monitorIds.forEach(id => updated.delete(id)); + return updated; + }); + } + }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@frontend/src/components/upload/FileList/FileList.tsx` around lines 257 - 271, The `setHideMonitor` updater in `FileList` is doing side effects by calling `setPage(1)` and `setSelectedForCompare(...)`, which should be moved out of the state updater and into the click handler body. Compute the next hide/show value first, then dispatch `setHideMonitor`, `setPage`, and the compare-selection cleanup separately so the updater stays pure and safe under React 19 Strict Mode. While refactoring, make the monitor selection cleanup use the full monitor-file set rather than only the current-page `files`, so `selectedForCompare` stays consistent with server-side paging.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@frontend/src/components/upload/FileList/FileList.tsx`:
- Around line 257-271: The `setHideMonitor` updater in `FileList` is doing side
effects by calling `setPage(1)` and `setSelectedForCompare(...)`, which should
be moved out of the state updater and into the click handler body. Compute the
next hide/show value first, then dispatch `setHideMonitor`, `setPage`, and the
compare-selection cleanup separately so the updater stays pure and safe under
React 19 Strict Mode. While refactoring, make the monitor selection cleanup use
the full monitor-file set rather than only the current-page `files`, so
`selectedForCompare` stays consistent with server-side paging.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 13ce348c-81b0-452f-bf1d-380d6bd8c055
📒 Files selected for processing (1)
frontend/src/components/upload/FileList/FileList.tsx
1 participant
Closes #476 — the final piece (the drift-panel half landed in #480).
Problem
FileListfetched a hard-cappedpageSize: 100and rendered every row inside a fixed-height scroll box, so on the prod-scale target (millions of files) only the 100 most-recent uploads were ever reachable — truncation, not pagination.Change (frontend-only)
The backend
GET /filesalready returns a proper 1-indexedPagedResponse(page/pageSize/sort/source), so no backend change was needed:page/pageSize/totalItems/totalPagesfrom the server response; reset to page 1 when page size or the monitor filter changes.?source=ANALYSIS(ANALYSIS is the only non-monitorFileSource) instead of filtering a client array, so counts/pages stay authoritative. Removed the redundant client filter +maxHeightscroll box.totalItems > 10(smallest page-size option), so bumping the size to a single page can't strand the user — the same fix as Paginate the extracted-files table (#478) #479.Verification
tsc --noEmitclean;docker compose up -d --buildsucceeds.…&source=ANALYSIS, no pager.source.page=2&pageSize=10.pageSize:100cap is gone.With this, every truncating fetch identified in the pagination audit is resolved.
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Bug Fixes