Skip to content

Commit bb00389

Browse files
neversettle17-101neversettle17-101
committed
feat: show multi-pr review status
1 parent b45f263 commit bb00389

2 files changed

Lines changed: 117 additions & 94 deletions

File tree

‎frontend/src/renderer/components/SessionInspector.test.tsx‎

Lines changed: 42 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ function renderWithQuery(children: ReactNode) {
5656
return render(<QueryClientProvider client={client}>{children}</QueryClientProvider>);
5757
}
5858

59-
function mockCommonGets(reviews: unknown[] = [], reviewerHandleId = "") {
59+
function mockCommonGets(_unusedRuns: unknown[] = [], reviewerHandleId = "", reviews: unknown[] = []) {
6060
getMock.mockImplementation(async (path: string) => {
6161
if (path === "/api/v1/sessions/{sessionId}/reviews") {
6262
return { data: { reviewerHandleId, reviews } };
@@ -94,6 +94,14 @@ const approvedReview = {
9494
createdAt: "2026-06-16T10:06:00Z",
9595
};
9696

97+
const reviewState = (n: number, status: string, targetSha = `sha-${n}`) => ({
98+
prUrl: `https://example.com/pr/${n}`,
99+
prNumber: n,
100+
targetSha,
101+
status,
102+
latestRun: status === "up_to_date" ? { ...approvedReview, prUrl: `https://example.com/pr/${n}`, targetSha } : undefined,
103+
});
104+
97105
beforeEach(() => {
98106
getMock.mockReset();
99107
postMock.mockReset();
@@ -155,12 +163,13 @@ describe("SessionInspector reviews tab", () => {
155163
const openReviewsTab = async () => userEvent.click(screen.getByRole("tab", { name: /Reviews/ }));
156164

157165
it("triggers a review and opens the returned reviewer terminal", async () => {
158-
mockCommonGets();
166+
mockCommonGets([], "", [reviewState(3, "needs_review")]);
167+
const runningReview = { ...approvedReview, status: "running", verdict: "", body: "" };
159168
postMock.mockResolvedValue({
160169
response: { status: 201 },
161170
data: {
162171
reviewerHandleId: "reviewer-pane",
163-
review: { ...approvedReview, status: "running", verdict: "", body: "" },
172+
reviews: [{ ...reviewState(3, "running"), latestRun: runningReview }],
164173
},
165174
});
166175
const onOpenReviewerTerminal = vi.fn();
@@ -170,7 +179,7 @@ describe("SessionInspector reviews tab", () => {
170179
);
171180
await openReviewsTab();
172181

173-
await userEvent.click(await screen.findByRole("button", { name: /run review/i }));
182+
await userEvent.click(await screen.findByRole("button", { name: /run needed reviews/i }));
174183

175184
await waitFor(() =>
176185
expect(postMock).toHaveBeenCalledWith("/api/v1/sessions/{sessionId}/reviews/trigger", {
@@ -180,11 +189,29 @@ describe("SessionInspector reviews tab", () => {
180189
expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" });
181190
});
182191

183-
it("shows an up-to-date notice instead of opening the terminal when the backend reuses a run", async () => {
184-
mockCommonGets([approvedReview], "reviewer-pane");
192+
it("shows eligible and up-to-date PR review rows", async () => {
193+
mockCommonGets([approvedReview], "reviewer-pane", [
194+
reviewState(3, "needs_review", "abc123"),
195+
reviewState(4, "up_to_date", "def456"),
196+
]);
197+
198+
renderWithQuery(<SessionInspector session={session([pr(3, "open"), pr(4, "open")])} />);
199+
await openReviewsTab();
200+
201+
expect(await screen.findByText("PR #3")).toBeInTheDocument();
202+
expect(screen.getByText("Needs review")).toBeInTheDocument();
203+
expect(screen.getByText("PR #4")).toBeInTheDocument();
204+
expect(screen.getByText("Up to date")).toBeInTheDocument();
205+
});
206+
207+
it("shows a no-needed-reviews notice instead of opening the terminal when the backend reuses runs", async () => {
208+
mockCommonGets([approvedReview], "reviewer-pane", [reviewState(3, "up_to_date")]);
185209
postMock.mockResolvedValue({
186210
response: { status: 200 },
187-
data: { reviewerHandleId: "reviewer-pane", review: approvedReview },
211+
data: {
212+
reviewerHandleId: "reviewer-pane",
213+
reviews: [],
214+
},
188215
});
189216
const onOpenReviewerTerminal = vi.fn();
190217

@@ -193,22 +220,25 @@ describe("SessionInspector reviews tab", () => {
193220
);
194221
await openReviewsTab();
195222

196-
await userEvent.click(await screen.findByRole("button", { name: /re-run review/i }));
223+
await userEvent.click(await screen.findByRole("button", { name: /run needed reviews/i }));
197224

198-
expect(await screen.findByText("Review is already up to date for this commit.")).toBeInTheDocument();
225+
expect(await screen.findByText("No needed reviews were started.")).toBeInTheDocument();
199226
expect(onOpenReviewerTerminal).not.toHaveBeenCalled();
200227
});
201228

202-
it("shows an approved review and opens its terminal", async () => {
203-
mockCommonGets([approvedReview], "reviewer-pane");
229+
it("shows one shared terminal action", async () => {
230+
mockCommonGets([approvedReview], "reviewer-pane", [
231+
reviewState(3, "running", "abc123"),
232+
reviewState(4, "up_to_date", "def456"),
233+
]);
204234
const onOpenReviewerTerminal = vi.fn();
205235

206236
renderWithQuery(
207237
<SessionInspector onOpenReviewerTerminal={onOpenReviewerTerminal} session={session([pr(3, "open")])} />,
208238
);
209239
await openReviewsTab();
210240

211-
await waitFor(() => expect(screen.getAllByText("Approved").length).toBeGreaterThan(0));
241+
await waitFor(() => expect(screen.getAllByText("Open terminal")).toHaveLength(1));
212242
await userEvent.click(screen.getByRole("button", { name: /open terminal/i }));
213243

214244
expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" });

‎frontend/src/renderer/components/SessionInspector.tsx‎

Lines changed: 75 additions & 82 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ import { cn } from "../lib/utils";
2626
import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay";
2727

2828
type ProjectConfig = components["schemas"]["ProjectConfig"];
29-
type ReviewRun = components["schemas"]["ReviewRun"];
29+
type PRReviewState = components["schemas"]["PRReviewState"];
3030
type ReviewsResponse = components["schemas"]["ListReviewsResponse"];
3131
type OpenReviewerTerminal = (target: { handleId: string; harness: string }) => void;
3232

@@ -387,7 +387,8 @@ function ReviewsView({
387387
enabled: hasPr,
388388
refetchInterval: (query) => {
389389
const data = query.state.data as ReviewsResponse | undefined;
390-
return data?.reviews.some((review) => review.status === "running") ? 2500 : false;
390+
const reviews = data?.reviews ?? [];
391+
return reviews.some((review) => review.status === "running") ? 2500 : false;
391392
},
392393
queryFn: async () => {
393394
const { data, error } = await apiClient.GET("/api/v1/sessions/{sessionId}/reviews", {
@@ -422,16 +423,18 @@ function ReviewsView({
422423
onSuccess: ({ data, reused }) => {
423424
void queryClient.invalidateQueries({ queryKey: ["session-reviews", session.id] });
424425
void queryClient.invalidateQueries({ queryKey: workspaceQueryKey });
425-
if (reused) {
426-
setReviewNotice("Review is already up to date for this commit.");
426+
const started = data?.reviews?.find((review) => review.status === "running" && review.latestRun);
427+
if (reused || !started?.latestRun) {
428+
setReviewNotice("No needed reviews were started.");
427429
return;
428430
}
429431
if (data?.reviewerHandleId) {
430-
onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness: data.review.harness || "reviewer" });
432+
const harness = started.latestRun.harness || "reviewer";
433+
onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness });
431434
}
432435
},
433436
});
434-
const reviews = reviewsQuery.data?.reviews ?? [];
437+
const reviewStates = reviewsQuery.data?.reviews ?? [];
435438

436439
return (
437440
<div role="tabpanel">
@@ -444,7 +447,7 @@ function ReviewsView({
444447
onOpenTerminal={onOpenReviewerTerminal}
445448
onTrigger={() => triggerReview.mutate()}
446449
reviewerHandleId={reviewsQuery.data?.reviewerHandleId ?? ""}
447-
reviews={reviews}
450+
reviewStates={reviewStates}
448451
notice={reviewNotice}
449452
session={session}
450453
/>
@@ -461,7 +464,7 @@ function projectConfig(project: components["schemas"]["ProjectOrDegraded"] | und
461464
function ReviewPanel({
462465
session,
463466
config,
464-
reviews,
467+
reviewStates,
465468
reviewerHandleId,
466469
isLoading,
467470
isTriggering,
@@ -472,7 +475,7 @@ function ReviewPanel({
472475
}: {
473476
session: WorkspaceSession;
474477
config?: ProjectConfig;
475-
reviews: ReviewRun[];
478+
reviewStates: PRReviewState[];
476479
reviewerHandleId: string;
477480
isLoading: boolean;
478481
isTriggering: boolean;
@@ -488,108 +491,98 @@ function ReviewPanel({
488491
return <p className="inspector-empty">Loading reviews...</p>;
489492
}
490493

491-
const latest = latestReview(reviews);
494+
const latest = reviewStates.find((review) => review.latestRun)?.latestRun;
492495
const harness = latest?.harness || config?.reviewers?.[0]?.harness || session.provider || "reviewer";
496+
const terminalEnabled = Boolean(reviewerHandleId && onOpenTerminal);
493497

494498
return (
495499
<div className="reviewer-list">
496500
{error ? <p className="reviewer-error">{apiErrorMessage(error, "Review request failed")}</p> : null}
497501
{notice ? <p className="reviewer-notice">{notice}</p> : null}
498-
<ReviewerCard
499-
handleId={reviewerHandleId}
500-
harness={harness}
501-
isTriggering={isTriggering}
502-
onOpenTerminal={onOpenTerminal}
503-
onTrigger={onTrigger}
504-
review={latest}
505-
/>
502+
<div className="reviewer-card">
503+
<div className="reviewer-card__top">
504+
<div className="reviewer-card__name">
505+
<Shield aria-hidden="true" />
506+
<span>{harness}</span>
507+
</div>
508+
<span className="reviewer-status reviewer-status--neutral">{reviewStates.length} PRs</span>
509+
</div>
510+
<div className="reviewer-card__actions">
511+
<button
512+
className="reviewer-card__action reviewer-card__action--primary"
513+
disabled={isTriggering}
514+
onClick={onTrigger}
515+
type="button"
516+
>
517+
<Play aria-hidden="true" />
518+
{isTriggering ? "Starting..." : "Run needed reviews"}
519+
</button>
520+
<button
521+
className="reviewer-card__action"
522+
disabled={!terminalEnabled}
523+
onClick={() => {
524+
if (!terminalEnabled) return;
525+
onOpenTerminal?.({ handleId: reviewerHandleId, harness });
526+
}}
527+
type="button"
528+
>
529+
<Terminal aria-hidden="true" />
530+
Open terminal
531+
</button>
532+
</div>
533+
</div>
534+
<div className="flex flex-col gap-2">
535+
{reviewStates.length === 0 ? <p className="inspector-empty">No review state loaded yet.</p> : null}
536+
{reviewStates.map((reviewState) => (
537+
<ReviewStateCard key={`${reviewState.prUrl}:${reviewState.targetSha}`} reviewState={reviewState} />
538+
))}
539+
</div>
506540
</div>
507541
);
508542
}
509543

510-
function latestReview(reviews: ReviewRun[]): ReviewRun | undefined {
511-
return [...reviews].sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))[0];
512-
}
513-
514-
function ReviewerCard({
515-
harness,
516-
review,
517-
handleId,
518-
isTriggering,
519-
onTrigger,
520-
onOpenTerminal,
521-
}: {
522-
harness: string;
523-
review?: ReviewRun;
524-
handleId: string;
525-
isTriggering: boolean;
526-
onTrigger: () => void;
527-
onOpenTerminal?: OpenReviewerTerminal;
528-
}) {
529-
const status = reviewStatus(review);
530-
const terminalEnabled = Boolean(handleId && onOpenTerminal);
531-
const runLabel = review ? "Re-run review" : "Run review";
532-
544+
function ReviewStateCard({ reviewState }: { reviewState: PRReviewState }) {
545+
const status = reviewStateStatus(reviewState);
533546
return (
534-
<div className={cn("reviewer-card", status.tone && `reviewer-card--${status.tone}`)}>
547+
<div className={cn("reviewer-card", status.tone && `reviewer-card--${status.tone}`, reviewState.status === "ineligible" && "opacity-70")}>
535548
<div className="reviewer-card__top">
536549
<div className="reviewer-card__name">
537-
<Shield aria-hidden="true" />
538-
<span>{harness}</span>
550+
<GitPullRequest aria-hidden="true" />
551+
<span>PR #{reviewState.prNumber}</span>
539552
</div>
540553
<span className={cn("reviewer-status", `reviewer-status--${status.tone}`)}>
541554
{status.icon}
542555
{status.label}
543556
</span>
544557
</div>
545-
<div className="reviewer-card__actions">
546-
<button
547-
className="reviewer-card__action reviewer-card__action--primary"
548-
disabled={isTriggering}
549-
onClick={onTrigger}
550-
type="button"
551-
>
552-
<Play aria-hidden="true" />
553-
{isTriggering ? "Starting..." : runLabel}
554-
</button>
555-
{review ? (
556-
<button
557-
className="reviewer-card__action"
558-
disabled={!terminalEnabled}
559-
onClick={() => {
560-
if (!terminalEnabled) return;
561-
onOpenTerminal?.({ handleId, harness });
562-
}}
563-
type="button"
564-
>
565-
<Terminal aria-hidden="true" />
566-
Open terminal
567-
</button>
568-
) : null}
558+
<div className="mt-2 min-w-0 truncate font-mono text-[10.5px] text-passive">
559+
<a href={reviewState.prUrl} target="_blank" rel="noopener noreferrer" className="text-accent hover:underline">
560+
{reviewState.prUrl}
561+
</a>
562+
{reviewState.targetSha ? <span className="ml-2">{reviewState.targetSha.slice(0, 7)}</span> : null}
569563
</div>
570564
</div>
571565
);
572566
}
573567

574-
function reviewStatus(review?: ReviewRun): {
568+
function reviewStateStatus(reviewState: PRReviewState): {
575569
label: string;
576570
tone: "neutral" | "running" | "success" | "danger";
577571
icon: ReactNode;
578572
} {
579-
if (!review) return { label: "Not run", tone: "neutral", icon: null };
580-
if (review.status === "running") {
581-
return { label: "Running", tone: "running", icon: <Play aria-hidden="true" /> };
582-
}
583-
if (review.status === "failed") {
584-
return { label: "Failed", tone: "danger", icon: <AlertCircle aria-hidden="true" /> };
585-
}
586-
if (review.verdict === "approved") {
587-
return { label: "Approved", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
588-
}
589-
if (review.verdict === "changes_requested") {
590-
return { label: "Changes requested", tone: "danger", icon: <CircleMinus aria-hidden="true" /> };
573+
switch (reviewState.status) {
574+
case "needs_review":
575+
return { label: "Needs review", tone: "neutral", icon: null };
576+
case "running":
577+
return { label: "Running", tone: "running", icon: <Play aria-hidden="true" /> };
578+
case "up_to_date":
579+
return { label: "Up to date", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
580+
case "changes_requested":
581+
return { label: "Changes requested", tone: "danger", icon: <CircleMinus aria-hidden="true" /> };
582+
case "ineligible":
583+
return { label: "Ineligible", tone: "neutral", icon: <AlertCircle aria-hidden="true" /> };
591584
}
592-
return { label: "Complete", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
585+
return { label: reviewState.status, tone: "neutral", icon: null };
593586
}
594587

595588
function BrowserView({

0 commit comments

Comments
 (0)