diff --git a/frontend/src/renderer/components/PRSummaryDisplay.test.tsx b/frontend/src/renderer/components/PRSummaryDisplay.test.tsx new file mode 100644 index 000000000..cc183b174 --- /dev/null +++ b/frontend/src/renderer/components/PRSummaryDisplay.test.tsx @@ -0,0 +1,80 @@ +import { render, screen } from "@testing-library/react"; +import { describe, expect, it } from "vitest"; +import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; +import { PRSummaryParts } from "./PRSummaryDisplay"; + +const summary = (overrides: Partial = {}): SessionPRSummary => ({ + url: "https://github.com/acme/repo/pull/7", + htmlUrl: "https://github.com/acme/repo/pull/7", + number: 7, + title: "Fix dashboard", + state: "open", + provider: "github", + repo: "acme/repo", + author: "ada", + sourceBranch: "fix/dashboard", + targetBranch: "main", + headSha: "abc123", + additions: 10, + deletions: 3, + changedFiles: 2, + ci: { state: "passing", failingChecks: [] }, + review: { decision: "approved", hasUnresolvedHumanComments: false, unresolvedBy: [] }, + mergeability: { state: "mergeable", reasons: [], prUrl: "https://github.com/acme/repo/pull/7" }, + updatedAt: "2026-06-15T00:00:00Z", + observedAt: "2026-06-15T00:00:00Z", + ciObservedAt: "2026-06-15T00:00:00Z", + reviewObservedAt: "2026-06-15T00:00:00Z", + ...overrides, +}); + +describe("PRSummaryParts", () => { + it("counts overflow from the rendered maxLinks limit", () => { + render( + , + ); + + expect(screen.getByText("unit")).toBeInTheDocument(); + expect(screen.getByText("lint")).toBeInTheDocument(); + expect(screen.queryByText("types")).not.toBeInTheDocument(); + expect(screen.getByText("+1 check")).toBeInTheDocument(); + }); + + it("counts overflow beyond helper-truncated links", () => { + render( + , + ); + + expect(screen.getByText("unit")).toBeInTheDocument(); + expect(screen.getByText("lint")).toBeInTheDocument(); + expect(screen.getByText("types")).toBeInTheDocument(); + expect(screen.queryByText("build")).not.toBeInTheDocument(); + expect(screen.getByText("+1 check")).toBeInTheDocument(); + }); +}); diff --git a/frontend/src/renderer/components/PRSummaryDisplay.tsx b/frontend/src/renderer/components/PRSummaryDisplay.tsx index 3eb54f154..bf45f5fc4 100644 --- a/frontend/src/renderer/components/PRSummaryDisplay.tsx +++ b/frontend/src/renderer/components/PRSummaryDisplay.tsx @@ -1,7 +1,7 @@ import { ArrowUpDown, ArrowUpRight } from "lucide-react"; import { Fragment, type ReactNode } from "react"; import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; -import { prAttentionItems, prStatusRows, type PRAttentionLink, type PRDisplayTone } from "../lib/pr-display"; +import { prSummaryParts, type PRDisplayTone, type PRSummaryLink } from "../lib/pr-display"; import { cn } from "../lib/utils"; const toneClass: Record = { @@ -12,20 +12,6 @@ const toneClass: Record = { error: "text-error", }; -export function PRStatusStrip({ className, pr }: { className?: string; pr: SessionPRSummary }) { - return ( -
- {prStatusRows(pr).map((row) => ( - - {row.label}{" "} - {row.value} - {row.detail ? · {row.detail} : null} - - ))} -
- ); -} - export function PRSummaryMeta({ className, leading, @@ -85,56 +71,66 @@ function PRDiffMeta({ pr }: { pr: SessionPRSummary }) { ); } -export function PRAttentionPanel({ +export function PRSummaryParts({ className, interactiveLinks = true, - maxItems = 3, + maxLinks = 3, pr, + variant = "compact", }: { className?: string; interactiveLinks?: boolean; - maxItems?: number; + maxLinks?: number; pr: SessionPRSummary; + variant?: "compact" | "stacked"; }) { - const items = prAttentionItems(pr); - if (items.length === 0) { - return null; - } - const visible = items.slice(0, maxItems); - const extra = items.length - visible.length; + const parts = prSummaryParts(pr); + const stacked = variant === "stacked"; return ( -
-
- Needs attention -
-
- {visible.map((item) => ( -
-
{item.title}
- {item.summary ? ( -
{item.summary}
- ) : null} - {item.links.length > 0 ? ( -
- {item.links.map((link, index) => ( - +
+ {parts.map((part) => { + const links = part.links.slice(0, maxLinks); + const overflowLabel = overflowPartLabel( + (part.linkTotal ?? part.links.length) - links.length, + part.overflowNoun, + ); + return ( +
+
+ {part.label}{" "} + {part.status} + {part.summary ? · {part.summary} : null} +
+ {links.length > 0 || overflowLabel ? ( +
+ {links.map((link, index) => ( + ))} - {item.overflowLabel ? {item.overflowLabel} : null} + {overflowLabel ? {overflowLabel} : null}
) : null}
- ))} - {extra > 0 ?
+{extra} more
: null} -
+ ); + })}
); } -function AttentionLink({ interactive, link }: { interactive: boolean; link: PRAttentionLink }) { +function overflowPartLabel(extra: number, noun?: string): string | undefined { + if (extra <= 0) { + return undefined; + } + return noun ? `+${extra} ${pluralize(noun, extra)}` : `+${extra}`; +} + +function SummaryLink({ interactive, link }: { interactive: boolean; link: PRSummaryLink }) { if (interactive && link.href) { return ( void }) { .filter(Boolean) .join(" · ")}
- - + diff --git a/frontend/src/renderer/components/SessionInspector.tsx b/frontend/src/renderer/components/SessionInspector.tsx index c22b997e9..9365c9379 100644 --- a/frontend/src/renderer/components/SessionInspector.tsx +++ b/frontend/src/renderer/components/SessionInspector.tsx @@ -6,7 +6,7 @@ import { apiClient, apiErrorMessage } from "../lib/api-client"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { formatTimeCompact } from "../lib/format-time"; import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; -import { prBrowserUrl, prStatusRows, sessionPRDisplaySummaries, type PRDisplayTone } from "../lib/pr-display"; +import { prBrowserUrl, sessionPRDisplaySummaries } from "../lib/pr-display"; import type { SessionActivityState, WorkspaceSession } from "../types/workspace"; import { canonicalTrackerIssueId, sortedPRs } from "../types/workspace"; import { BrowserPanelView } from "./BrowserPanel"; @@ -14,7 +14,7 @@ import type { BrowserViewModel } from "../hooks/useBrowserView"; import { Badge } from "./ui/badge"; import { Button } from "./ui/button"; import { cn } from "../lib/utils"; -import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay"; +import { PRSummaryMeta, PRSummaryParts } from "./PRSummaryDisplay"; type ProjectConfig = components["schemas"]["ProjectConfig"]; type PRReviewState = components["schemas"]["PRReviewState"]; @@ -231,40 +231,11 @@ function PRSummaryCard({ pr }: { pr: SessionPRSummary }) {
{pr.title ?
{pr.title}
: null} - - +
); } -function PRStatusStack({ className, pr }: { className?: string; pr: SessionPRSummary }) { - return ( -
- {prStatusRows(pr).map((row) => ( -
- {row.label}{" "} - {row.value} -
- ))} -
- ); -} - -function inspectorStatusToneClass(tone: PRDisplayTone): string { - switch (tone) { - case "success": - return "text-success"; - case "warning": - return "text-warning"; - case "error": - return "text-error"; - case "neutral": - return "text-muted-foreground"; - case "passive": - return "text-passive"; - } -} - type TimelineTone = "now" | "good" | "warn" | "neutral"; function ActivityTimeline({ session }: { session: WorkspaceSession }) { diff --git a/frontend/src/renderer/hooks/useSessionScmSummary.ts b/frontend/src/renderer/hooks/useSessionScmSummary.ts index 5b26eb8fd..cca1a6694 100644 --- a/frontend/src/renderer/hooks/useSessionScmSummary.ts +++ b/frontend/src/renderer/hooks/useSessionScmSummary.ts @@ -1,6 +1,7 @@ import { useQuery } from "@tanstack/react-query"; import type { components } from "../../api/schema"; import { apiClient } from "../lib/api-client"; +import { mockSessionScmSummaries } from "../lib/mock-data"; export type SessionPRSummary = components["schemas"]["SessionPRSummary"]; @@ -20,8 +21,9 @@ export async function fetchSessionScmSummary(sessionId: string): Promise fetchSessionScmSummary(sessionId), + enabled: Boolean(sessionId), + queryFn: () => + usePreviewData ? Promise.resolve(mockSessionScmSummaries[sessionId] ?? []) : fetchSessionScmSummary(sessionId), retry: 1, }; } @@ -29,8 +31,9 @@ export function sessionScmSummaryQueryOptions(sessionId: string) { export function useSessionScmSummary(sessionId?: string) { return useQuery({ queryKey: sessionScmSummaryQueryKey(sessionId), - enabled: Boolean(sessionId) && !usePreviewData, - queryFn: () => fetchSessionScmSummary(sessionId!), + enabled: Boolean(sessionId), + queryFn: () => + usePreviewData ? Promise.resolve(mockSessionScmSummaries[sessionId!] ?? []) : fetchSessionScmSummary(sessionId!), retry: 1, }); } diff --git a/frontend/src/renderer/lib/mock-data.ts b/frontend/src/renderer/lib/mock-data.ts index 5d3af9520..c354ab20d 100644 --- a/frontend/src/renderer/lib/mock-data.ts +++ b/frontend/src/renderer/lib/mock-data.ts @@ -1,4 +1,5 @@ import type { PRState, PullRequestFacts, WorkspaceSummary } from "../types/workspace"; +import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; const now = new Date().toISOString(); const hoursAgo = (hours: number) => new Date(Date.now() - hours * 60 * 60 * 1000).toISOString(); @@ -284,3 +285,199 @@ export const mockWorkspaces: WorkspaceSummary[] = [ ], }, ]; + +const prSummary = (sessionId: string, number: number, overrides: Partial = {}): SessionPRSummary => { + const session = mockWorkspaces.flatMap((workspace) => workspace.sessions).find((item) => item.id === sessionId); + const facts = session?.prs.find((item) => item.number === number); + const url = facts?.url ?? `https://github.com/me/${session?.workspaceName ?? "preview"}/pull/${number}`; + return { + url, + htmlUrl: url, + number, + title: session?.title ?? `PR #${number}`, + state: facts?.state ?? "open", + provider: "github", + repo: `me/${session?.workspaceName ?? "preview"}`, + author: "preview-agent", + sourceBranch: session?.branch ?? "", + targetBranch: "main", + headSha: `preview-${number}`, + additions: 42, + deletions: 8, + changedFiles: 3, + ci: { + state: facts?.ci === "failing" ? "failing" : facts?.ci === "pending" ? "pending" : "passing", + failingChecks: [], + }, + review: { + decision: + facts?.review === "changes_requested" + ? "changes_requested" + : facts?.review === "approved" + ? "approved" + : "none", + hasUnresolvedHumanComments: facts?.reviewComments ?? false, + unresolvedBy: [], + }, + mergeability: { + state: + facts?.mergeability === "conflicting" + ? "conflicting" + : facts?.mergeability === "blocked" + ? "blocked" + : facts?.mergeability === "unstable" + ? "unstable" + : facts?.mergeability === "unknown" + ? "unknown" + : "mergeable", + reasons: [], + prUrl: url, + conflictFiles: [], + }, + updatedAt: facts?.updatedAt ?? now, + observedAt: facts?.updatedAt ?? now, + ciObservedAt: facts?.updatedAt ?? now, + reviewObservedAt: facts?.updatedAt ?? now, + ...overrides, + }; +}; + +export const mockSessionScmSummaries: Record = { + "fix-auth-timeouts": [ + prSummary("fix-auth-timeouts", 184, { + changedFiles: 5, + additions: 91, + deletions: 17, + ci: { + state: "failing", + failingChecks: [ + { + name: "backend / go test ./...", + status: "failed", + conclusion: "failure", + url: "https://github.com/me/api-gateway/actions/runs/184001/job/1", + }, + { + name: "lint / golangci", + status: "failed", + conclusion: "failure", + url: "https://github.com/me/api-gateway/actions/runs/184001/job/2", + }, + { + name: "api contract drift", + status: "failed", + conclusion: "failure", + url: "https://github.com/me/api-gateway/actions/runs/184001/job/3", + }, + { + name: "frontend typecheck", + status: "failed", + conclusion: "", + url: "https://github.com/me/api-gateway/actions/runs/184001/job/4", + }, + ], + }, + }), + ], + "texture-leak": [ + prSummary("texture-leak", 51, { + changedFiles: 4, + additions: 74, + deletions: 22, + ci: { + state: "failing", + failingChecks: [ + { + name: "render tests", + status: "failed", + conclusion: "failure", + url: "https://github.com/me/webgl-preview/actions/runs/51001/job/1", + }, + { + name: "visual regression", + status: "failed", + conclusion: "failure", + url: "https://github.com/me/webgl-preview/actions/runs/51001/job/2", + }, + ], + }, + mergeability: { + state: "conflicting", + reasons: ["conflicts"], + prUrl: "https://github.com/me/webgl-preview/pull/51", + conflictFiles: [ + { + path: "src/render/texture-cache.ts", + url: "https://github.com/me/webgl-preview/pull/51/conflicts#src-render-texture-cache-ts", + }, + { + path: "src/render/webgl-context.ts", + url: "https://github.com/me/webgl-preview/pull/51/conflicts#src-render-webgl-context-ts", + }, + ], + }, + }), + ], + "review-camera-pan": [ + prSummary("review-camera-pan", 52, { + changedFiles: 6, + additions: 128, + deletions: 31, + review: { + decision: "review_required", + hasUnresolvedHumanComments: false, + unresolvedBy: [], + }, + }), + ], + "input-pointer-lock": [ + prSummary("input-pointer-lock", 56, { + changedFiles: 3, + additions: 48, + deletions: 14, + review: { + decision: "changes_requested", + hasUnresolvedHumanComments: true, + unresolvedBy: [ + { + reviewerId: "maya", + count: 3, + reviewUrl: "https://github.com/me/webgl-preview/pull/56#pullrequestreview-1001", + links: [ + { + url: "https://github.com/me/webgl-preview/pull/56#discussion_r1001", + file: "src/input/pointer-lock.ts", + line: 88, + }, + { + url: "https://github.com/me/webgl-preview/pull/56#discussion_r1002", + file: "src/input/keyboard.ts", + line: 41, + }, + ], + }, + { + reviewerId: "copilot", + count: 1, + isBot: true, + reviewUrl: "https://github.com/me/webgl-preview/pull/56#pullrequestreview-1002", + links: [], + }, + ], + }, + }), + ], + "invoice-export": [ + prSummary("invoice-export", 117, { + changedFiles: 8, + additions: 212, + deletions: 36, + mergeability: { + state: "blocked", + reasons: ["behind_base", "review_required", "blocked_by_provider", "ci_failing"], + prUrl: "https://github.com/me/billing-portal/pull/117", + conflictFiles: [], + }, + }), + ], +}; diff --git a/frontend/src/renderer/lib/pr-display.test.ts b/frontend/src/renderer/lib/pr-display.test.ts index 3a904f5ec..119b8b051 100644 --- a/frontend/src/renderer/lib/pr-display.test.ts +++ b/frontend/src/renderer/lib/pr-display.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from "vitest"; import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; -import { prAttentionItems, prBrowserUrl, prDiffSummary, prStatusRows } from "./pr-display"; +import { prBrowserUrl, prDiffSummary, prStatusRows, prSummaryParts } from "./pr-display"; const summary = (overrides: Partial = {}): SessionPRSummary => ({ url: "https://github.com/acme/repo/pull/7", @@ -37,7 +37,7 @@ describe("prStatusRows", () => { }), ); - expect(rows.map((row) => `${row.label}:${row.value}`)).toEqual(["CI:Checking", "Review:None", "Merge:Checking"]); + expect(rows.map((row) => `${row.label}:${row.value}`)).toEqual(["CI:Checking", "Merge:Checking", "Review:None"]); }); it("includes minimal diff detail on the merge row", () => { @@ -69,13 +69,13 @@ describe("prBrowserUrl", () => { }); }); -describe("prAttentionItems", () => { - it("returns no attention for clean open PRs", () => { - expect(prAttentionItems(summary())).toEqual([]); +describe("prSummaryParts", () => { + it("always returns CI, Merge, and Review parts", () => { + expect(prSummaryParts(summary()).map((part) => part.label)).toEqual(["CI", "Merge", "Review"]); }); - it("details active CI, review, and merge blockers", () => { - const items = prAttentionItems( + it("details active CI, merge, and review blockers under their parts", () => { + const parts = prSummaryParts( summary({ ci: { state: "failing", @@ -102,19 +102,34 @@ describe("prAttentionItems", () => { }), ); - expect(items.map((item) => item.kind)).toEqual(["merge_blocked", "ci_failing", "review_changes_requested"]); - expect(items.find((item) => item.kind === "ci_failing")?.links[0]).toMatchObject({ + expect(parts.map((part) => part.key)).toEqual(["ci", "merge", "review"]); + expect(parts.find((part) => part.key === "ci")).toMatchObject({ + status: "Failing", + summary: undefined, + tone: "error", + }); + expect(parts.find((part) => part.key === "ci")?.links[0]).toMatchObject({ label: "copy-check", href: "https://checks.example/copy", }); - expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "merge")).toMatchObject({ + status: "Blocked", + summary: undefined, + tone: "warning", + }); + expect(parts.find((part) => part.key === "review")).toMatchObject({ + status: "Changes requested", + summary: undefined, + tone: "warning", + }); + expect(parts.find((part) => part.key === "review")?.links[0]).toMatchObject({ label: "alice +5", href: "https://github.com/acme/repo/pull/7#discussion_r1", }); }); it("links failing CI checks to their provider URLs", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ ci: { state: "failing", @@ -128,17 +143,17 @@ describe("prAttentionItems", () => { }), ); - const ciItem = items.find((item) => item.kind === "ci_failing"); - expect(ciItem?.links).toEqual([ + const ciPart = parts.find((part) => part.key === "ci"); + expect(ciPart?.links).toEqual([ { label: "unit", href: "https://checks.example/unit", title: "failure" }, { label: "lint", href: "https://checks.example/lint", title: "failure" }, { label: "build", href: "https://checks.example/build", title: "failure" }, ]); - expect(ciItem?.overflowLabel).toBe("+1 check"); + expect(ciPart?.overflowLabel).toBe("+1 check"); }); it("prefers the submitted review summary over inline comments", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ review: { decision: "changes_requested", @@ -158,7 +173,7 @@ describe("prAttentionItems", () => { }), ); - expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "review")?.links[0]).toMatchObject({ label: "alice +1", href: "https://github.com/acme/repo/pull/7#pullrequestreview-1", title: "Open requested-changes review from alice", @@ -166,7 +181,7 @@ describe("prAttentionItems", () => { }); it("falls back to the first inline comment when no review summary exists", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ review: { decision: "changes_requested", @@ -185,7 +200,7 @@ describe("prAttentionItems", () => { }), ); - expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "review")?.links[0]).toMatchObject({ label: "alice +1", href: "https://github.com/acme/repo/pull/7#discussion_r1", title: "2 unresolved comments from alice", @@ -193,7 +208,7 @@ describe("prAttentionItems", () => { }); it("falls back to the PR page when review summary and inline comment URLs are missing", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ url: "https://github.com/acme/repo/issues/7", htmlUrl: "https://github.com/acme/repo/issues/7", @@ -205,7 +220,7 @@ describe("prAttentionItems", () => { }), ); - expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "review")?.links[0]).toMatchObject({ label: "alice", href: "https://github.com/acme/repo/pull/7", title: "Open pull request for alice", @@ -213,7 +228,7 @@ describe("prAttentionItems", () => { }); it("shows bot reviewers with a bot label", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ review: { decision: "changes_requested", @@ -231,7 +246,7 @@ describe("prAttentionItems", () => { }), ); - expect(items.find((item) => item.kind === "review_changes_requested")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "review")?.links[0]).toMatchObject({ label: "copilot bot", href: "https://github.com/acme/repo/pull/7#pullrequestreview-2", title: "Open requested-changes review from copilot bot", @@ -239,7 +254,7 @@ describe("prAttentionItems", () => { }); it("links merge conflicts to GitHub's conflict resolution page", () => { - const items = prAttentionItems( + const parts = prSummaryParts( summary({ url: "https://github.com/acme/repo/issues/7", htmlUrl: "https://github.com/acme/repo/issues/7", @@ -251,22 +266,39 @@ describe("prAttentionItems", () => { }), ); - expect(items.find((item) => item.kind === "merge_conflict")?.links[0]).toMatchObject({ + expect(parts.find((part) => part.key === "merge")).toMatchObject({ + status: "Conflict", + summary: undefined, + }); + expect(parts.find((part) => part.key === "merge")?.links[0]).toMatchObject({ label: "conflicts", href: "https://github.com/acme/repo/pull/7/conflicts", }); }); - it("suppresses attention once the PR is closed or merged", () => { - expect( - prAttentionItems( - summary({ - state: "merged", - ci: { state: "failing", failingChecks: [{ name: "unit", status: "failed", conclusion: "failure" }] }, - review: { decision: "changes_requested", hasUnresolvedHumanComments: true, unresolvedBy: [] }, - mergeability: { state: "conflicting", reasons: ["conflicts"], prUrl: "https://github.com/acme/repo/pull/7" }, - }), - ), - ).toEqual([]); + it("keeps closed or merged PR summaries to the three status parts", () => { + const parts = prSummaryParts( + summary({ + state: "merged", + ci: { state: "failing", failingChecks: [{ name: "unit", status: "failed", conclusion: "failure" }] }, + review: { decision: "changes_requested", hasUnresolvedHumanComments: true, unresolvedBy: [] }, + mergeability: { state: "conflicting", reasons: ["conflicts"], prUrl: "https://github.com/acme/repo/pull/7" }, + }), + ); + + expect(parts).toHaveLength(3); + expect(parts.find((part) => part.key === "merge")?.links).toEqual([]); + expect(parts.find((part) => part.key === "review")?.links).toEqual([]); + }); + + it("puts draft readiness under Review", () => { + const parts = prSummaryParts( + summary({ state: "draft", review: { decision: "none", hasUnresolvedHumanComments: false, unresolvedBy: [] } }), + ); + + expect(parts.find((part) => part.key === "review")).toMatchObject({ + status: "None", + summary: "Draft PR · Not ready for review", + }); }); }); diff --git a/frontend/src/renderer/lib/pr-display.ts b/frontend/src/renderer/lib/pr-display.ts index 7ce2c3f45..da1861df0 100644 --- a/frontend/src/renderer/lib/pr-display.ts +++ b/frontend/src/renderer/lib/pr-display.ts @@ -27,18 +27,23 @@ export type PRStatusRow = { tone: PRDisplayTone; }; -export type PRAttentionLink = { +export type PRSummaryPartKey = "ci" | "review" | "merge"; + +export type PRSummaryLink = { label: string; href?: string; title?: string; }; -export type PRAttentionItem = { - kind: "draft" | "ci_failing" | "review_changes_requested" | "review_pending" | "merge_conflict" | "merge_blocked"; - title: string; +export type PRSummaryPart = { + key: PRSummaryPartKey; + label: string; + status: string; summary?: string; - links: PRAttentionLink[]; + links: PRSummaryLink[]; + linkTotal?: number; overflowLabel?: string; + overflowNoun?: string; tone: PRDisplayTone; }; @@ -103,26 +108,53 @@ function sessionPRFactToSummary(session: WorkspaceSession, pr: PullRequestFacts) } export function prStatusRows(pr: SessionPRSummary): PRStatusRow[] { + return prSummaryParts(pr).map((part) => ({ + key: part.key, + label: part.label, + value: part.status, + detail: part.key === "merge" ? formatDiffSummary(pr) : undefined, + tone: part.tone, + })); +} + +export function prSummaryParts(pr: SessionPRSummary): PRSummaryPart[] { return [ { key: "ci", label: "CI", - value: ciLabel(pr.ci.state), + status: ciLabel(pr.ci.state), + summary: ciSummary(pr), + links: ciLinks(pr), + linkTotal: pr.ci.state === "failing" ? pr.ci.failingChecks.length : 0, + overflowLabel: pr.ci.state === "failing" ? overflowLabel(pr.ci.failingChecks.length, 3, "check") : undefined, + overflowNoun: "check", tone: ciTone(pr.ci.state), }, - { - key: "review", - label: "Review", - value: reviewLabel(pr.review.decision), - tone: reviewTone(pr.review.decision, pr.review.hasUnresolvedHumanComments), - }, { key: "merge", label: "Merge", - value: mergeabilityLabel(pr.mergeability.state), - detail: formatDiffSummary(pr), + status: mergeabilityLabel(pr.mergeability.state), + summary: mergeSummary(pr), + links: mergeLinks(pr), + linkTotal: mergeLinkTotal(pr), + overflowLabel: mergeOverflowLabel(pr), + overflowNoun: mergeOverflowNoun(pr), tone: mergeabilityTone(pr.mergeability.state), }, + { + key: "review", + label: "Review", + status: reviewLabel(pr.review.decision), + summary: reviewSummary(pr), + links: reviewLinks(pr), + linkTotal: reviewLinkTotal(pr), + overflowLabel: + pr.state === "draft" || pr.review.decision === "review_required" + ? undefined + : overflowLabel(pr.review.unresolvedBy.length, 3, "reviewer"), + overflowNoun: "reviewer", + tone: reviewTone(pr.review.decision, pr.review.hasUnresolvedHumanComments), + }, ]; } @@ -138,69 +170,120 @@ export function prDiffSummary(pr: SessionPRSummary): string | undefined { return parts.length > 0 ? parts.join(" · ") : undefined; } -export function prAttentionItems(pr: SessionPRSummary): PRAttentionItem[] { +function ciSummary(pr: SessionPRSummary): string | undefined { + if (pr.ci.state === "failing") { + return pr.ci.failingChecks.length === 0 ? "No failing check link observed" : undefined; + } + return undefined; +} + +function ciLinks(pr: SessionPRSummary): PRSummaryLink[] { + if (pr.ci.state !== "failing") { + return []; + } + return pr.ci.failingChecks.slice(0, 3).map((check) => ({ + label: check.name, + href: check.url || undefined, + title: check.conclusion || check.status, + })); +} + +function reviewSummary(pr: SessionPRSummary): string | undefined { + if (pr.state === "merged" || pr.state === "closed") { + return undefined; + } + if (pr.state === "draft") { + return "Draft PR · Not ready for review"; + } + if (pr.review.decision === "changes_requested" || pr.review.hasUnresolvedHumanComments) { + return reviewLinks(pr).length === 0 ? "Requested changes still active" : undefined; + } + if (pr.review.decision === "review_required") { + return "Required review not submitted"; + } + return undefined; +} + +function reviewLinks(pr: SessionPRSummary): PRSummaryLink[] { + if (pr.state === "merged" || pr.state === "closed" || pr.state === "draft") { + return []; + } + if (pr.review.decision !== "changes_requested" && !pr.review.hasUnresolvedHumanComments) { + return []; + } + const links = pr.review.unresolvedBy.slice(0, 3).map((reviewer) => reviewAttentionLink(pr, reviewer)); + if (links.length === 0 && pr.review.decision === "changes_requested") { + links.push({ label: "PR", href: prBrowserUrl(pr), title: "Open pull request" }); + } + return links; +} + +function mergeSummary(pr: SessionPRSummary): string | undefined { + if (pr.state === "merged" || pr.state === "closed") { + return formatDiffSummary(pr); + } + if (pr.mergeability.state === "conflicting") { + return mergeLinks(pr).length === 0 ? "Conflicts with the base branch" : undefined; + } + if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { + return mergeLinks(pr).length === 0 ? "Provider reports merge is blocked" : undefined; + } + return formatDiffSummary(pr); +} + +function mergeLinks(pr: SessionPRSummary): PRSummaryLink[] { if (pr.state === "merged" || pr.state === "closed") { return []; } - if (pr.state === "draft") { - return [ - { - kind: "draft", - title: "Draft PR", - summary: "Not ready for review", - links: [], - tone: "passive", - }, - ]; - } - - const items: PRAttentionItem[] = []; if (pr.mergeability.state === "conflicting") { - items.push( - mergeAttention(pr, "merge_conflict", "Resolve merge conflict", "Conflicts with the base branch", "error"), - ); - } else if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { - items.push(mergeAttention(pr, "merge_blocked", "Merge blocked", "Provider reports merge is blocked", "warning")); + return mergeAttentionLinks(pr, "merge_conflict"); } - if (pr.ci.state === "failing") { - const links = pr.ci.failingChecks.slice(0, 3).map((check) => ({ - label: check.name, - href: check.url || undefined, - title: check.conclusion || check.status, - })); - items.push({ - kind: "ci_failing", - title: "Fix failing CI", - summary: links.length === 0 ? "No failing check link observed" : undefined, - links, - overflowLabel: overflowLabel(pr.ci.failingChecks.length, 3, "check"), - tone: "error", - }); + if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { + return mergeAttentionLinks(pr, "merge_blocked"); } - if (pr.review.decision === "changes_requested" || pr.review.hasUnresolvedHumanComments) { - const reviewers = pr.review.unresolvedBy.slice(0, 3); - const links = reviewers.map((reviewer) => reviewAttentionLink(pr, reviewer)); - if (links.length === 0 && pr.review.decision === "changes_requested") { - links.push({ label: "PR", href: prBrowserUrl(pr), title: "Open pull request" }); - } - items.push({ - kind: "review_changes_requested", - title: "Address requested changes", - summary: links.length === 0 ? "Requested changes still active" : undefined, - links, - overflowLabel: overflowLabel(pr.review.unresolvedBy.length, 3, "reviewer"), - tone: "warning", - }); - } else if (pr.review.decision === "review_required") { - items.push({ - kind: "review_pending", - title: "Review pending", - summary: "Required review not submitted", - links: [], - tone: "neutral", - }); + return []; +} + +function mergeOverflowLabel(pr: SessionPRSummary): string | undefined { + if (pr.state === "merged" || pr.state === "closed") { + return undefined; } - return items; + const hasFileLinks = (pr.mergeability.conflictFiles ?? []).length > 0; + if (hasFileLinks) { + return overflowLabel(pr.mergeability.conflictFiles?.length ?? 0, 3, "file"); + } + if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { + return overflowLabel(pr.mergeability.reasons.length, 3, "reason"); + } + return undefined; +} + +function mergeLinkTotal(pr: SessionPRSummary): number { + if (pr.state === "merged" || pr.state === "closed") { + return 0; + } + if (pr.mergeability.state === "conflicting") { + const conflictFileCount = pr.mergeability.conflictFiles?.length ?? 0; + return conflictFileCount > 0 ? conflictFileCount : mergeLinks(pr).length; + } + if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") { + return pr.mergeability.reasons.length; + } + return 0; +} + +function mergeOverflowNoun(pr: SessionPRSummary): string { + return (pr.mergeability.conflictFiles ?? []).length > 0 ? "file" : "reason"; +} + +function reviewLinkTotal(pr: SessionPRSummary): number { + if (pr.state === "merged" || pr.state === "closed" || pr.state === "draft") { + return 0; + } + if (pr.review.decision !== "changes_requested" && !pr.review.hasUnresolvedHumanComments) { + return 0; + } + return pr.review.unresolvedBy.length > 0 ? pr.review.unresolvedBy.length : reviewLinks(pr).length; } function toCIState(value: string): SessionPRSummary["ci"]["state"] { @@ -327,13 +410,7 @@ function formatLineDelta(additions: number, deletions: number): string | undefin return parts.length > 0 ? parts.join(" ") : undefined; } -function mergeAttention( - pr: SessionPRSummary, - kind: Extract, - title: string, - fallback: string, - tone: PRDisplayTone, -): PRAttentionItem { +function mergeAttentionLinks(pr: SessionPRSummary, kind: "merge_conflict" | "merge_blocked"): PRSummaryLink[] { const href = kind === "merge_conflict" ? mergeConflictUrl(pr) : pr.mergeability.prUrl || pr.htmlUrl || pr.url || undefined; const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({ @@ -350,18 +427,7 @@ function mergeAttention( })); const fallbackLink = kind === "merge_conflict" && href ? [{ label: "conflicts", href, title: "Open merge conflicts" }] : []; - const links = fileLinks.length > 0 ? fileLinks : reasonLinks.length > 0 ? reasonLinks : fallbackLink; - return { - kind, - title, - summary: links.length === 0 ? fallback : undefined, - links, - overflowLabel: - fileLinks.length > 0 - ? overflowLabel(pr.mergeability.conflictFiles?.length ?? 0, 3, "file") - : overflowLabel(pr.mergeability.reasons.length, 3, "reason"), - tone, - }; + return fileLinks.length > 0 ? fileLinks : reasonLinks.length > 0 ? reasonLinks : fallbackLink; } function mergeConflictUrl(pr: SessionPRSummary): string | undefined { @@ -412,7 +478,7 @@ function reviewerDisplayName(reviewer: SessionPRSummary["review"]["unresolvedBy" function reviewAttentionLink( pr: SessionPRSummary, reviewer: SessionPRSummary["review"]["unresolvedBy"][number], -): PRAttentionLink { +): PRSummaryLink { const inlineURL = reviewer.links.find((link) => link.url)?.url; if (reviewer.reviewUrl) { return {