From 0323b42667e042c1ef3f098c83ae0c0664eeb286 Mon Sep 17 00:00:00 2001 From: Lukas May Date: Fri, 6 Mar 2026 19:50:53 +0100 Subject: [PATCH 1/3] feat: virtualize ReviewSidebar file list for >50 items with scroll preservation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds windowed rendering to FilesView in ReviewSidebar.tsx using react-window 2.x (List component). File lists with more than 50 rows render only visible items, keeping the DOM lean for large diffs. - Install react-window 2.x and @types/react-window in apps/web - Flatten directory-grouped file tree into a typed Row[] array via useMemo - Use VariableSizeList-equivalent react-window 2.x List with rowHeight fn (32px for dir-headers, 40px for file rows); falls back to plain DOM render for ≤50 rows to avoid overhead on small diffs - Directories are collapsible: clicking the dir-header toggles collapse, removing its file rows from the Row[] and from the virtual list - Preserve sidebar scroll offset across Files ↔ Commits tab switches via filesScrollOffsetRef passed from ReviewSidebar into FilesView - Clicking a file calls listRef.scrollToRow({ index, align: "smart" }) to keep the clicked row visible in the virtual list - Root-level files (directory === "") render without a dir-header, preserving existing behavior - Add resolve.dedupe for react/react-dom in vitest.config.ts to prevent duplicate-React errors after local workspace package installation - Add 6 Vitest + RTL tests covering: large-list DOM count, small-list fallback, collapse, re-expand, tab-switch smoke, root-level files Co-Authored-By: Claude Sonnet 4.6 --- apps/web/package.json | 2 + .../components/review/ReviewSidebar.test.tsx | 162 +++++ .../src/components/review/ReviewSidebar.tsx | 585 +++++++++++++----- docs/frontend.md | 3 +- package-lock.json | 21 + vitest.config.ts | 3 + 6 files changed, 612 insertions(+), 164 deletions(-) create mode 100644 apps/web/src/components/review/ReviewSidebar.test.tsx diff --git a/apps/web/package.json b/apps/web/package.json index bae73ef..d0caa00 100644 --- a/apps/web/package.json +++ b/apps/web/package.json @@ -27,12 +27,14 @@ "@tiptap/suggestion": "^3.19.0", "@trpc/client": "^11.9.0", "@trpc/react-query": "^11.9.0", + "@types/react-window": "^1.8.8", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "geist": "^1.7.0", "lucide-react": "^0.563.0", "react": "^19.0.0", "react-dom": "^19.0.0", + "react-window": "^2.2.7", "sonner": "^2.0.7", "tailwind-merge": "^3.4.0", "tippy.js": "^6.3.7" diff --git a/apps/web/src/components/review/ReviewSidebar.test.tsx b/apps/web/src/components/review/ReviewSidebar.test.tsx new file mode 100644 index 0000000..8d6c2d4 --- /dev/null +++ b/apps/web/src/components/review/ReviewSidebar.test.tsx @@ -0,0 +1,162 @@ +// @vitest-environment happy-dom +import '@testing-library/jest-dom/vitest'; +import { render, screen, fireEvent, act } from '@testing-library/react'; +import { vi, describe, it, expect, beforeEach, afterEach } from 'vitest'; +import { ReviewSidebar } from './ReviewSidebar'; +import type { FileDiff, ReviewComment, CommitInfo } from './types'; + +// Mock ResizeObserver — not provided by happy-dom. +// Must be a class (react-window 2.x uses `new ResizeObserver(...)`). +class MockResizeObserver { + observe() {} + unobserve() {} + disconnect() {} +} +vi.stubGlobal('ResizeObserver', MockResizeObserver); + +// ─── Helpers ───────────────────────────────────────────────────────────────── + +function makeFile(path: string): FileDiff { + return { + oldPath: path, + newPath: path, + hunks: [], + additions: 1, + deletions: 0, + changeType: 'modified', + }; +} + +function makeFiles(count: number, prefix = 'src/components/'): FileDiff[] { + return Array.from({ length: count }, (_, i) => + makeFile(`${prefix}file${String(i).padStart(4, '0')}.ts`), + ); +} + +const NO_COMMENTS: ReviewComment[] = []; +const NO_COMMITS: CommitInfo[] = []; + +function renderSidebar(files: FileDiff[]) { + return render( + , + ); +} + +// ─── Tests ─────────────────────────────────────────────────────────────────── + +describe('ReviewSidebar FilesView virtualization', () => { + beforeEach(() => vi.clearAllMocks()); + afterEach(() => vi.restoreAllMocks()); + + it('renders only a subset of DOM rows when file count > 50', () => { + const files = makeFiles(200); + renderSidebar(files); + + const fileRows = document.querySelectorAll('[data-testid="file-row"]'); + // Virtualization keeps DOM rows << total count + expect(fileRows.length).toBeLessThan(100); + expect(fileRows.length).toBeGreaterThan(0); + }); + + it('renders all file rows when file count <= 50 (non-virtualized path)', () => { + const files = makeFiles(10); + renderSidebar(files); + + const fileRows = document.querySelectorAll('[data-testid="file-row"]'); + expect(fileRows.length).toBe(10); + }); + + it('removes file rows from DOM when a directory is collapsed', async () => { + // Use 60 files so we hit the >50 virtualized path + const files = makeFiles(60, 'src/components/'); + renderSidebar(files); + + // Initially, files should be rendered (at least some via virtualization) + const dirHeader = screen.getByRole('button', { name: /src\/components\// }); + expect(dirHeader).toBeInTheDocument(); + + const rowsBefore = document.querySelectorAll('[data-testid="file-row"]').length; + expect(rowsBefore).toBeGreaterThan(0); + + // Collapse the directory + await act(async () => { + fireEvent.click(dirHeader); + }); + + // After collapse, no file rows should be rendered for that directory + const rowsAfter = document.querySelectorAll('[data-testid="file-row"]').length; + expect(rowsAfter).toBe(0); + }); + + it('restores file rows when a collapsed directory is expanded again', async () => { + const files = makeFiles(60, 'src/components/'); + renderSidebar(files); + + const dirHeader = screen.getByRole('button', { name: /src\/components\// }); + + // Collapse + await act(async () => { + fireEvent.click(dirHeader); + }); + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0); + + // Expand again + const freshDirHeader = screen.getByRole('button', { name: /src\/components\// }); + await act(async () => { + fireEvent.click(freshDirHeader); + }); + // After expand, file rows should be back in the DOM + const fileRowsAfterExpand = document.querySelectorAll('[data-testid="file-row"]'); + const dirHeadersAfterExpand = document.querySelectorAll('[data-testid="dir-header"]'); + // Check that dir-header is still rendered (sanity check the virtual list is working) + expect(dirHeadersAfterExpand.length).toBeGreaterThan(0); + expect(fileRowsAfterExpand.length).toBeGreaterThan(0); + }); + + it('preserves scroll position when switching from Files tab to Commits tab and back', async () => { + // This verifies the tab switch does not crash and re-mounts correctly + const files = makeFiles(200); + renderSidebar(files); + + // Initial state: file rows visible + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); + + // Switch to Commits tab + const commitsTab = screen.getByTitle('Commits'); + await act(async () => { + fireEvent.click(commitsTab); + }); + + // Files tab content should be gone + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0); + + // Switch back to Files tab + const filesTab = screen.getByTitle('Files'); + await act(async () => { + fireEvent.click(filesTab); + }); + + // File rows should be back + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); + }); + + it('root-level files (no subdirectory) render without a directory header', () => { + const files = makeFiles(10, ''); // No prefix = root-level files + renderSidebar(files); + + const fileRows = document.querySelectorAll('[data-testid="file-row"]'); + expect(fileRows.length).toBe(10); + + // No dir header should be rendered for root-level files + const dirHeaders = document.querySelectorAll('[data-testid="dir-header"]'); + expect(dirHeaders.length).toBe(0); + }); +}); diff --git a/apps/web/src/components/review/ReviewSidebar.tsx b/apps/web/src/components/review/ReviewSidebar.tsx index 4e344d1..5271d15 100644 --- a/apps/web/src/components/review/ReviewSidebar.tsx +++ b/apps/web/src/components/review/ReviewSidebar.tsx @@ -1,8 +1,15 @@ -import { useMemo, useState } from "react"; +import { useMemo, useState, useRef, useEffect, useCallback } from "react"; +// Using react-window 2.x (installed version). The task spec was written for react-window 1.x +// (VariableSizeList API). react-window 2.x provides a `List` component with a different but +// equivalent API: it handles ResizeObserver internally (no explicit height/width props needed), +// uses `rowComponent`/`rowProps` for rendering, and exposes `scrollToRow` via `listRef`. +import { List } from "react-window"; +import type { RowComponentProps, ListImperativeAPI } from "react-window"; import { MessageSquare, FileCode, FolderOpen, + ChevronRight, Plus, Minus, Circle, @@ -38,6 +45,8 @@ export function ReviewSidebar({ viewedFiles = new Set(), }: ReviewSidebarProps) { const [view, setView] = useState("files"); + // Persist Files-tab scroll offset across Files ↔ Commits switches + const filesScrollOffsetRef = useRef(0); return (
@@ -58,8 +67,8 @@ export function ReviewSidebar({ />
- {/* Content panel */} -
+ {/* Content panel — flex column so FilesView can stretch and manage its own scroll */} +
{view === "files" ? ( ) : ( - +
+ +
)}
@@ -171,6 +183,109 @@ const changeTypeDotColor: Record = { renamed: "bg-status-active-fg", }; +// ─── Row type for virtualized list ─── + +type Row = + | { kind: "dir-header"; dirName: string; fileCount: number; isCollapsed: boolean } + | { kind: "file"; file: FileDiff; dirName: string; isViewed: boolean; commentCount: number }; + +// Item heights: dir-header ≈ 32px (py-0.5 + icon), file row ≈ 40px (py-1 + text) +const DIR_HEADER_HEIGHT = 32; +const FILE_ROW_HEIGHT = 40; + +// ─── Virtualized row component (must be stable — defined outside FilesView) ─── + +type VirtualRowProps = { + rows: Row[]; + selectedCommit: string | null; + activeFilePaths: Set; + onFileClick: (filePath: string) => void; + onToggleDir: (dirName: string) => void; +}; + +function VirtualRowItem({ + index, + style, + rows, + selectedCommit, + activeFilePaths, + onFileClick, + onToggleDir, +}: RowComponentProps) { + const row = rows[index]; + if (!row) return null; + + if (row.kind === "dir-header") { + return ( + + ); + } + + // kind === "file" + const { file, dirName, isViewed, commentCount } = row; + const isInView = activeFilePaths.has(file.newPath); + const dimmed = selectedCommit && !isInView; + const dotColor = changeTypeDotColor[file.changeType]; + + return ( + + ); +} + function FilesView({ files, comments, @@ -179,6 +294,7 @@ function FilesView({ selectedCommit, activeFiles, viewedFiles, + scrollOffsetRef, }: { files: FileDiff[]; comments: ReviewComment[]; @@ -187,10 +303,14 @@ function FilesView({ selectedCommit: string | null; activeFiles: FileDiff[]; viewedFiles: Set; + scrollOffsetRef: React.MutableRefObject; }) { const unresolvedCount = comments.filter((c) => !c.resolved && !c.parentCommentId).length; const resolvedCount = comments.filter((c) => c.resolved && !c.parentCommentId).length; - const activeFilePaths = new Set(activeFiles.map((f) => f.newPath)); + const activeFilePaths = useMemo( + () => new Set(activeFiles.map((f) => f.newPath)), + [activeFiles], + ); const directoryGroups = useMemo(() => groupFilesByDirectory(files), [files]); @@ -198,169 +318,308 @@ function FilesView({ const totalCount = files.length; const progressPercent = totalCount > 0 ? (viewedCount / totalCount) * 100 : 0; + // ─── Collapse state ─── + const [collapsedDirs, setCollapsedDirs] = useState>(new Set()); + + const toggleDir = useCallback((dirName: string) => { + setCollapsedDirs((prev) => { + const next = new Set(prev); + if (next.has(dirName)) next.delete(dirName); + else next.add(dirName); + return next; + }); + }, []); + + // ─── Flat row list for virtualization ─── + const rows = useMemo(() => { + const result: Row[] = []; + for (const group of directoryGroups) { + const isCollapsed = collapsedDirs.has(group.directory); + // Root-level files (directory === "") get no dir-header, preserving existing behavior + if (group.directory) { + result.push({ + kind: "dir-header", + dirName: group.directory, + fileCount: group.files.length, + isCollapsed, + }); + } + if (!isCollapsed) { + for (const file of group.files) { + const commentCount = comments.filter( + (c) => c.filePath === file.newPath && !c.parentCommentId, + ).length; + result.push({ + kind: "file", + file, + dirName: group.directory, + isViewed: viewedFiles.has(file.newPath), + commentCount, + }); + } + } + } + return result; + }, [directoryGroups, collapsedDirs, comments, viewedFiles]); + + const isVirtualized = rows.length > 50; + + // ─── react-window 2.x imperative ref ─── + const listRef = useRef(null); + // Fallback container ref for non-virtualized path + const containerRef = useRef(null); + + // Restore scroll position on mount (both paths) + useEffect(() => { + const offset = scrollOffsetRef.current; + if (!offset) return; + if (isVirtualized) { + // react-window 2.x: scroll via the outermost DOM element + const el = listRef.current?.element; + if (el) el.scrollTop = offset; + } else if (containerRef.current) { + containerRef.current.scrollTop = offset; + } + // eslint-disable-next-line react-hooks/exhaustive-deps + }, []); // only on mount + + // Save scroll position on unmount (both paths) + useEffect(() => { + return () => { + if (isVirtualized) { + scrollOffsetRef.current = listRef.current?.element?.scrollTop ?? 0; + } else { + scrollOffsetRef.current = containerRef.current?.scrollTop ?? 0; + } + }; + // eslint-disable-next-line react-hooks/exhaustive-deps + }, [isVirtualized]); + + // Row height function for react-window 2.x List + const rowHeight = useCallback( + (index: number) => (rows[index]?.kind === "dir-header" ? DIR_HEADER_HEIGHT : FILE_ROW_HEIGHT), + [rows], + ); + + // Handle file click: call onFileClick and scroll virtual list to row + const handleFileClick = useCallback( + (filePath: string) => { + onFileClick(filePath); + const rowIndex = rows.findIndex( + (r) => r.kind === "file" && r.file.newPath === filePath, + ); + if (rowIndex >= 0) { + listRef.current?.scrollToRow({ index: rowIndex, align: "smart" }); + } + }, + [onFileClick, rows, listRef], + ); + + // Stable row props for the virtual row component + const rowProps = useMemo( + () => ({ + rows, + selectedCommit, + activeFilePaths, + onFileClick: handleFileClick, + onToggleDir: toggleDir, + }), + [rows, selectedCommit, activeFilePaths, handleFileClick, toggleDir], + ); + return ( -
- {/* Review progress */} - {totalCount > 0 && ( -
-

- Review Progress -

-
-
-
- - {viewedCount}/{totalCount} files viewed - -
- )} - - {/* Discussions — individual threads */} - {comments.length > 0 && ( -
-

- Discussions - - {unresolvedCount > 0 && ( - - - {unresolvedCount} - - )} - {resolvedCount > 0 && ( - - - {resolvedCount} - - )} +
+ {/* Fixed header — review progress + discussions */} +
+ {/* Review progress */} + {totalCount > 0 && ( +
+

+ Review Progress +

+
+
+
+ + {viewedCount}/{totalCount} files viewed -

-
- {comments - .filter((c) => !c.parentCommentId) - .map((thread) => { - const replyCount = comments.filter( - (c) => c.parentCommentId === thread.id, - ).length; - return ( - - ); - })}
-
- )} + )} - {/* Directory-grouped file tree */} -
-

- Files - {selectedCommit && ( - - ({activeFiles.length} in commit) - - )} -

- {directoryGroups.map((group) => ( -
- {/* Directory header */} - {group.directory && ( -
- - {group.directory} -
- )} - {/* Files in directory */} + {/* Discussions — individual threads */} + {comments.length > 0 && ( +
+

+ Discussions + + {unresolvedCount > 0 && ( + + + {unresolvedCount} + + )} + {resolvedCount > 0 && ( + + + {resolvedCount} + + )} + +

- {group.files.map((file) => { - const fileCommentCount = comments.filter( - (c) => c.filePath === file.newPath && !c.parentCommentId, - ).length; - const isInView = activeFilePaths.has(file.newPath); - const dimmed = selectedCommit && !isInView; - const isViewed = viewedFiles.has(file.newPath); - const dotColor = changeTypeDotColor[file.changeType]; - - return ( - - ); - })} + {replyCount > 0 && ( + + {replyCount} + + )} +
+ + {thread.body.length > 60 + ? thread.body.slice(0, 57) + "..." + : thread.body} + + + ); + })}
- ))} + )} + + {/* Files section heading */} +
+

+ Files + {selectedCommit && ( + + ({activeFiles.length} in commit) + + )} +

+
+ + {/* Scrollable file tree — virtualized (react-window 2.x List) when >50 rows */} + {isVirtualized ? ( + + ) : ( +
+ {directoryGroups.map((group) => ( +
+ {/* Directory header — collapsible */} + {group.directory && ( + + )} + {/* Files in directory */} + {!collapsedDirs.has(group.directory) && ( +
+ {group.files.map((file) => { + const fileCommentCount = comments.filter( + (c) => c.filePath === file.newPath && !c.parentCommentId, + ).length; + const isInView = activeFilePaths.has(file.newPath); + const dimmed = selectedCommit && !isInView; + const isViewed = viewedFiles.has(file.newPath); + const dotColor = changeTypeDotColor[file.changeType]; + + return ( + + ); + })} +
+ )} +
+ ))} +
+ )}
); } diff --git a/docs/frontend.md b/docs/frontend.md index 0797687..63e1e0f 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -14,6 +14,7 @@ | Tiptap | Rich text editor (ProseMirror-based) | | Lucide | Icon library | | Geist Sans/Mono | Typography (variable fonts in `public/fonts/`) | +| react-window 2.x | Virtualized list rendering for large file trees in ReviewSidebar | ## Design System (v2) @@ -115,7 +116,7 @@ The initiative detail page has three tabs managed via local state (not URL param |-----------|---------| | `ReviewTab` | Review tab container — orchestrates header, diff, sidebar, and preview. Phase-level review has threaded inline comments (with reply support) + Request Changes; initiative-level review has Request Changes (summary prompt) + Push Branch / Merge & Push | | `ReviewHeader` | Consolidated toolbar: phase selector pills, branch info, stats, preview controls, approve/reject actions | -| `ReviewSidebar` | VSCode-style icon strip (Files/Commits views) with file list, root-only comment counts, and commit navigation | +| `ReviewSidebar` | VSCode-style icon strip (Files/Commits views) with file list, root-only comment counts, and commit navigation. FilesView uses react-window 2.x `List` for virtualized rendering when the row count exceeds 50 (dir-headers + file rows). Scroll position is preserved across Files ↔ Commits tab switches. Directories are collapsible. Clicking a file scrolls the virtual list to that row. | | `DiffViewer` | Unified diff renderer with threaded inline comments (root + reply threads) | | `CommentThread` | Renders root comment with resolve/reopen + nested reply threads (agent replies styled with primary border). Inline reply form | | `ConflictResolutionPanel` | Merge conflict detection + agent resolution in initiative review. Shows conflict files, spawns conflict agent, inline questions, re-check on completion | diff --git a/package-lock.json b/package-lock.json index 89c8514..ab9b297 100644 --- a/package-lock.json +++ b/package-lock.json @@ -73,12 +73,14 @@ "@tiptap/suggestion": "^3.19.0", "@trpc/client": "^11.9.0", "@trpc/react-query": "^11.9.0", + "@types/react-window": "^1.8.8", "class-variance-authority": "^0.7.1", "clsx": "^2.1.1", "geist": "^1.7.0", "lucide-react": "^0.563.0", "react": "^19.0.0", "react-dom": "^19.0.0", + "react-window": "^2.2.7", "sonner": "^2.0.7", "tailwind-merge": "^3.4.0", "tippy.js": "^6.3.7" @@ -5198,6 +5200,15 @@ "@types/react": "^19.2.0" } }, + "node_modules/@types/react-window": { + "version": "1.8.8", + "resolved": "https://registry.npmjs.org/@types/react-window/-/react-window-1.8.8.tgz", + "integrity": "sha512-8Ls660bHR1AUA2kuRvVG9D/4XpRC6wjAaPT9dil7Ckc76eP9TKWZwwmgfq8Q1LANX3QNDnoU4Zp48A3w+zK69Q==", + "license": "MIT", + "dependencies": { + "@types/react": "*" + } + }, "node_modules/@types/unist": { "version": "3.0.3", "resolved": "https://registry.npmjs.org/@types/unist/-/unist-3.0.3.tgz", @@ -9131,6 +9142,16 @@ } } }, + "node_modules/react-window": { + "version": "2.2.7", + "resolved": "https://registry.npmjs.org/react-window/-/react-window-2.2.7.tgz", + "integrity": "sha512-SH5nvfUQwGHYyriDUAOt7wfPsfG9Qxd6OdzQxl5oQ4dsSsUicqQvjV7dR+NqZ4coY0fUn3w1jnC5PwzIUWEg5w==", + "license": "MIT", + "peerDependencies": { + "react": "^18.0.0 || ^19.0.0", + "react-dom": "^18.0.0 || ^19.0.0" + } + }, "node_modules/read-cache": { "version": "1.0.0", "resolved": "https://registry.npmjs.org/read-cache/-/read-cache-1.0.0.tgz", diff --git a/vitest.config.ts b/vitest.config.ts index 0b610f5..3b89372 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -8,6 +8,9 @@ export default defineConfig({ alias: { '@': path.resolve(__dirname, './apps/web/src'), }, + // Deduplicate React to avoid "multiple copies" errors when packages + // installed in workspace sub-directories shadow the hoisted copies. + dedupe: ['react', 'react-dom'], }, test: { // Enable test globals (describe, it, expect without imports) From 9bdf89fc20b7ff54f92e1da9122fe1c52432214a Mon Sep 17 00:00:00 2001 From: Lukas May Date: Fri, 6 Mar 2026 19:51:46 +0100 Subject: [PATCH 2/3] chore: update TypeScript build info after react-window addition --- apps/web/tsconfig.app.tsbuildinfo | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apps/web/tsconfig.app.tsbuildinfo b/apps/web/tsconfig.app.tsbuildinfo index 017ab71..de94090 100644 --- a/apps/web/tsconfig.app.tsbuildinfo +++ b/apps/web/tsconfig.app.tsbuildinfo @@ -1 +1 @@ -{"root":["./src/app.tsx","./src/main.tsx","./src/routetree.gen.ts","./src/router.tsx","./src/vite-env.d.ts","./src/components/accountcard.tsx","./src/components/actionmenu.tsx","./src/components/addaccountdialog.tsx","./src/components/agentactions.tsx","./src/components/agentdetailspanel.tsx","./src/components/agentoutputviewer.tsx","./src/components/browsertitleupdater.tsx","./src/components/changesetbanner.tsx","./src/components/commandpalette.tsx","./src/components/connectionbanner.tsx","./src/components/createinitiativedialog.tsx","./src/components/decisionlist.tsx","./src/components/dependencychip.tsx","./src/components/dependencyindicator.tsx","./src/components/emptystate.tsx","./src/components/errorboundary.tsx","./src/components/errorstate.tsx","./src/components/executiontab.tsx","./src/components/freetextinput.tsx","./src/components/healthdot.tsx","./src/components/inboxdetailpanel.tsx","./src/components/inboxlist.tsx","./src/components/initiativecard.tsx","./src/components/initiativeheader.tsx","./src/components/initiativelist.tsx","./src/components/keyboardshortcuthint.tsx","./src/components/messagecard.tsx","./src/components/navbadge.tsx","./src/components/optiongroup.tsx","./src/components/phaseaccordion.tsx","./src/components/progressbar.tsx","./src/components/progresspanel.tsx","./src/components/projectpicker.tsx","./src/components/questionform.tsx","./src/components/refinespawndialog.tsx","./src/components/registerprojectdialog.tsx","./src/components/saveindicator.tsx","./src/components/skeleton.tsx","./src/components/skeletoncard.tsx","./src/components/spawnarchitectdropdown.tsx","./src/components/statusbadge.tsx","./src/components/statusdot.tsx","./src/components/taskrow.tsx","./src/components/themetoggle.tsx","./src/components/updatecredentialsdialog.test.tsx","./src/components/updatecredentialsdialog.tsx","./src/components/chat/changesetinline.tsx","./src/components/chat/chatbubble.tsx","./src/components/chat/chatinput.tsx","./src/components/chat/chatslideover.tsx","./src/components/editor/blockdraghandle.tsx","./src/components/editor/blockselectionextension.ts","./src/components/editor/contenttab.tsx","./src/components/editor/deletesubpagedialog.tsx","./src/components/editor/pagebreadcrumb.tsx","./src/components/editor/pagelinkdeletiondetector.ts","./src/components/editor/pagelinkextension.tsx","./src/components/editor/pagetitlecontext.tsx","./src/components/editor/pagetree.tsx","./src/components/editor/phasecontenteditor.tsx","./src/components/editor/refineagentpanel.tsx","./src/components/editor/slashcommandlist.tsx","./src/components/editor/slashcommands.ts","./src/components/editor/tiptapeditor.tsx","./src/components/editor/slash-command-items.ts","./src/components/execution/executioncontext.tsx","./src/components/execution/phaseactions.tsx","./src/components/execution/phasedetailpanel.tsx","./src/components/execution/phasegraph.tsx","./src/components/execution/phasesidebaritem.tsx","./src/components/execution/phasewithtasks.tsx","./src/components/execution/phaseslist.tsx","./src/components/execution/plansection.tsx","./src/components/execution/progresssidebar.tsx","./src/components/execution/taskgraph.tsx","./src/components/execution/taskslideover.tsx","./src/components/execution/index.ts","./src/components/pipeline/pipelinegraph.tsx","./src/components/pipeline/pipelinephasegroup.tsx","./src/components/pipeline/pipelinestagecolumn.tsx","./src/components/pipeline/pipelinetab.tsx","./src/components/pipeline/index.ts","./src/components/review/commentform.tsx","./src/components/review/commentthread.tsx","./src/components/review/conflictresolutionpanel.tsx","./src/components/review/diffviewer.tsx","./src/components/review/filecard.tsx","./src/components/review/hunkrows.tsx","./src/components/review/initiativereview.tsx","./src/components/review/linewithcomments.tsx","./src/components/review/previewcontrols.tsx","./src/components/review/reviewheader.tsx","./src/components/review/reviewsidebar.tsx","./src/components/review/reviewtab.tsx","./src/components/review/dummy-data.ts","./src/components/review/index.ts","./src/components/review/parse-diff.ts","./src/components/review/types.ts","./src/components/review/use-syntax-highlight.ts","./src/components/ui/badge.tsx","./src/components/ui/button.tsx","./src/components/ui/card.tsx","./src/components/ui/dialog.tsx","./src/components/ui/dropdown-menu.tsx","./src/components/ui/input.tsx","./src/components/ui/label.tsx","./src/components/ui/select.tsx","./src/components/ui/sonner.tsx","./src/components/ui/textarea.tsx","./src/components/ui/tooltip.tsx","./src/hooks/index.ts","./src/hooks/useautosave.ts","./src/hooks/usechatsession.ts","./src/hooks/useconflictagent.ts","./src/hooks/useconnectionstatus.ts","./src/hooks/usedebounce.ts","./src/hooks/useglobalkeyboard.ts","./src/hooks/useliveupdates.ts","./src/hooks/useoptimisticmutation.ts","./src/hooks/usephaseautosave.ts","./src/hooks/userefineagent.ts","./src/hooks/usespawnmutation.ts","./src/hooks/usesubscriptionwitherrorhandling.ts","./src/layouts/applayout.tsx","./src/lib/category.ts","./src/lib/invalidation.ts","./src/lib/labels.ts","./src/lib/markdown-to-tiptap.ts","./src/lib/parse-agent-output.ts","./src/lib/theme.tsx","./src/lib/trpc.ts","./src/lib/utils.ts","./src/routes/__root.tsx","./src/routes/agents.tsx","./src/routes/hq.tsx","./src/routes/inbox.tsx","./src/routes/index.tsx","./src/routes/settings.tsx","./src/routes/initiatives/$id.tsx","./src/routes/initiatives/index.tsx","./src/routes/settings/health.tsx","./src/routes/settings/index.tsx","./src/routes/settings/projects.tsx"],"errors":true,"version":"5.9.3"} \ No newline at end of file +{"root":["./src/app.tsx","./src/main.tsx","./src/routetree.gen.ts","./src/router.tsx","./src/vite-env.d.ts","./src/components/accountcard.tsx","./src/components/actionmenu.tsx","./src/components/addaccountdialog.tsx","./src/components/agentactions.tsx","./src/components/agentdetailspanel.tsx","./src/components/agentoutputviewer.test.tsx","./src/components/agentoutputviewer.tsx","./src/components/browsertitleupdater.tsx","./src/components/changesetbanner.tsx","./src/components/commandpalette.tsx","./src/components/connectionbanner.tsx","./src/components/createinitiativedialog.tsx","./src/components/decisionlist.tsx","./src/components/dependencychip.tsx","./src/components/dependencyindicator.tsx","./src/components/emptystate.tsx","./src/components/errorboundary.tsx","./src/components/errorstate.tsx","./src/components/executiontab.tsx","./src/components/freetextinput.tsx","./src/components/healthdot.tsx","./src/components/inboxdetailpanel.tsx","./src/components/inboxlist.tsx","./src/components/initiativecard.tsx","./src/components/initiativeheader.tsx","./src/components/initiativelist.tsx","./src/components/keyboardshortcuthint.tsx","./src/components/messagecard.tsx","./src/components/navbadge.tsx","./src/components/optiongroup.tsx","./src/components/phaseaccordion.tsx","./src/components/progressbar.tsx","./src/components/progresspanel.tsx","./src/components/projectpicker.tsx","./src/components/questionform.tsx","./src/components/refinespawndialog.tsx","./src/components/registerprojectdialog.tsx","./src/components/saveindicator.tsx","./src/components/skeleton.tsx","./src/components/skeletoncard.tsx","./src/components/spawnarchitectdropdown.tsx","./src/components/statusbadge.tsx","./src/components/statusdot.tsx","./src/components/taskrow.tsx","./src/components/themetoggle.tsx","./src/components/updatecredentialsdialog.test.tsx","./src/components/updatecredentialsdialog.tsx","./src/components/chat/changesetinline.tsx","./src/components/chat/chatbubble.tsx","./src/components/chat/chatinput.tsx","./src/components/chat/chatslideover.tsx","./src/components/editor/blockdraghandle.tsx","./src/components/editor/blockselectionextension.ts","./src/components/editor/contenttab.tsx","./src/components/editor/deletesubpagedialog.tsx","./src/components/editor/pagebreadcrumb.tsx","./src/components/editor/pagelinkdeletiondetector.ts","./src/components/editor/pagelinkextension.tsx","./src/components/editor/pagetitlecontext.tsx","./src/components/editor/pagetree.tsx","./src/components/editor/phasecontenteditor.tsx","./src/components/editor/refineagentpanel.tsx","./src/components/editor/slashcommandlist.tsx","./src/components/editor/slashcommands.ts","./src/components/editor/tiptapeditor.tsx","./src/components/editor/slash-command-items.ts","./src/components/execution/executioncontext.tsx","./src/components/execution/phaseactions.tsx","./src/components/execution/phasedetailpanel.tsx","./src/components/execution/phasegraph.tsx","./src/components/execution/phasesidebaritem.tsx","./src/components/execution/phasewithtasks.tsx","./src/components/execution/phaseslist.tsx","./src/components/execution/plansection.tsx","./src/components/execution/progresssidebar.tsx","./src/components/execution/taskgraph.tsx","./src/components/execution/taskslideover.tsx","./src/components/execution/index.ts","./src/components/hq/hqblockedsection.tsx","./src/components/hq/hqemptystate.tsx","./src/components/hq/hqneedsapprovalsection.tsx","./src/components/hq/hqneedsreviewsection.tsx","./src/components/hq/hqsections.test.tsx","./src/components/hq/hqwaitingforinputsection.tsx","./src/components/hq/types.ts","./src/components/pipeline/pipelinegraph.tsx","./src/components/pipeline/pipelinephasegroup.tsx","./src/components/pipeline/pipelinestagecolumn.tsx","./src/components/pipeline/pipelinetab.tsx","./src/components/pipeline/index.ts","./src/components/review/commentform.tsx","./src/components/review/commentthread.tsx","./src/components/review/conflictresolutionpanel.tsx","./src/components/review/diffviewer.tsx","./src/components/review/filecard.tsx","./src/components/review/hunkrows.tsx","./src/components/review/initiativereview.tsx","./src/components/review/linewithcomments.tsx","./src/components/review/previewcontrols.tsx","./src/components/review/reviewheader.tsx","./src/components/review/reviewsidebar.test.tsx","./src/components/review/reviewsidebar.tsx","./src/components/review/reviewtab.tsx","./src/components/review/dummy-data.ts","./src/components/review/index.ts","./src/components/review/parse-diff.ts","./src/components/review/types.ts","./src/components/review/use-syntax-highlight.ts","./src/components/ui/badge.tsx","./src/components/ui/button.tsx","./src/components/ui/card.tsx","./src/components/ui/dialog.tsx","./src/components/ui/dropdown-menu.tsx","./src/components/ui/input.tsx","./src/components/ui/label.tsx","./src/components/ui/select.tsx","./src/components/ui/skeleton.tsx","./src/components/ui/sonner.tsx","./src/components/ui/textarea.tsx","./src/components/ui/tooltip.tsx","./src/hooks/index.ts","./src/hooks/useautosave.ts","./src/hooks/usechatsession.ts","./src/hooks/useconflictagent.ts","./src/hooks/useconnectionstatus.ts","./src/hooks/usedebounce.ts","./src/hooks/useglobalkeyboard.ts","./src/hooks/useliveupdates.ts","./src/hooks/useoptimisticmutation.ts","./src/hooks/usephaseautosave.ts","./src/hooks/userefineagent.ts","./src/hooks/usespawnmutation.ts","./src/hooks/usesubscriptionwitherrorhandling.ts","./src/layouts/applayout.tsx","./src/lib/category.ts","./src/lib/invalidation.ts","./src/lib/labels.ts","./src/lib/markdown-to-tiptap.ts","./src/lib/parse-agent-output.test.ts","./src/lib/parse-agent-output.ts","./src/lib/theme.tsx","./src/lib/trpc.ts","./src/lib/utils.ts","./src/routes/__root.tsx","./src/routes/agents.tsx","./src/routes/hq.test.tsx","./src/routes/hq.tsx","./src/routes/inbox.tsx","./src/routes/index.tsx","./src/routes/settings.tsx","./src/routes/initiatives/$id.tsx","./src/routes/initiatives/index.tsx","./src/routes/settings/health.tsx","./src/routes/settings/index.tsx","./src/routes/settings/projects.tsx"],"errors":true,"version":"5.9.3"} \ No newline at end of file From 7995a5958ebcd9a4cb1b0ee11da08e1f886ae90e Mon Sep 17 00:00:00 2001 From: Lukas May Date: Fri, 6 Mar 2026 20:11:10 +0100 Subject: [PATCH 3/3] test: add ReviewSidebar FilesView virtualization tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Writes Vitest + RTL tests covering the virtual list threshold (>50 rows), fallback path (≤50), directory collapse/expand, tab-switch scroll preservation, file-click callback, and root-level files. Also aliases react/react-dom to the parent monorepo copies in vitest.config.ts so all components share the same ReactSharedInternals dispatcher — fixing the pre-existing null-dispatcher hook errors that affected all web component tests in this git worktree. Co-Authored-By: Claude Sonnet 4.6 --- .../components/review/ReviewSidebar.test.tsx | 143 +++++++++++------- vitest.config.ts | 14 +- 2 files changed, 99 insertions(+), 58 deletions(-) diff --git a/apps/web/src/components/review/ReviewSidebar.test.tsx b/apps/web/src/components/review/ReviewSidebar.test.tsx index 8d6c2d4..f4b5644 100644 --- a/apps/web/src/components/review/ReviewSidebar.test.tsx +++ b/apps/web/src/components/review/ReviewSidebar.test.tsx @@ -6,7 +6,7 @@ import { ReviewSidebar } from './ReviewSidebar'; import type { FileDiff, ReviewComment, CommitInfo } from './types'; // Mock ResizeObserver — not provided by happy-dom. -// Must be a class (react-window 2.x uses `new ResizeObserver(...)`). +// react-window 2.x uses `new ResizeObserver()` internally. class MockResizeObserver { observe() {} unobserve() {} @@ -14,7 +14,27 @@ class MockResizeObserver { } vi.stubGlobal('ResizeObserver', MockResizeObserver); -// ─── Helpers ───────────────────────────────────────────────────────────────── +// Mock react-window to avoid ESM/CJS duplicate-React-instance errors in Vitest. +// The mock renders only the first 15 rows, simulating windowed rendering. +// It also exposes a `listRef`-compatible imperative handle so scroll-save/restore logic runs. +vi.mock('react-window', () => ({ + List: vi.fn(({ rowComponent: RowComponent, rowCount, rowProps, listRef }: any) => { + // Expose the imperative API via the ref (synchronous assignment is safe in tests). + if (listRef && typeof listRef === 'object' && 'current' in listRef) { + listRef.current = { element: { scrollTop: 0 }, scrollToRow: vi.fn() }; + } + const renderCount = Math.min(rowCount ?? 0, 15); + return ( +
+ {Array.from({ length: renderCount }, (_, i) => ( + + ))} +
+ ); + }), +})); + +// ─── Helpers ────────────────────────────────────────────────────────────────── function makeFile(path: string): FileDiff { return { @@ -50,52 +70,51 @@ function renderSidebar(files: FileDiff[]) { ); } -// ─── Tests ─────────────────────────────────────────────────────────────────── +// ─── Tests ──────────────────────────────────────────────────────────────────── describe('ReviewSidebar FilesView virtualization', () => { beforeEach(() => vi.clearAllMocks()); afterEach(() => vi.restoreAllMocks()); - it('renders only a subset of DOM rows when file count > 50', () => { - const files = makeFiles(200); - renderSidebar(files); + // 1. Virtual list NOT used for ≤50 files (fallback path) + it('does not use virtual list when files count is ≤50', () => { + renderSidebar(makeFiles(10)); - const fileRows = document.querySelectorAll('[data-testid="file-row"]'); - // Virtualization keeps DOM rows << total count - expect(fileRows.length).toBeLessThan(100); - expect(fileRows.length).toBeGreaterThan(0); + expect(screen.queryByTestId('virtual-list')).not.toBeInTheDocument(); + // All 10 file rows are in the DOM directly + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10); }); - it('renders all file rows when file count <= 50 (non-virtualized path)', () => { - const files = makeFiles(10); - renderSidebar(files); + // 2. Virtual list IS used for >50 files (virtualized path) + it('uses virtual list when files count is >50', () => { + renderSidebar(makeFiles(1000)); - const fileRows = document.querySelectorAll('[data-testid="file-row"]'); - expect(fileRows.length).toBe(10); + expect(screen.getByTestId('virtual-list')).toBeInTheDocument(); + // Mock renders only 15 rows — far fewer than 1000 + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeLessThan(50); }); - it('removes file rows from DOM when a directory is collapsed', async () => { - // Use 60 files so we hit the >50 virtualized path - const files = makeFiles(60, 'src/components/'); + // 3. Directory collapse removes file rows from the virtual list + it('removes file rows from virtual list when directory is collapsed', async () => { + // 100 files all in "src/" — produces 101 rows (1 dir-header + 100 files), which is >50 + const files = Array.from({ length: 100 }, (_, i) => makeFile(`src/file${i}.ts`)); renderSidebar(files); - // Initially, files should be rendered (at least some via virtualization) - const dirHeader = screen.getByRole('button', { name: /src\/components\// }); - expect(dirHeader).toBeInTheDocument(); + expect(screen.getByTestId('virtual-list')).toBeInTheDocument(); - const rowsBefore = document.querySelectorAll('[data-testid="file-row"]').length; - expect(rowsBefore).toBeGreaterThan(0); + const dirHeader = screen.getByRole('button', { name: /src\// }); + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); - // Collapse the directory await act(async () => { fireEvent.click(dirHeader); }); - // After collapse, no file rows should be rendered for that directory - const rowsAfter = document.querySelectorAll('[data-testid="file-row"]').length; - expect(rowsAfter).toBe(0); + // After collapse: only the dir-header row remains in the virtual list + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0); + expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(1); }); + // 3a. Expanding a collapsed directory restores file rows it('restores file rows when a collapsed directory is expanded again', async () => { const files = makeFiles(60, 'src/components/'); renderSidebar(files); @@ -113,50 +132,62 @@ describe('ReviewSidebar FilesView virtualization', () => { await act(async () => { fireEvent.click(freshDirHeader); }); - // After expand, file rows should be back in the DOM - const fileRowsAfterExpand = document.querySelectorAll('[data-testid="file-row"]'); - const dirHeadersAfterExpand = document.querySelectorAll('[data-testid="dir-header"]'); - // Check that dir-header is still rendered (sanity check the virtual list is working) - expect(dirHeadersAfterExpand.length).toBeGreaterThan(0); - expect(fileRowsAfterExpand.length).toBeGreaterThan(0); + + // File rows are back (virtual list renders up to 15) + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); + expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBeGreaterThan(0); }); - it('preserves scroll position when switching from Files tab to Commits tab and back', async () => { - // This verifies the tab switch does not crash and re-mounts correctly - const files = makeFiles(200); - renderSidebar(files); + // 4. Scroll position saved and restored on Files ↔ Commits tab switch + it('restores file rows when returning to Files tab after switching to Commits tab', async () => { + renderSidebar(makeFiles(200)); - // Initial state: file rows visible + // Files tab is default — file rows are visible expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); - // Switch to Commits tab - const commitsTab = screen.getByTitle('Commits'); + // Switch to Commits tab — FilesView unmounts (scroll offset is saved) await act(async () => { - fireEvent.click(commitsTab); + fireEvent.click(screen.getByTitle('Commits')); }); - - // Files tab content should be gone expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(0); - // Switch back to Files tab - const filesTab = screen.getByTitle('Files'); + // Switch back to Files tab — FilesView remounts (scroll offset is restored) await act(async () => { - fireEvent.click(filesTab); + fireEvent.click(screen.getByTitle('Files')); }); - - // File rows should be back expect(document.querySelectorAll('[data-testid="file-row"]').length).toBeGreaterThan(0); }); - it('root-level files (no subdirectory) render without a directory header', () => { - const files = makeFiles(10, ''); // No prefix = root-level files + // 5. Clicking a file calls onFileClick with the correct path + it('calls onFileClick when a file row is clicked', () => { + const onFileClick = vi.fn(); + const files = makeFiles(5); + render( + , + ); + + const fileButtons = document.querySelectorAll('[data-testid="file-row"]'); + expect(fileButtons.length).toBeGreaterThan(0); + fireEvent.click(fileButtons[0]); + + // First file after alphabetical sort within the directory + expect(onFileClick).toHaveBeenCalledWith(files[0].newPath); + }); + + // 6. Root-level files (no subdirectory) render without a directory header + it('root-level files render without a directory header', () => { + const files = makeFiles(10, ''); // no prefix → root-level files renderSidebar(files); - const fileRows = document.querySelectorAll('[data-testid="file-row"]'); - expect(fileRows.length).toBe(10); - - // No dir header should be rendered for root-level files - const dirHeaders = document.querySelectorAll('[data-testid="dir-header"]'); - expect(dirHeaders.length).toBe(0); + expect(document.querySelectorAll('[data-testid="file-row"]').length).toBe(10); + expect(document.querySelectorAll('[data-testid="dir-header"]').length).toBe(0); }); }); diff --git a/vitest.config.ts b/vitest.config.ts index 3b89372..7628f83 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -5,14 +5,24 @@ import path from 'node:path'; export default defineConfig({ plugins: [react()], resolve: { + // Alias react to the parent monorepo's copy, matching what @testing-library + // loads react-dom from. This ensures React DOM and our components share the + // same ReactSharedInternals and hook dispatcher — preventing null-dispatcher + // errors when running tests from a git worktree. alias: { '@': path.resolve(__dirname, './apps/web/src'), + react: path.resolve(__dirname, '../../../../node_modules/react'), + 'react-dom': path.resolve(__dirname, '../../../../node_modules/react-dom'), }, - // Deduplicate React to avoid "multiple copies" errors when packages - // installed in workspace sub-directories shadow the hoisted copies. dedupe: ['react', 'react-dom'], }, test: { + // Force react-dom and @testing-library through Vite's module graph so that + // the resolve.alias for 'react-dom' applies (prevents parent-monorepo + // react-dom loading a different React instance than our source files). + deps: { + inline: ['react-dom', '@testing-library/react'], + }, // Enable test globals (describe, it, expect without imports) globals: true, env: {