diff --git a/apps/web/src/components/review/DiffViewer.tsx b/apps/web/src/components/review/DiffViewer.tsx index 5e9c536..1387393 100644 --- a/apps/web/src/components/review/DiffViewer.tsx +++ b/apps/web/src/components/review/DiffViewer.tsx @@ -162,6 +162,7 @@ export function DiffViewer({ // Suppress unused variable warning — used only to force re-render on visibility change void visibilityVersion; + void queryClient; // imported for type alignment; actual prefetch goes through trpc utils const isSingleFile = files.length === 1; diff --git a/apps/web/src/components/review/ReviewHeader.tsx b/apps/web/src/components/review/ReviewHeader.tsx index 44e55fc..44cc260 100644 --- a/apps/web/src/components/review/ReviewHeader.tsx +++ b/apps/web/src/components/review/ReviewHeader.tsx @@ -42,6 +42,9 @@ interface ReviewHeaderProps { preview: PreviewState | null; viewedCount?: number; totalCount?: number; + totalAdditions?: number; + totalDeletions?: number; + onExpandAll?: () => void; } export function ReviewHeader({ @@ -62,9 +65,12 @@ export function ReviewHeader({ preview, viewedCount, totalCount, + totalAdditions: totalAdditionsProp, + totalDeletions: totalDeletionsProp, + onExpandAll, }: ReviewHeaderProps) { - const totalAdditions = files.reduce((s, f) => s + f.additions, 0); - const totalDeletions = files.reduce((s, f) => s + f.deletions, 0); + const totalAdditions = totalAdditionsProp ?? files.reduce((s, f) => s + f.additions, 0); + const totalDeletions = totalDeletionsProp ?? files.reduce((s, f) => s + f.deletions, 0); const [showConfirmation, setShowConfirmation] = useState(false); const [showRequestConfirm, setShowRequestConfirm] = useState(false); const confirmRef = useRef(null); @@ -186,6 +192,16 @@ export function ReviewHeader({ {/* Right: preview + actions */}
+ {onExpandAll && ( + + )} {/* Preview controls */} {preview && } diff --git a/apps/web/src/components/review/ReviewTab.test.tsx b/apps/web/src/components/review/ReviewTab.test.tsx new file mode 100644 index 0000000..c7c770a --- /dev/null +++ b/apps/web/src/components/review/ReviewTab.test.tsx @@ -0,0 +1,226 @@ +// @vitest-environment happy-dom +import "@testing-library/jest-dom/vitest"; +import { render, screen, act } from "@testing-library/react"; +import { vi, describe, it, expect, beforeEach } from "vitest"; + +// ── Capture props passed to stubs ───────────────────────────────────────────── +// These are module-level so the vi.mock factories (which are hoisted) can close over them. +let diffViewerProps: Record = {}; +let reviewSidebarProps: Record = {}; + +vi.mock("./DiffViewer", () => ({ + DiffViewer: (props: Record) => { + diffViewerProps = props; + return
; + }, +})); + +vi.mock("./ReviewSidebar", () => ({ + ReviewSidebar: (props: Record) => { + reviewSidebarProps = props; + return
; + }, +})); + +vi.mock("./ReviewHeader", () => ({ + ReviewHeader: (props: Record) => ( +
+ {props.onExpandAll && ( + + )} +
+ ), +})); + +vi.mock("./InitiativeReview", () => ({ + InitiativeReview: () =>
, +})); + +vi.mock("./comment-index", () => ({ + buildCommentIndex: vi.fn(() => new Map()), +})); + +vi.mock("sonner", () => ({ + toast: { success: vi.fn(), error: vi.fn() }, +})); + +// ── parseUnifiedDiff spy ─────────────────────────────────────────────────────── +const mockParseUnifiedDiff = vi.fn((_raw: string) => [ + { + oldPath: "a.ts", + newPath: "a.ts", + status: "modified" as const, + additions: 3, + deletions: 1, + hunks: [], + }, +]); + +vi.mock("./parse-diff", () => ({ + get parseUnifiedDiff() { + return mockParseUnifiedDiff; + }, +})); + +// ── tRPC mock factory ───────────────────────────────────────────────────────── + +const noopMutation = () => ({ + mutate: vi.fn(), + isPending: false, +}); + +const noopQuery = (data: unknown = undefined) => ({ + data, + isLoading: false, + isError: false, + refetch: vi.fn(), +}); + +const mockUtils = { + listReviewComments: { invalidate: vi.fn() }, +}; + +// Server format (FileStatEntry): uses `path` not `newPath` +const PHASE_FILES = [ + { + path: "a.ts", + status: "modified" as const, + additions: 5, + deletions: 2, + }, +]; + +// trpcMock is a let so tests can override it. The getter in the mock reads the current value. +let trpcMock = buildTrpcMock(); + +function buildTrpcMock(overrides: Record = {}) { + return { + getInitiative: { useQuery: vi.fn(() => noopQuery({ status: "in_progress" })) }, + listPhases: { + useQuery: vi.fn(() => + noopQuery([{ id: "phase-1", name: "Phase 1", status: "pending_review" }]) + ), + }, + getInitiativeProjects: { useQuery: vi.fn(() => noopQuery([{ id: "proj-1" }])) }, + getPhaseReviewDiff: { + useQuery: vi.fn(() => + noopQuery({ + phaseName: "Phase 1", + sourceBranch: "cw/phase-1", + targetBranch: "main", + files: PHASE_FILES, + totalAdditions: 5, + totalDeletions: 2, + }) + ), + }, + getPhaseReviewCommits: { + useQuery: vi.fn(() => + noopQuery({ commits: [], sourceBranch: "cw/phase-1", targetBranch: "main" }) + ), + }, + getCommitDiff: { + useQuery: vi.fn(() => noopQuery({ rawDiff: "" })), + }, + listPreviews: { useQuery: vi.fn(() => noopQuery([])) }, + getPreviewStatus: { useQuery: vi.fn(() => noopQuery(null)) }, + listReviewComments: { useQuery: vi.fn(() => noopQuery([])) }, + startPreview: { useMutation: vi.fn(() => noopMutation()) }, + stopPreview: { useMutation: vi.fn(() => noopMutation()) }, + createReviewComment: { useMutation: vi.fn(() => noopMutation()) }, + resolveReviewComment: { useMutation: vi.fn(() => noopMutation()) }, + unresolveReviewComment: { useMutation: vi.fn(() => noopMutation()) }, + replyToReviewComment: { useMutation: vi.fn(() => noopMutation()) }, + updateReviewComment: { useMutation: vi.fn(() => noopMutation()) }, + approvePhaseReview: { useMutation: vi.fn(() => noopMutation()) }, + requestPhaseChanges: { useMutation: vi.fn(() => noopMutation()) }, + useUtils: vi.fn(() => mockUtils), + ...overrides, + }; +} + +vi.mock("@/lib/trpc", () => ({ + get trpc() { + return trpcMock; + }, +})); + +// ── Import component after mocks ────────────────────────────────────────────── +import { ReviewTab } from "./ReviewTab"; + +// ── Tests ───────────────────────────────────────────────────────────────────── + +describe("ReviewTab", () => { + beforeEach(() => { + diffViewerProps = {}; + reviewSidebarProps = {}; + mockParseUnifiedDiff.mockClear(); + trpcMock = buildTrpcMock(); + }); + + it("1. phase diff loads metadata: DiffViewer receives files array and commitMode=false", () => { + render(); + + expect(screen.getByTestId("diff-viewer")).toBeInTheDocument(); + const files = diffViewerProps.files as unknown[]; + expect(files).toHaveLength(1); + expect(diffViewerProps.commitMode).toBe(false); + }); + + it("2. no rawDiff parsing in phase mode: parseUnifiedDiff is NOT called", () => { + render(); + + expect(mockParseUnifiedDiff).not.toHaveBeenCalled(); + }); + + it("3. commit view parses rawDiff: parseUnifiedDiff called and DiffViewer gets commitMode=true", async () => { + trpcMock = buildTrpcMock({ + getCommitDiff: { + useQuery: vi.fn(() => + noopQuery({ rawDiff: "diff --git a/a.ts b/a.ts\nindex 000..111 100644\n--- a/a.ts\n+++ b/a.ts\n@@ -1,1 +1,1 @@\n-old\n+new\n" }) + ), + }, + }); + + render(); + + // Select a commit via the sidebar stub's onSelectCommit prop + const { onSelectCommit } = reviewSidebarProps as { + onSelectCommit: (hash: string | null) => void; + }; + + await act(async () => { + onSelectCommit("abc123"); + }); + + expect(diffViewerProps.commitMode).toBe(true); + expect(mockParseUnifiedDiff).toHaveBeenCalled(); + }); + + it("4. allFiles uses metadata for sidebar: ReviewSidebar receives files from diffQuery.data.files", () => { + render(); + + const sidebarFiles = reviewSidebarProps.files as Array<{ newPath: string }>; + expect(sidebarFiles).toHaveLength(1); + expect(sidebarFiles[0].newPath).toBe("a.ts"); + }); + + it("5. expandAll prop passed: clicking Expand all button causes DiffViewer to receive expandAll=true", async () => { + render(); + + // Before clicking, expandAll should be false + expect(diffViewerProps.expandAll).toBe(false); + + const expandBtn = screen.getByTestId("expand-all-btn"); + await act(async () => { + expandBtn.click(); + }); + + expect(diffViewerProps.expandAll).toBe(true); + }); +}); diff --git a/apps/web/src/components/review/ReviewTab.tsx b/apps/web/src/components/review/ReviewTab.tsx index df91c58..4e6a2b9 100644 --- a/apps/web/src/components/review/ReviewTab.tsx +++ b/apps/web/src/components/review/ReviewTab.tsx @@ -8,7 +8,7 @@ import { ReviewSidebar } from "./ReviewSidebar"; import { ReviewHeader } from "./ReviewHeader"; import { InitiativeReview } from "./InitiativeReview"; import { buildCommentIndex } from "./comment-index"; -import type { ReviewStatus, DiffLine } from "./types"; +import type { ReviewStatus, DiffLine, FileDiff, FileDiffDetail } from "./types"; interface ReviewTabProps { initiativeId: string; @@ -18,6 +18,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { const [status, setStatus] = useState("pending"); const [selectedCommit, setSelectedCommit] = useState(null); const [viewedFiles, setViewedFiles] = useState>(new Set()); + const [expandAll, setExpandAll] = useState(false); const fileRefs = useRef>(new Map()); const headerRef = useRef(null); const [headerHeight, setHeaderHeight] = useState(0); @@ -74,7 +75,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { const projectsQuery = trpc.getInitiativeProjects.useQuery({ initiativeId }); const firstProjectId = projectsQuery.data?.[0]?.id ?? null; - // Fetch full branch diff for active phase + // Fetch full branch diff for active phase (metadata only, no rawDiff) const diffQuery = trpc.getPhaseReviewDiff.useQuery( { phaseId: activePhaseId! }, { enabled: !!activePhaseId && !isInitiativePendingReview }, @@ -96,7 +97,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { // Preview state const previewsQuery = trpc.listPreviews.useQuery({ initiativeId }); const existingPreview = previewsQuery.data?.find( - (p) => p.phaseId === activePhaseId || p.initiativeId === initiativeId, + (p: { phaseId?: string; initiativeId?: string }) => p.phaseId === activePhaseId || p.initiativeId === initiativeId, ); const [activePreviewId, setActivePreviewId] = useState(null); const previewStatusQuery = trpc.getPreviewStatus.useQuery( @@ -107,12 +108,12 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { const sourceBranch = diffQuery.data?.sourceBranch ?? commitsQuery.data?.sourceBranch ?? ""; const startPreview = trpc.startPreview.useMutation({ - onSuccess: (data) => { + onSuccess: (data: { id: string; url: string }) => { setActivePreviewId(data.id); previewsQuery.refetch(); toast.success(`Preview running at ${data.url}`); }, - onError: (err) => toast.error(`Preview failed: ${err.message}`), + onError: (err: { message: string }) => toast.error(`Preview failed: ${err.message}`), }); const stopPreview = trpc.stopPreview.useMutation({ @@ -121,7 +122,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { toast.success("Preview stopped"); previewsQuery.refetch(); }, - onError: (err) => toast.error(`Failed to stop: ${err.message}`), + onError: (err: { message: string }) => toast.error(`Failed to stop: ${err.message}`), }); const previewState = firstProjectId && sourceBranch @@ -157,7 +158,17 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { { enabled: !!activePhaseId && !isInitiativePendingReview }, ); const comments = useMemo(() => { - return (commentsQuery.data ?? []).map((c) => ({ + return (commentsQuery.data ?? []).map((c: { + id: string; + filePath: string; + lineNumber: number | null; + lineType: string; + body: string; + author: string; + createdAt: string | number; + resolved: boolean; + parentCommentId?: string | null; + }) => ({ id: c.id, filePath: c.filePath, lineNumber: c.lineNumber, @@ -179,7 +190,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { onSuccess: () => { utils.listReviewComments.invalidate({ phaseId: activePhaseId! }); }, - onError: (err) => toast.error(`Failed to save comment: ${err.message}`), + onError: (err: { message: string }) => toast.error(`Failed to save comment: ${err.message}`), }); const resolveCommentMutation = trpc.resolveReviewComment.useMutation({ @@ -198,14 +209,14 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { onSuccess: () => { utils.listReviewComments.invalidate({ phaseId: activePhaseId! }); }, - onError: (err) => toast.error(`Failed to post reply: ${err.message}`), + onError: (err: { message: string }) => toast.error(`Failed to post reply: ${err.message}`), }); const editCommentMutation = trpc.updateReviewComment.useMutation({ onSuccess: () => { utils.listReviewComments.invalidate({ phaseId: activePhaseId! }); }, - onError: (err) => toast.error(`Failed to update comment: ${err.message}`), + onError: (err: { message: string }) => toast.error(`Failed to update comment: ${err.message}`), }); const approveMutation = trpc.approvePhaseReview.useMutation({ @@ -214,23 +225,48 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { toast.success("Phase approved and merged"); phasesQuery.refetch(); }, - onError: (err) => toast.error(err.message), + onError: (err: { message: string }) => toast.error(err.message), }); - // Determine which diff to display - const activeDiffRaw = selectedCommit - ? commitDiffQuery.data?.rawDiff - : diffQuery.data?.rawDiff; + // Phase branch diff — metadata only, no parsing + const phaseFiles: FileDiff[] = useMemo( + () => { + const serverFiles = diffQuery.data?.files ?? []; + // Map server FileStatEntry (path) to frontend FileDiff (newPath) + return serverFiles.map((f: { + path: string; + oldPath?: string; + status: FileDiff['status']; + additions: number; + deletions: number; + projectId?: string; + }) => ({ + newPath: f.path, + oldPath: f.oldPath ?? f.path, + status: f.status, + additions: f.additions, + deletions: f.deletions, + projectId: f.projectId, + })); + }, + [diffQuery.data?.files], + ); - const files = useMemo(() => { - if (!activeDiffRaw) return []; - return parseUnifiedDiff(activeDiffRaw); - }, [activeDiffRaw]); + // Commit diff — still raw, parse client-side + const commitFiles: FileDiffDetail[] = useMemo(() => { + if (!commitDiffQuery.data?.rawDiff) return []; + return parseUnifiedDiff(commitDiffQuery.data.rawDiff); + }, [commitDiffQuery.data?.rawDiff]); const isDiffLoading = selectedCommit ? commitDiffQuery.isLoading : diffQuery.isLoading; + // All files for sidebar — always from phase metadata + const allFiles = phaseFiles; + + const activeFiles: FileDiff[] | FileDiffDetail[] = selectedCommit ? commitFiles : phaseFiles; + const handleAddComment = useCallback( (filePath: string, lineNumber: number, lineType: DiffLine["type"], body: string) => { if (!activePhaseId) return; @@ -273,7 +309,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { toast.success("Changes requested — revision task dispatched"); phasesQuery.refetch(); }, - onError: (err) => toast.error(err.message), + onError: (err: { message: string }) => toast.error(err.message), }); const handleRequestChanges = useCallback(() => { @@ -303,6 +339,11 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { setSelectedCommit(null); setStatus("pending"); setViewedFiles(new Set()); + setExpandAll(false); + }, []); + + const handleExpandAll = useCallback(() => { + setExpandAll(v => !v); }, []); const unresolvedCount = comments.filter((c) => !c.resolved && !c.parentCommentId).length; @@ -312,12 +353,6 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { reviewablePhases.find((p) => p.id === activePhaseId)?.name ?? "Phase"; - // All files from the full branch diff (for sidebar file list) - const allFiles = useMemo(() => { - if (!diffQuery.data?.rawDiff) return []; - return parseUnifiedDiff(diffQuery.data.rawDiff); - }, [diffQuery.data?.rawDiff]); - // Initiative-level review takes priority if (isInitiativePendingReview) { return ( @@ -363,6 +398,9 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { preview={previewState} viewedCount={viewedFiles.size} totalCount={allFiles.length} + totalAdditions={selectedCommit ? undefined : diffQuery.data?.totalAdditions} + totalDeletions={selectedCommit ? undefined : diffQuery.data?.totalDeletions} + onExpandAll={handleExpandAll} /> {/* Main content area — sidebar always rendered to preserve state */} @@ -382,7 +420,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { onFileClick={handleFileClick} onCommentClick={handleCommentClick} selectedCommit={selectedCommit} - activeFiles={files} + activeFiles={activeFiles} commits={commits} onSelectCommit={setSelectedCommit} viewedFiles={viewedFiles} @@ -397,7 +435,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) { Loading diff...
- ) : files.length === 0 ? ( + ) : activeFiles.length === 0 ? (
{selectedCommit ? "No changes in this commit" @@ -405,7 +443,7 @@ export function ReviewTab({ initiativeId }: ReviewTabProps) {
) : ( )}
diff --git a/docs/frontend.md b/docs/frontend.md index 0797687..ae7e448 100644 --- a/docs/frontend.md +++ b/docs/frontend.md @@ -113,10 +113,10 @@ The initiative detail page has three tabs managed via local state (not URL param ### Review Components (`src/components/review/`) | Component | Purpose | |-----------|---------| -| `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 | +| `ReviewTab` | Review tab container — orchestrates header, diff, sidebar, and preview. Phase diff uses metadata-only `FileDiff[]` from `getPhaseReviewDiff` (no rawDiff parsing). Commit diff parses `rawDiff` via `parseUnifiedDiff` → `FileDiffDetail[]`. Passes `commitMode`, `phaseId`, `expandAll` to DiffViewer | +| `ReviewHeader` | Consolidated toolbar: phase selector pills, branch info, stats (uses `totalAdditions`/`totalDeletions` props when available, falls back to summing files), preview controls, Expand all button, approve/reject actions | | `ReviewSidebar` | VSCode-style icon strip (Files/Commits views) with file list, root-only comment counts, and commit navigation | -| `DiffViewer` | Unified diff renderer with threaded inline comments (root + reply threads) | +| `DiffViewer` | Unified diff renderer with threaded inline comments (root + reply threads). Accepts `FileDiff[] | FileDiffDetail[]`, `phaseId`, `commitMode`, `expandAll` props | | `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 | | `PreviewPanel` | Docker preview status: building/running/failed with start/stop (legacy, now integrated into ReviewHeader) |