From b01179dd3186fe2fe3aa696255b810b3131454c8 Mon Sep 17 00:00:00 2001 From: Aditi Chauhan Date: Tue, 23 Jun 2026 15:55:27 +0530 Subject: [PATCH 1/7] feat: show multi-pr review status --- .../components/SessionInspector.test.tsx | 59 ++++--- .../renderer/components/SessionInspector.tsx | 158 ++++++++---------- 2 files changed, 113 insertions(+), 104 deletions(-) diff --git a/frontend/src/renderer/components/SessionInspector.test.tsx b/frontend/src/renderer/components/SessionInspector.test.tsx index 244d9c4e..a3716066 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,12 @@ 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, + targetSha, + status, + latestRun: status === "up_to_date" ? { ...approvedReview, prUrl: `https://example.com/pr/${n}`, targetSha } : undefined, }); beforeEach(() => { @@ -164,12 +163,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(); @@ -179,7 +179,7 @@ describe("SessionInspector reviews tab", () => { ); await openReviewsTab(); - await userEvent.click(await screen.findByRole("button", { name: /run review/i })); + await userEvent.click(await screen.findByRole("button", { name: /run needed reviews/i })); await waitFor(() => expect(postMock).toHaveBeenCalledWith("/api/v1/sessions/{sessionId}/reviews/trigger", { @@ -189,11 +189,29 @@ 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("PR #3")).toBeInTheDocument(); + expect(screen.getByText("Needs review")).toBeInTheDocument(); + expect(screen.getByText("PR #4")).toBeInTheDocument(); + expect(screen.getByText("Up to date")).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(); @@ -202,14 +220,17 @@ describe("SessionInspector reviews tab", () => { ); await openReviewsTab(); - await userEvent.click(await screen.findByRole("button", { name: /re-run review/i })); + await userEvent.click(await screen.findByRole("button", { name: /run needed reviews/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,7 +238,7 @@ describe("SessionInspector reviews tab", () => { ); await openReviewsTab(); - await waitFor(() => expect(screen.getAllByText("Approved").length).toBeGreaterThan(0)); + await waitFor(() => expect(screen.getAllByText("Open terminal")).toHaveLength(1)); await userEvent.click(screen.getByRole("button", { name: /open terminal/i })); expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" }); diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index ef6c496f..ad52b7d2 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -26,7 +26,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 +387,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 +423,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 +447,7 @@ function ReviewsView({ onOpenTerminal={onOpenReviewerTerminal} onTrigger={() => triggerReview.mutate()} reviewerHandleId={reviewsQuery.data?.reviewerHandleId ?? ""} - reviews={reviews} + reviewStates={reviewStates} notice={reviewNotice} session={session} /> @@ -463,7 +464,7 @@ function projectConfig(project: components["schemas"]["ProjectOrDegraded"] | und function ReviewPanel({ session, config, - reviews, + reviewStates, reviewerHandleId, isLoading, isTriggering, @@ -474,7 +475,7 @@ function ReviewPanel({ }: { session: WorkspaceSession; config?: ProjectConfig; - reviews: PRReviewState[]; + reviewStates: PRReviewState[]; reviewerHandleId: string; isLoading: boolean; isTriggering: boolean; @@ -490,111 +491,98 @@ 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); return (
{error ?

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

: null} {notice ?

{notice}

: null} - +
+
+
+
+ {reviewStates.length} PRs +
+
+ + +
+
+
+ {reviewStates.length === 0 ?

No review state loaded yet.

: null} + {reviewStates.map((reviewState) => ( + + ))} +
); } -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"; - +function ReviewStateCard({ reviewState }: { reviewState: PRReviewState }) { + const status = reviewStateStatus(reviewState); return ( -
+
-
{status.icon} {status.label}
-
- - {review ? ( - - ) : null} +
+ + {reviewState.prUrl} + + {reviewState.targetSha ? {reviewState.targetSha.slice(0, 7)} : null}
); } -function reviewStatus(review?: ReviewRun): { +function reviewStateStatus(reviewState: 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: