Skip to content

Commit 8db8581

Browse files
neversettle17-101neversettle17-101
committed
feat: show multi-pr review status
1 parent 0be7112 commit 8db8581

2 files changed

Lines changed: 113 additions & 104 deletions

File tree

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

Lines changed: 40 additions & 19 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 } };
@@ -85,7 +85,6 @@ const approvedReview = {
8585
id: "run-1",
8686
reviewId: "review-1",
8787
sessionId: "sess-1",
88-
batchId: "batch-1",
8988
harness: "codex",
9089
status: "complete",
9190
verdict: "approved",
@@ -95,12 +94,12 @@ const approvedReview = {
9594
createdAt: "2026-06-16T10:06:00Z",
9695
};
9796

98-
const reviewState = (latestRun = approvedReview) => ({
99-
prUrl: latestRun.prUrl,
100-
prNumber: 3,
101-
targetSha: latestRun.targetSha,
102-
status: latestRun.status === "running" ? "running" : "up_to_date",
103-
latestRun,
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,
104103
});
105104

106105
beforeEach(() => {
@@ -164,12 +163,13 @@ describe("SessionInspector reviews tab", () => {
164163
const openReviewsTab = async () => userEvent.click(screen.getByRole("tab", { name: /Reviews/ }));
165164

166165
it("triggers a review and opens the returned reviewer terminal", async () => {
167-
mockCommonGets();
166+
mockCommonGets([], "", [reviewState(3, "needs_review")]);
167+
const runningReview = { ...approvedReview, status: "running", verdict: "", body: "" };
168168
postMock.mockResolvedValue({
169169
response: { status: 201 },
170170
data: {
171171
reviewerHandleId: "reviewer-pane",
172-
reviews: [reviewState({ ...approvedReview, status: "running", verdict: "", body: "" })],
172+
reviews: [{ ...reviewState(3, "running"), latestRun: runningReview }],
173173
},
174174
});
175175
const onOpenReviewerTerminal = vi.fn();
@@ -179,7 +179,7 @@ describe("SessionInspector reviews tab", () => {
179179
);
180180
await openReviewsTab();
181181

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

184184
await waitFor(() =>
185185
expect(postMock).toHaveBeenCalledWith("/api/v1/sessions/{sessionId}/reviews/trigger", {
@@ -189,11 +189,29 @@ describe("SessionInspector reviews tab", () => {
189189
expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" });
190190
});
191191

192-
it("shows an up-to-date notice instead of opening the terminal when the backend reuses a run", async () => {
193-
mockCommonGets([reviewState()], "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")]);
194209
postMock.mockResolvedValue({
195210
response: { status: 200 },
196-
data: { reviewerHandleId: "reviewer-pane", reviews: [reviewState()] },
211+
data: {
212+
reviewerHandleId: "reviewer-pane",
213+
reviews: [],
214+
},
197215
});
198216
const onOpenReviewerTerminal = vi.fn();
199217

@@ -202,22 +220,25 @@ describe("SessionInspector reviews tab", () => {
202220
);
203221
await openReviewsTab();
204222

205-
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 }));
206224

207-
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();
208226
expect(onOpenReviewerTerminal).not.toHaveBeenCalled();
209227
});
210228

211-
it("shows an approved review and opens its terminal", async () => {
212-
mockCommonGets([reviewState()], "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+
]);
213234
const onOpenReviewerTerminal = vi.fn();
214235

215236
renderWithQuery(
216237
<SessionInspector onOpenReviewerTerminal={onOpenReviewerTerminal} session={session([pr(3, "open")])} />,
217238
);
218239
await openReviewsTab();
219240

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

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

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

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

2828
type ProjectConfig = components["schemas"]["ProjectConfig"];
29-
type ReviewRun = components["schemas"]["ReviewRun"];
3029
type PRReviewState = components["schemas"]["PRReviewState"];
3130
type ReviewsResponse = components["schemas"]["ListReviewsResponse"];
3231
type OpenReviewerTerminal = (target: { handleId: string; harness: string }) => void;
@@ -388,7 +387,8 @@ function ReviewsView({
388387
enabled: hasPr,
389388
refetchInterval: (query) => {
390389
const data = query.state.data as ReviewsResponse | undefined;
391-
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;
392392
},
393393
queryFn: async () => {
394394
const { data, error } = await apiClient.GET("/api/v1/sessions/{sessionId}/reviews", {
@@ -423,17 +423,18 @@ function ReviewsView({
423423
onSuccess: ({ data, reused }) => {
424424
void queryClient.invalidateQueries({ queryKey: ["session-reviews", session.id] });
425425
void queryClient.invalidateQueries({ queryKey: workspaceQueryKey });
426-
if (reused) {
427-
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.");
428429
return;
429430
}
430431
if (data?.reviewerHandleId) {
431-
const harness = latestReview(data.reviews)?.harness || "reviewer";
432+
const harness = started.latestRun.harness || "reviewer";
432433
onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness });
433434
}
434435
},
435436
});
436-
const reviews = reviewsQuery.data?.reviews ?? [];
437+
const reviewStates = reviewsQuery.data?.reviews ?? [];
437438

438439
return (
439440
<div role="tabpanel">
@@ -446,7 +447,7 @@ function ReviewsView({
446447
onOpenTerminal={onOpenReviewerTerminal}
447448
onTrigger={() => triggerReview.mutate()}
448449
reviewerHandleId={reviewsQuery.data?.reviewerHandleId ?? ""}
449-
reviews={reviews}
450+
reviewStates={reviewStates}
450451
notice={reviewNotice}
451452
session={session}
452453
/>
@@ -463,7 +464,7 @@ function projectConfig(project: components["schemas"]["ProjectOrDegraded"] | und
463464
function ReviewPanel({
464465
session,
465466
config,
466-
reviews,
467+
reviewStates,
467468
reviewerHandleId,
468469
isLoading,
469470
isTriggering,
@@ -474,7 +475,7 @@ function ReviewPanel({
474475
}: {
475476
session: WorkspaceSession;
476477
config?: ProjectConfig;
477-
reviews: PRReviewState[];
478+
reviewStates: PRReviewState[];
478479
reviewerHandleId: string;
479480
isLoading: boolean;
480481
isTriggering: boolean;
@@ -490,111 +491,98 @@ function ReviewPanel({
490491
return <p className="inspector-empty">Loading reviews...</p>;
491492
}
492493

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

496498
return (
497499
<div className="reviewer-list">
498500
{error ? <p className="reviewer-error">{apiErrorMessage(error, "Review request failed")}</p> : null}
499501
{notice ? <p className="reviewer-notice">{notice}</p> : null}
500-
<ReviewerCard
501-
handleId={reviewerHandleId}
502-
harness={harness}
503-
isTriggering={isTriggering}
504-
onOpenTerminal={onOpenTerminal}
505-
onTrigger={onTrigger}
506-
review={latest}
507-
/>
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>
508540
</div>
509541
);
510542
}
511543

512-
function latestReview(reviews: PRReviewState[]): ReviewRun | undefined {
513-
return reviews
514-
.map((review) => review.latestRun)
515-
.filter((review): review is ReviewRun => Boolean(review))
516-
.sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))[0];
517-
}
518-
519-
function ReviewerCard({
520-
harness,
521-
review,
522-
handleId,
523-
isTriggering,
524-
onTrigger,
525-
onOpenTerminal,
526-
}: {
527-
harness: string;
528-
review?: ReviewRun;
529-
handleId: string;
530-
isTriggering: boolean;
531-
onTrigger: () => void;
532-
onOpenTerminal?: OpenReviewerTerminal;
533-
}) {
534-
const status = reviewStatus(review);
535-
const terminalEnabled = Boolean(handleId && onOpenTerminal);
536-
const runLabel = review ? "Re-run review" : "Run review";
537-
544+
function ReviewStateCard({ reviewState }: { reviewState: PRReviewState }) {
545+
const status = reviewStateStatus(reviewState);
538546
return (
539-
<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")}>
540548
<div className="reviewer-card__top">
541549
<div className="reviewer-card__name">
542-
<Shield aria-hidden="true" />
543-
<span>{harness}</span>
550+
<GitPullRequest aria-hidden="true" />
551+
<span>PR #{reviewState.prNumber}</span>
544552
</div>
545553
<span className={cn("reviewer-status", `reviewer-status--${status.tone}`)}>
546554
{status.icon}
547555
{status.label}
548556
</span>
549557
</div>
550-
<div className="reviewer-card__actions">
551-
<button
552-
className="reviewer-card__action reviewer-card__action--primary"
553-
disabled={isTriggering}
554-
onClick={onTrigger}
555-
type="button"
556-
>
557-
<Play aria-hidden="true" />
558-
{isTriggering ? "Starting..." : runLabel}
559-
</button>
560-
{review ? (
561-
<button
562-
className="reviewer-card__action"
563-
disabled={!terminalEnabled}
564-
onClick={() => {
565-
if (!terminalEnabled) return;
566-
onOpenTerminal?.({ handleId, harness });
567-
}}
568-
type="button"
569-
>
570-
<Terminal aria-hidden="true" />
571-
Open terminal
572-
</button>
573-
) : 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}
574563
</div>
575564
</div>
576565
);
577566
}
578567

579-
function reviewStatus(review?: ReviewRun): {
568+
function reviewStateStatus(reviewState: PRReviewState): {
580569
label: string;
581570
tone: "neutral" | "running" | "success" | "danger";
582571
icon: ReactNode;
583572
} {
584-
if (!review) return { label: "Not run", tone: "neutral", icon: null };
585-
if (review.status === "running") {
586-
return { label: "Running", tone: "running", icon: <Play aria-hidden="true" /> };
587-
}
588-
if (review.status === "failed") {
589-
return { label: "Failed", tone: "danger", icon: <AlertCircle aria-hidden="true" /> };
590-
}
591-
if (review.verdict === "approved") {
592-
return { label: "Approved", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
593-
}
594-
if (review.verdict === "changes_requested") {
595-
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" /> };
596584
}
597-
return { label: "Complete", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
585+
return { label: reviewState.status, tone: "neutral", icon: null };
598586
}
599587

600588
function BrowserView({

0 commit comments

Comments
 (0)