Skip to content

Commit 5cf6314

Browse files
neversettle17-101neversettle17-101
committed
feat: show multi-pr review status
1 parent 58426b8 commit 5cf6314

2 files changed

Lines changed: 118 additions & 87 deletions

File tree

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

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

59-
function mockCommonGets(reviews: unknown[] = [], reviewerHandleId = "") {
59+
function mockCommonGets(reviews: unknown[] = [], reviewerHandleId = "", items: unknown[] = []) {
6060
getMock.mockImplementation(async (path: string) => {
6161
if (path === "/api/v1/sessions/{sessionId}/reviews") {
62-
return { data: { reviewerHandleId, reviews } };
62+
return { data: { reviewerHandleId, reviews, items } };
6363
}
6464
if (path === "/api/v1/projects/{id}") {
6565
return {
@@ -94,10 +94,18 @@ const approvedReview = {
9494
createdAt: "2026-06-16T10:06:00Z",
9595
};
9696

97+
const reviewItem = (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();
100-
getMock.mockResolvedValue({ data: { reviewerHandleId: "", reviews: [] }, error: undefined });
108+
getMock.mockResolvedValue({ data: { reviewerHandleId: "", reviews: [], items: [] }, error: undefined });
101109
postMock.mockResolvedValue({ data: { ok: true, sessionId: "sess-1" }, error: undefined });
102110
});
103111

@@ -155,12 +163,15 @@ 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([], "", [reviewItem(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+
review: runningReview,
173+
reviews: [runningReview],
174+
items: [{ ...reviewItem(3, "running"), latestRun: runningReview }],
164175
},
165176
});
166177
const onOpenReviewerTerminal = vi.fn();
@@ -170,7 +181,7 @@ describe("SessionInspector reviews tab", () => {
170181
);
171182
await openReviewsTab();
172183

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

175186
await waitFor(() =>
176187
expect(postMock).toHaveBeenCalledWith("/api/v1/sessions/{sessionId}/reviews/trigger", {
@@ -180,11 +191,26 @@ describe("SessionInspector reviews tab", () => {
180191
expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" });
181192
});
182193

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");
194+
it("shows eligible and up-to-date PR review rows", async () => {
195+
mockCommonGets([approvedReview], "reviewer-pane", [
196+
reviewItem(3, "needs_review", "abc123"),
197+
reviewItem(4, "up_to_date", "def456"),
198+
]);
199+
200+
renderWithQuery(<SessionInspector session={session([pr(3, "open"), pr(4, "open")])} />);
201+
await openReviewsTab();
202+
203+
expect(await screen.findByText("PR #3")).toBeInTheDocument();
204+
expect(screen.getByText("Needs review")).toBeInTheDocument();
205+
expect(screen.getByText("PR #4")).toBeInTheDocument();
206+
expect(screen.getByText("Up to date")).toBeInTheDocument();
207+
});
208+
209+
it("shows a no-needed-reviews notice instead of opening the terminal when the backend reuses runs", async () => {
210+
mockCommonGets([approvedReview], "reviewer-pane", [reviewItem(3, "up_to_date")]);
185211
postMock.mockResolvedValue({
186212
response: { status: 200 },
187-
data: { reviewerHandleId: "reviewer-pane", review: approvedReview },
213+
data: { reviewerHandleId: "reviewer-pane", review: approvedReview, reviews: [], items: [reviewItem(3, "up_to_date")] },
188214
});
189215
const onOpenReviewerTerminal = vi.fn();
190216

@@ -193,22 +219,25 @@ describe("SessionInspector reviews tab", () => {
193219
);
194220
await openReviewsTab();
195221

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

198-
expect(await screen.findByText("Review is already up to date for this commit.")).toBeInTheDocument();
224+
expect(await screen.findByText("No needed reviews were started.")).toBeInTheDocument();
199225
expect(onOpenReviewerTerminal).not.toHaveBeenCalled();
200226
});
201227

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

206235
renderWithQuery(
207236
<SessionInspector onOpenReviewerTerminal={onOpenReviewerTerminal} session={session([pr(3, "open")])} />,
208237
);
209238
await openReviewsTab();
210239

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

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

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

Lines changed: 75 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay";
2727

2828
type ProjectConfig = components["schemas"]["ProjectConfig"];
2929
type ReviewRun = components["schemas"]["ReviewRun"];
30+
type PRReviewItem = components["schemas"]["PRReviewItem"];
3031
type ReviewsResponse = components["schemas"]["ListReviewsResponse"];
3132
type OpenReviewerTerminal = (target: { handleId: string; harness: string }) => void;
3233

@@ -387,14 +388,16 @@ function ReviewsView({
387388
enabled: hasPr,
388389
refetchInterval: (query) => {
389390
const data = query.state.data as ReviewsResponse | undefined;
390-
return data?.reviews.some((review) => review.status === "running") ? 2500 : false;
391+
return data?.items.some((item) => item.status === "running") || data?.reviews.some((review) => review.status === "running")
392+
? 2500
393+
: false;
391394
},
392395
queryFn: async () => {
393396
const { data, error } = await apiClient.GET("/api/v1/sessions/{sessionId}/reviews", {
394397
params: { path: { sessionId: session.id } },
395398
});
396399
if (error) throw new Error(apiErrorMessage(error, "Unable to load reviews"));
397-
return data ?? ({ reviewerHandleId: "", reviews: [] } satisfies ReviewsResponse);
400+
return data ?? ({ reviewerHandleId: "", reviews: [], items: [] } satisfies ReviewsResponse);
398401
},
399402
});
400403
const projectConfigQuery = useQuery({
@@ -422,16 +425,18 @@ function ReviewsView({
422425
onSuccess: ({ data, reused }) => {
423426
void queryClient.invalidateQueries({ queryKey: ["session-reviews", session.id] });
424427
void queryClient.invalidateQueries({ queryKey: workspaceQueryKey });
425-
if (reused) {
426-
setReviewNotice("Review is already up to date for this commit.");
428+
if (reused || !data?.reviews?.length) {
429+
setReviewNotice("No needed reviews were started.");
427430
return;
428431
}
429432
if (data?.reviewerHandleId) {
430-
onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness: data.review.harness || "reviewer" });
433+
const harness = data.reviews[0]?.harness || data.review.harness || "reviewer";
434+
onOpenReviewerTerminal?.({ handleId: data.reviewerHandleId, harness });
431435
}
432436
},
433437
});
434438
const reviews = reviewsQuery.data?.reviews ?? [];
439+
const items = reviewsQuery.data?.items ?? [];
435440

436441
return (
437442
<div role="tabpanel">
@@ -445,6 +450,7 @@ function ReviewsView({
445450
onTrigger={() => triggerReview.mutate()}
446451
reviewerHandleId={reviewsQuery.data?.reviewerHandleId ?? ""}
447452
reviews={reviews}
453+
items={items}
448454
notice={reviewNotice}
449455
session={session}
450456
/>
@@ -462,6 +468,7 @@ function ReviewPanel({
462468
session,
463469
config,
464470
reviews,
471+
items,
465472
reviewerHandleId,
466473
isLoading,
467474
isTriggering,
@@ -473,6 +480,7 @@ function ReviewPanel({
473480
session: WorkspaceSession;
474481
config?: ProjectConfig;
475482
reviews: ReviewRun[];
483+
items: PRReviewItem[];
476484
reviewerHandleId: string;
477485
isLoading: boolean;
478486
isTriggering: boolean;
@@ -490,19 +498,50 @@ function ReviewPanel({
490498

491499
const latest = latestReview(reviews);
492500
const harness = latest?.harness || config?.reviewers?.[0]?.harness || session.provider || "reviewer";
501+
const terminalEnabled = Boolean(reviewerHandleId && onOpenTerminal);
493502

494503
return (
495504
<div className="reviewer-list">
496505
{error ? <p className="reviewer-error">{apiErrorMessage(error, "Review request failed")}</p> : null}
497506
{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-
/>
507+
<div className="reviewer-card">
508+
<div className="reviewer-card__top">
509+
<div className="reviewer-card__name">
510+
<Shield aria-hidden="true" />
511+
<span>{harness}</span>
512+
</div>
513+
<span className="reviewer-status reviewer-status--neutral">{items.length} PRs</span>
514+
</div>
515+
<div className="reviewer-card__actions">
516+
<button
517+
className="reviewer-card__action reviewer-card__action--primary"
518+
disabled={isTriggering}
519+
onClick={onTrigger}
520+
type="button"
521+
>
522+
<Play aria-hidden="true" />
523+
{isTriggering ? "Starting..." : "Run needed reviews"}
524+
</button>
525+
<button
526+
className="reviewer-card__action"
527+
disabled={!terminalEnabled}
528+
onClick={() => {
529+
if (!terminalEnabled) return;
530+
onOpenTerminal?.({ handleId: reviewerHandleId, harness });
531+
}}
532+
type="button"
533+
>
534+
<Terminal aria-hidden="true" />
535+
Open terminal
536+
</button>
537+
</div>
538+
</div>
539+
<div className="flex flex-col gap-2">
540+
{items.length === 0 ? <p className="inspector-empty">No review state loaded yet.</p> : null}
541+
{items.map((item) => (
542+
<ReviewItemCard key={`${item.prUrl}:${item.targetSha}`} item={item} />
543+
))}
544+
</div>
506545
</div>
507546
);
508547
}
@@ -511,85 +550,48 @@ function latestReview(reviews: ReviewRun[]): ReviewRun | undefined {
511550
return [...reviews].sort((a, b) => Date.parse(b.createdAt) - Date.parse(a.createdAt))[0];
512551
}
513552

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-
553+
function ReviewItemCard({ item }: { item: PRReviewItem }) {
554+
const status = reviewItemStatus(item);
533555
return (
534-
<div className={cn("reviewer-card", status.tone && `reviewer-card--${status.tone}`)}>
556+
<div className={cn("reviewer-card", status.tone && `reviewer-card--${status.tone}`, item.status === "ineligible" && "opacity-70")}>
535557
<div className="reviewer-card__top">
536558
<div className="reviewer-card__name">
537-
<Shield aria-hidden="true" />
538-
<span>{harness}</span>
559+
<GitPullRequest aria-hidden="true" />
560+
<span>PR #{item.prNumber}</span>
539561
</div>
540562
<span className={cn("reviewer-status", `reviewer-status--${status.tone}`)}>
541563
{status.icon}
542564
{status.label}
543565
</span>
544566
</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}
567+
<div className="mt-2 min-w-0 truncate font-mono text-[10.5px] text-passive">
568+
<a href={item.prUrl} target="_blank" rel="noopener noreferrer" className="text-accent hover:underline">
569+
{item.prUrl}
570+
</a>
571+
{item.targetSha ? <span className="ml-2">{item.targetSha.slice(0, 7)}</span> : null}
569572
</div>
570573
</div>
571574
);
572575
}
573576

574-
function reviewStatus(review?: ReviewRun): {
577+
function reviewItemStatus(item: PRReviewItem): {
575578
label: string;
576579
tone: "neutral" | "running" | "success" | "danger";
577580
icon: ReactNode;
578581
} {
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" /> };
582+
switch (item.status) {
583+
case "needs_review":
584+
return { label: "Needs review", tone: "neutral", icon: null };
585+
case "running":
586+
return { label: "Running", tone: "running", icon: <Play aria-hidden="true" /> };
587+
case "up_to_date":
588+
return { label: "Up to date", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
589+
case "changes_requested":
590+
return { label: "Changes requested", tone: "danger", icon: <CircleMinus aria-hidden="true" /> };
591+
case "ineligible":
592+
return { label: "Ineligible", tone: "neutral", icon: <AlertCircle aria-hidden="true" /> };
591593
}
592-
return { label: "Complete", tone: "success", icon: <CheckCircle2 aria-hidden="true" /> };
594+
return { label: item.status, tone: "neutral", icon: null };
593595
}
594596

595597
function BrowserView({

0 commit comments

Comments
 (0)