Skip to content

Commit d31238a

Browse files
fix: match multi-pr review card design
1 parent 902b8db commit d31238a

3 files changed

Lines changed: 139 additions & 108 deletions

File tree

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

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -181,7 +181,7 @@ describe("SessionInspector reviews tab", () => {
181181
);
182182
await openReviewsTab();
183183

184-
await userEvent.click(await screen.findByRole("button", { name: /^run$/i }));
184+
await userEvent.click(await screen.findByRole("button", { name: /run review/i }));
185185

186186
await waitFor(() =>
187187
expect(postMock).toHaveBeenCalledWith("/api/v1/sessions/{sessionId}/reviews/trigger", {
@@ -206,8 +206,9 @@ describe("SessionInspector reviews tab", () => {
206206
expect(screen.getByText("#4")).toBeInTheDocument();
207207
expect(screen.getAllByText("Not run")).not.toHaveLength(0);
208208
expect(screen.getAllByText("Approved")).not.toHaveLength(0);
209-
expect(screen.getByRole("button", { name: "Run" })).toBeInTheDocument();
210-
expect(screen.getByRole("button", { name: "Re-run" })).toBeInTheDocument();
209+
expect(screen.getByRole("button", { name: "Re-run review" })).toBeInTheDocument();
210+
expect(screen.queryByRole("button", { name: "Run" })).not.toBeInTheDocument();
211+
expect(screen.queryByRole("button", { name: "Re-run" })).not.toBeInTheDocument();
211212
});
212213

213214
it("shows a no-needed-reviews notice instead of opening the terminal when the backend reuses runs", async () => {
@@ -226,7 +227,7 @@ describe("SessionInspector reviews tab", () => {
226227
);
227228
await openReviewsTab();
228229

229-
await userEvent.click(await screen.findByRole("button", { name: /re-run/i }));
230+
await userEvent.click(await screen.findByRole("button", { name: /re-run review/i }));
230231

231232
expect(await screen.findByText("No needed reviews were started.")).toBeInTheDocument();
232233
expect(onOpenReviewerTerminal).not.toHaveBeenCalled();
@@ -245,6 +246,7 @@ describe("SessionInspector reviews tab", () => {
245246
await openReviewsTab();
246247

247248
await waitFor(() => expect(screen.getAllByText("Open terminal")).toHaveLength(1));
249+
expect(screen.getAllByRole("button", { name: /review/i })).toHaveLength(1);
248250
await userEvent.click(screen.getByRole("button", { name: /open terminal/i }));
249251

250252
expect(onOpenReviewerTerminal).toHaveBeenCalledWith({ handleId: "reviewer-pane", harness: "codex" });
@@ -257,7 +259,9 @@ describe("SessionInspector reviews tab", () => {
257259
await openReviewsTab();
258260

259261
expect(await screen.findByText("codex")).toBeInTheDocument();
262+
expect(screen.getByText("reviewer")).toBeInTheDocument();
260263
expect(screen.getByText("sess-1")).toBeInTheDocument();
264+
expect(screen.getByText("review session")).toBeInTheDocument();
261265
expect(screen.getAllByText("Changes requested")).not.toHaveLength(0);
262266
});
263267

frontend/src/renderer/components/SessionInspector.tsx

Lines changed: 56 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query";
22
import { useState, type ReactNode } from "react";
3-
import { ArrowUpRight, GitPullRequest, Shield, Terminal } from "lucide-react";
3+
import { ArrowUpRight, GitPullRequest, Play, Shield, Terminal } from "lucide-react";
44
import type { components } from "../../api/schema";
55
import { apiClient, apiErrorMessage } from "../lib/api-client";
66
import { workspaceQueryKey } from "../hooks/useWorkspaceQuery";
@@ -486,64 +486,71 @@ function ReviewPanel({
486486
const harness = latest?.harness || config?.reviewers?.[0]?.harness || session.provider || "reviewer";
487487
const terminalEnabled = Boolean(reviewerHandleId && onOpenTerminal);
488488
const aggregateVerdict = sessionReviewVerdict(reviewStates);
489+
const runAction = reviewSessionRunAction(reviewStates, isTriggering);
490+
const runDisabled =
491+
isTriggering ||
492+
reviewStates.length === 0 ||
493+
reviewStates.some((reviewState) => reviewState.status === "running") ||
494+
reviewStates.every((reviewState) => reviewState.status === "ineligible");
489495

490496
return (
491497
<div className="reviewer-list">
492498
{error ? <p className="reviewer-error">{apiErrorMessage(error, "Review request failed")}</p> : null}
493499
{notice ? <p className="reviewer-notice">{notice}</p> : null}
500+
<div className="reviewer-kicker">
501+
<Shield aria-hidden="true" />
502+
<span>{harness}</span>
503+
<span>reviewer</span>
504+
</div>
494505
<div className="reviewer-card">
495506
<div className="reviewer-card__top">
496507
<div className="reviewer-card__identity">
497508
<div className="reviewer-card__name">
498-
<Shield aria-hidden="true" />
499-
<span>{harness}</span>
509+
<GitPullRequest aria-hidden="true" />
510+
<span>{session.id}</span>
500511
</div>
501-
<div className="reviewer-card__session">{session.id}</div>
512+
<div className="reviewer-card__session">review session</div>
502513
</div>
503514
<span className={cn("reviewer-status", `reviewer-status--${aggregateVerdict.tone}`)}>
504515
{aggregateVerdict.label}
505516
</span>
506517
</div>
518+
<div className="reviewer-summary-list">
519+
{reviewStates.length === 0 ? <p className="inspector-empty">No review state loaded yet.</p> : null}
520+
{reviewStates.map((reviewState) => (
521+
<ReviewStateRow key={`${reviewState.prUrl}:${reviewState.targetSha}`} reviewState={reviewState} />
522+
))}
523+
</div>
524+
<div className="reviewer-card__actions">
525+
<button
526+
className="reviewer-card__action reviewer-card__action--primary"
527+
disabled={runDisabled}
528+
onClick={onTrigger}
529+
type="button"
530+
>
531+
<Play aria-hidden="true" />
532+
{runAction}
533+
</button>
534+
<button
535+
className="reviewer-card__action"
536+
disabled={!terminalEnabled}
537+
onClick={() => {
538+
if (!terminalEnabled) return;
539+
onOpenTerminal?.({ handleId: reviewerHandleId, harness });
540+
}}
541+
type="button"
542+
>
543+
<Terminal aria-hidden="true" />
544+
Open terminal
545+
</button>
546+
</div>
507547
</div>
508-
<div className="reviewer-summary-list">
509-
{reviewStates.length === 0 ? <p className="inspector-empty">No review state loaded yet.</p> : null}
510-
{reviewStates.map((reviewState) => (
511-
<ReviewStateRow
512-
key={`${reviewState.prUrl}:${reviewState.targetSha}`}
513-
isTriggering={isTriggering}
514-
onTrigger={onTrigger}
515-
reviewState={reviewState}
516-
/>
517-
))}
518-
</div>
519-
<button
520-
className="reviewer-card__action reviewer-card__action--wide"
521-
disabled={!terminalEnabled}
522-
onClick={() => {
523-
if (!terminalEnabled) return;
524-
onOpenTerminal?.({ handleId: reviewerHandleId, harness });
525-
}}
526-
type="button"
527-
>
528-
<Terminal aria-hidden="true" />
529-
Open terminal
530-
</button>
531548
</div>
532549
);
533550
}
534551

535-
function ReviewStateRow({
536-
reviewState,
537-
isTriggering,
538-
onTrigger,
539-
}: {
540-
reviewState: PRReviewState;
541-
isTriggering: boolean;
542-
onTrigger: () => void;
543-
}) {
552+
function ReviewStateRow({ reviewState }: { reviewState: PRReviewState }) {
544553
const verdict = reviewVerdict(reviewState);
545-
const buttonLabel = reviewRunButtonLabel(reviewState, isTriggering);
546-
const disabled = isTriggering || reviewState.status === "running" || reviewState.status === "ineligible";
547554
const title = reviewState.title?.trim() || `PR #${reviewState.prNumber}`;
548555
return (
549556
<div
@@ -563,10 +570,9 @@ function ReviewStateRow({
563570
<span className="reviewer-row__number">#{reviewState.prNumber}</span>
564571
</div>
565572
</div>
566-
<span className={cn("reviewer-row__verdict", `reviewer-row__verdict--${verdict.tone}`)}>{verdict.label}</span>
567-
<button className="reviewer-row__action" disabled={disabled} onClick={onTrigger} type="button">
568-
{buttonLabel}
569-
</button>
573+
<span className={cn("reviewer-row__verdict", `reviewer-row__verdict--${verdict.tone}`)}>
574+
{verdict.label}
575+
</span>
570576
</div>
571577
);
572578
}
@@ -606,9 +612,14 @@ function reviewVerdict(reviewState: PRReviewState): {
606612
return { label: "Not run", tone: "neutral" };
607613
}
608614

609-
function reviewRunButtonLabel(reviewState: PRReviewState, isTriggering: boolean): string {
610-
if (reviewState.status === "running" || isTriggering) return "Reviewing...";
611-
return reviewState.latestRun ? "Re-run" : "Run";
615+
function reviewSessionRunAction(reviewStates: PRReviewState[], isTriggering: boolean): string {
616+
if (isTriggering || reviewStates.some((reviewState) => reviewState.status === "running")) {
617+
return "Reviewing...";
618+
}
619+
if (reviewStates.some((reviewState) => reviewState.status === "changes_requested" || reviewState.latestRun)) {
620+
return "Re-run review";
621+
}
622+
return "Run review";
612623
}
613624

614625
function BrowserView({

0 commit comments

Comments
 (0)