diff --git a/frontend/src/renderer/components/SessionInspector.test.tsx b/frontend/src/renderer/components/SessionInspector.test.tsx index 244d9c4e..b36f4f35 100644 --- a/frontend/src/renderer/components/SessionInspector.test.tsx +++ b/frontend/src/renderer/components/SessionInspector.test.tsx @@ -56,7 +56,7 @@ function renderWithQuery(children: ReactNode) { return render({children}); } -function mockCommonGets(reviews: unknown[] = [], reviewerHandleId = "") { +function mockCommonGets(_unusedRuns: unknown[] = [], reviewerHandleId = "", reviews: unknown[] = []) { getMock.mockImplementation(async (path: string) => { if (path === "/api/v1/sessions/{sessionId}/reviews") { return { data: { reviewerHandleId, reviews } }; @@ -85,7 +85,6 @@ const approvedReview = { id: "run-1", reviewId: "review-1", sessionId: "sess-1", - batchId: "batch-1", harness: "codex", status: "complete", verdict: "approved", @@ -95,12 +94,14 @@ const approvedReview = { createdAt: "2026-06-16T10:06:00Z", }; -const reviewState = (latestRun = approvedReview) => ({ - prUrl: latestRun.prUrl, - prNumber: 3, - targetSha: latestRun.targetSha, - status: latestRun.status === "running" ? "running" : "up_to_date", - latestRun, +const reviewState = (n: number, status: string, targetSha = `sha-${n}`) => ({ + prUrl: `https://example.com/pr/${n}`, + prNumber: n, + title: `Reviewable change ${n}`, + targetSha, + status, + latestRun: + status === "up_to_date" ? { ...approvedReview, prUrl: `https://example.com/pr/${n}`, targetSha } : undefined, }); beforeEach(() => { @@ -164,12 +165,13 @@ describe("SessionInspector reviews tab", () => { const openReviewsTab = async () => userEvent.click(screen.getByRole("tab", { name: /Reviews/ })); it("triggers a review and opens the returned reviewer terminal", async () => { - mockCommonGets(); + mockCommonGets([], "", [reviewState(3, "needs_review")]); + const runningReview = { ...approvedReview, status: "running", verdict: "", body: "" }; postMock.mockResolvedValue({ response: { status: 201 }, data: { reviewerHandleId: "reviewer-pane", - reviews: [reviewState({ ...approvedReview, status: "running", verdict: "", body: "" })], + reviews: [{ ...reviewState(3, "running"), latestRun: runningReview }], }, }); const onOpenReviewerTerminal = vi.fn(); @@ -189,11 +191,34 @@ describe("SessionInspector reviews tab", () => { expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" }); }); - it("shows an up-to-date notice instead of opening the terminal when the backend reuses a run", async () => { - mockCommonGets([reviewState()], "reviewer-pane"); + it("shows eligible and up-to-date PR review rows", async () => { + mockCommonGets([approvedReview], "reviewer-pane", [ + reviewState(3, "needs_review", "abc123"), + reviewState(4, "up_to_date", "def456"), + ]); + + renderWithQuery(); + await openReviewsTab(); + + expect(await screen.findByText("Reviewable change 3")).toBeInTheDocument(); + expect(screen.getByText("#3")).toBeInTheDocument(); + expect(screen.getByText("Reviewable change 4")).toBeInTheDocument(); + expect(screen.getByText("#4")).toBeInTheDocument(); + expect(screen.getAllByText("Not run")).not.toHaveLength(0); + expect(screen.getAllByText("Approved")).not.toHaveLength(0); + expect(screen.getByRole("button", { name: "Re-run review" })).toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Run" })).not.toBeInTheDocument(); + expect(screen.queryByRole("button", { name: "Re-run" })).not.toBeInTheDocument(); + }); + + it("shows a no-needed-reviews notice instead of opening the terminal when the backend reuses runs", async () => { + mockCommonGets([approvedReview], "reviewer-pane", [reviewState(3, "up_to_date")]); postMock.mockResolvedValue({ response: { status: 200 }, - data: { reviewerHandleId: "reviewer-pane", reviews: [reviewState()] }, + data: { + reviewerHandleId: "reviewer-pane", + reviews: [], + }, }); const onOpenReviewerTerminal = vi.fn(); @@ -204,12 +229,15 @@ describe("SessionInspector reviews tab", () => { await userEvent.click(await screen.findByRole("button", { name: /re-run review/i })); - expect(await screen.findByText("Review is already up to date for this commit.")).toBeInTheDocument(); + expect(await screen.findByText("No needed reviews were started.")).toBeInTheDocument(); expect(onOpenReviewerTerminal).not.toHaveBeenCalled(); }); - it("shows an approved review and opens its terminal", async () => { - mockCommonGets([reviewState()], "reviewer-pane"); + it("shows one shared terminal action", async () => { + mockCommonGets([approvedReview], "reviewer-pane", [ + reviewState(3, "running", "abc123"), + reviewState(4, "up_to_date", "def456"), + ]); const onOpenReviewerTerminal = vi.fn(); renderWithQuery( @@ -217,12 +245,26 @@ describe("SessionInspector reviews tab", () => { ); await openReviewsTab(); - await waitFor(() => expect(screen.getAllByText("Approved").length).toBeGreaterThan(0)); + await waitFor(() => expect(screen.getAllByText("Open terminal")).toHaveLength(1)); + expect(screen.getAllByRole("button", { name: /review/i })).toHaveLength(1); await userEvent.click(screen.getByRole("button", { name: /open terminal/i })); expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" }); }); + it("shows the reviewer identity and aggregate verdict", async () => { + mockCommonGets([approvedReview], "reviewer-pane", [reviewState(3, "changes_requested", "abc123")]); + + renderWithQuery(); + await openReviewsTab(); + + expect(await screen.findByText("codex")).toBeInTheDocument(); + expect(screen.getByText("reviewer")).toBeInTheDocument(); + expect(screen.queryByText("sess-1")).not.toBeInTheDocument(); + expect(screen.queryByText("review session")).not.toBeInTheDocument(); + expect(screen.getAllByText("Changes requested")).not.toHaveLength(0); + }); + it("shows the no-PR empty state when the session has no PRs", async () => { mockCommonGets(); renderWithQuery(); diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index ef6c496f..17b6c6ee 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -1,15 +1,6 @@ import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; import { useState, type ReactNode } from "react"; -import { - AlertCircle, - ArrowUpRight, - CheckCircle2, - CircleMinus, - GitPullRequest, - Play, - Shield, - Terminal, -} from "lucide-react"; +import { ArrowUpRight, GitPullRequest, Play, Shield, Terminal } from "lucide-react"; import type { components } from "../../api/schema"; import { apiClient, apiErrorMessage } from "../lib/api-client"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; @@ -26,7 +17,6 @@ import { cn } from "../lib/utils"; import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay"; type ProjectConfig = components["schemas"]["ProjectConfig"]; -type ReviewRun = components["schemas"]["ReviewRun"]; type PRReviewState = components["schemas"]["PRReviewState"]; type ReviewsResponse = components["schemas"]["ListReviewsResponse"]; type OpenReviewerTerminal = (target: { handleId: string; harness: string }) => void; @@ -388,7 +378,8 @@ function ReviewsView({ enabled: hasPr, refetchInterval: (query) => { const data = query.state.data as ReviewsResponse | undefined; - return data?.reviews.some((review) => review.status === "running") ? 2500 : false; + const reviews = data?.reviews ?? []; + return reviews.some((review) => review.status === "running") ? 2500 : false; }, queryFn: async () => { const { data, error } = await apiClient.GET("/api/v1/sessions/{sessionId}/reviews", { @@ -423,17 +414,18 @@ function ReviewsView({ onSuccess: ({ data, reused }) => { void queryClient.invalidateQueries({ queryKey: ["session-reviews", session.id] }); void queryClient.invalidateQueries({ queryKey: workspaceQueryKey }); - if (reused) { - setReviewNotice("Review is already up to date for this commit."); + const started = data?.reviews?.find((review) => review.status === "running" && review.latestRun); + if (reused || !started?.latestRun) { + setReviewNotice("No needed reviews were started."); return; } if (data?.reviewerHandleId) { - const harness = latestReview(data.reviews)?.harness || "reviewer"; + const harness = started.latestRun.harness || "reviewer"; onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness }); } }, }); - const reviews = reviewsQuery.data?.reviews ?? []; + const reviewStates = reviewsQuery.data?.reviews ?? []; return (
@@ -446,7 +438,7 @@ function ReviewsView({ onOpenTerminal={onOpenReviewerTerminal} onTrigger={() => triggerReview.mutate()} reviewerHandleId={reviewsQuery.data?.reviewerHandleId ?? ""} - reviews={reviews} + reviewStates={reviewStates} notice={reviewNotice} session={session} /> @@ -463,7 +455,7 @@ function projectConfig(project: components["schemas"]["ProjectOrDegraded"] | und function ReviewPanel({ session, config, - reviews, + reviewStates, reviewerHandleId, isLoading, isTriggering, @@ -474,7 +466,7 @@ function ReviewPanel({ }: { session: WorkspaceSession; config?: ProjectConfig; - reviews: PRReviewState[]; + reviewStates: PRReviewState[]; reviewerHandleId: string; isLoading: boolean; isTriggering: boolean; @@ -490,111 +482,136 @@ function ReviewPanel({ return

Loading reviews...

; } - const latest = latestReview(reviews); + const latest = reviewStates.find((review) => review.latestRun)?.latestRun; const harness = latest?.harness || config?.reviewers?.[0]?.harness || session.provider || "reviewer"; + const terminalEnabled = Boolean(reviewerHandleId && onOpenTerminal); + const aggregateVerdict = sessionReviewVerdict(reviewStates); + const runAction = reviewSessionRunAction(reviewStates, isTriggering); + const runDisabled = + isTriggering || + reviewStates.length === 0 || + reviewStates.some((reviewState) => reviewState.status === "running") || + reviewStates.every((reviewState) => reviewState.status === "ineligible"); return (
{error ?

{apiErrorMessage(error, "Review request failed")}

: null} {notice ?

{notice}

: null} - -
- ); -} - -function latestReview(reviews: PRReviewState[]): ReviewRun | undefined { - return reviews - .map((review) => review.latestRun) - .filter((review): review is ReviewRun => Boolean(review)) - .sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))[0]; -} - -function ReviewerCard({ - harness, - review, - handleId, - isTriggering, - onTrigger, - onOpenTerminal, -}: { - harness: string; - review?: ReviewRun; - handleId: string; - isTriggering: boolean; - onTrigger: () => void; - onOpenTerminal?: OpenReviewerTerminal; -}) { - const status = reviewStatus(review); - const terminalEnabled = Boolean(handleId && onOpenTerminal); - const runLabel = review ? "Re-run review" : "Run review"; - - return ( -
-
-
-
- - {status.icon} - {status.label} - +
+
-
- - {review ? ( +
+
+ Pull requests + + {aggregateVerdict.label} + +
+
+ {reviewStates.length === 0 ?

No review state loaded yet.

: null} + {reviewStates.map((reviewState) => ( + + ))} +
+
+ - ) : null} +
+
+
+ ); +} + +function ReviewStateRow({ reviewState }: { reviewState: PRReviewState }) { + const verdict = reviewVerdict(reviewState); + const title = reviewState.title?.trim() || `PR #${reviewState.prNumber}`; + return ( +
+
+ +
+
+ {verdict.label}
); } -function reviewStatus(review?: ReviewRun): { +function sessionReviewVerdict(reviewStates: PRReviewState[]): { label: string; tone: "neutral" | "running" | "success" | "danger"; - icon: ReactNode; } { - if (!review) return { label: "Not run", tone: "neutral", icon: null }; - if (review.status === "running") { - return { label: "Running", tone: "running", icon: