fix: consolidate PR summary status details (#2406)

* fix: consolidate pr summary status details

* chore: format pr summary changes

* chore: format rebased frontend files

* chore: format generated route tree

* fix: count pr summary overflow by rendered links

* fix: remove duplicate pr summary action copy

* fix: preserve pr summary hidden link totals

* fix: keep board pr footer lifecycle-only
This commit is contained in:
neversettle 2026-07-05 18:50:42 +05:30 committed by GitHub
parent 608333bd54
commit 21294e6481
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
8 changed files with 556 additions and 212 deletions

View File

@ -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> = {}): 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(
<PRSummaryParts
interactiveLinks={false}
maxLinks={2}
pr={summary({
ci: {
state: "failing",
failingChecks: [
{ name: "unit", status: "failed", conclusion: "failure", url: "https://checks.example/unit" },
{ name: "lint", status: "failed", conclusion: "failure", url: "https://checks.example/lint" },
{ name: "types", status: "failed", conclusion: "failure", url: "https://checks.example/types" },
],
},
})}
/>,
);
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(
<PRSummaryParts
interactiveLinks={false}
pr={summary({
ci: {
state: "failing",
failingChecks: [
{ name: "unit", status: "failed", conclusion: "failure", url: "https://checks.example/unit" },
{ name: "lint", status: "failed", conclusion: "failure", url: "https://checks.example/lint" },
{ name: "types", status: "failed", conclusion: "failure", url: "https://checks.example/types" },
{ name: "build", status: "failed", conclusion: "failure", url: "https://checks.example/build" },
],
},
})}
/>,
);
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();
});
});

View File

@ -1,7 +1,7 @@
import { ArrowUpDown, ArrowUpRight } from "lucide-react"; import { ArrowUpDown, ArrowUpRight } from "lucide-react";
import { Fragment, type ReactNode } from "react"; import { Fragment, type ReactNode } from "react";
import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; 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"; import { cn } from "../lib/utils";
const toneClass: Record<PRDisplayTone, string> = { const toneClass: Record<PRDisplayTone, string> = {
@ -12,20 +12,6 @@ const toneClass: Record<PRDisplayTone, string> = {
error: "text-error", error: "text-error",
}; };
export function PRStatusStrip({ className, pr }: { className?: string; pr: SessionPRSummary }) {
return (
<div className={cn("flex flex-wrap gap-x-3 gap-y-1 font-mono text-[10.5px]", className)}>
{prStatusRows(pr).map((row) => (
<span key={row.key} className="min-w-0">
<span className="text-passive">{row.label}</span>{" "}
<span className={cn("font-medium", toneClass[row.tone])}>{row.value}</span>
{row.detail ? <span className="text-passive"> · {row.detail}</span> : null}
</span>
))}
</div>
);
}
export function PRSummaryMeta({ export function PRSummaryMeta({
className, className,
leading, leading,
@ -85,56 +71,66 @@ function PRDiffMeta({ pr }: { pr: SessionPRSummary }) {
); );
} }
export function PRAttentionPanel({ export function PRSummaryParts({
className, className,
interactiveLinks = true, interactiveLinks = true,
maxItems = 3, maxLinks = 3,
pr, pr,
variant = "compact",
}: { }: {
className?: string; className?: string;
interactiveLinks?: boolean; interactiveLinks?: boolean;
maxItems?: number; maxLinks?: number;
pr: SessionPRSummary; pr: SessionPRSummary;
variant?: "compact" | "stacked";
}) { }) {
const items = prAttentionItems(pr); const parts = prSummaryParts(pr);
if (items.length === 0) { const stacked = variant === "stacked";
return null;
}
const visible = items.slice(0, maxItems);
const extra = items.length - visible.length;
return ( return (
<div className={cn("mt-2 border-t border-border pt-2", className)}> <div
<div className="mb-1 font-mono text-[9.5px] font-semibold uppercase tracking-[0.08em] text-passive"> className={cn(
Needs attention stacked
</div> ? "flex flex-col gap-1.5 font-mono text-[10.5px] leading-4"
<div className="flex flex-col gap-1.5"> : "flex flex-wrap gap-x-3 gap-y-1 font-mono text-[10.5px]",
{visible.map((item) => ( className,
<div key={item.kind} className="min-w-0 text-[11px] leading-4"> )}
<div className={cn("font-medium", toneClass[item.tone])}>{item.title}</div> >
{item.summary ? ( {parts.map((part) => {
<div className="truncate font-mono text-[10.5px] text-muted-foreground">{item.summary}</div> const links = part.links.slice(0, maxLinks);
) : null} const overflowLabel = overflowPartLabel(
{item.links.length > 0 ? ( (part.linkTotal ?? part.links.length) - links.length,
<div className="mt-0.5 flex min-w-0 flex-wrap gap-x-1.5 gap-y-1 font-mono text-[10.5px]"> part.overflowNoun,
{item.links.map((link, index) => ( );
<AttentionLink return (
interactive={interactiveLinks} <div key={part.key} className={cn("min-w-0", stacked ? "flex flex-col" : "inline-flex flex-wrap gap-x-1")}>
key={`${item.kind}-${index}-${link.label}`} <div className="min-w-0 truncate">
link={link} <span className="text-passive">{part.label}</span>{" "}
/> <span className={cn("font-medium", toneClass[part.tone])}>{part.status}</span>
{part.summary ? <span className="text-passive"> · {part.summary}</span> : null}
</div>
{links.length > 0 || overflowLabel ? (
<div className={cn("flex min-w-0 flex-wrap gap-x-1.5 gap-y-1", stacked ? "mt-0.5" : "")}>
{links.map((link, index) => (
<SummaryLink interactive={interactiveLinks} key={`${part.key}-${index}-${link.label}`} link={link} />
))} ))}
{item.overflowLabel ? <span className="text-passive">{item.overflowLabel}</span> : null} {overflowLabel ? <span className="text-passive">{overflowLabel}</span> : null}
</div> </div>
) : null} ) : null}
</div> </div>
))} );
{extra > 0 ? <div className="font-mono text-[10.5px] text-passive">+{extra} more</div> : null} })}
</div>
</div> </div>
); );
} }
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) { if (interactive && link.href) {
return ( return (
<a <a

View File

@ -13,7 +13,7 @@ import type { WorkspaceSession } from "../types/workspace";
import { DashboardSubhead } from "./DashboardSubhead"; import { DashboardSubhead } from "./DashboardSubhead";
import { Badge } from "./ui/badge"; import { Badge } from "./ui/badge";
import { Button } from "./ui/button"; import { Button } from "./ui/button";
import { PRAttentionPanel, PRStatusStrip } from "./PRSummaryDisplay"; import { PRSummaryParts } from "./PRSummaryDisplay";
import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "./ui/table"; import { Table, TableBody, TableCell, TableHead, TableHeader, TableRow } from "./ui/table";
import { cn } from "../lib/utils"; import { cn } from "../lib/utils";
@ -145,8 +145,7 @@ function PRRowView({ row, onOpen }: { row: PRRow; onOpen: () => void }) {
.filter(Boolean) .filter(Boolean)
.join(" · ")} .join(" · ")}
</div> </div>
<PRStatusStrip className="mt-1" pr={row.pr} /> <PRSummaryParts className="mt-1" maxLinks={2} pr={row.pr} />
<PRAttentionPanel className="mt-1.5 pt-1.5" maxItems={2} pr={row.pr} />
</TableCell> </TableCell>
<TableCell> <TableCell>
<Badge variant="outline" className={cn("h-5 px-1.5 text-[10px] font-medium", stateTone[row.pr.state])}> <Badge variant="outline" className={cn("h-5 px-1.5 text-[10px] font-medium", stateTone[row.pr.state])}>

View File

@ -6,7 +6,7 @@ import { apiClient, apiErrorMessage } from "../lib/api-client";
import { workspaceQueryKey } from "../hooks/useWorkspaceQuery"; import { workspaceQueryKey } from "../hooks/useWorkspaceQuery";
import { formatTimeCompact } from "../lib/format-time"; import { formatTimeCompact } from "../lib/format-time";
import { useSessionScmSummary, type SessionPRSummary } from "../hooks/useSessionScmSummary"; 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 type { SessionActivityState, WorkspaceSession } from "../types/workspace";
import { canonicalTrackerIssueId, sortedPRs } from "../types/workspace"; import { canonicalTrackerIssueId, sortedPRs } from "../types/workspace";
import { BrowserPanelView } from "./BrowserPanel"; import { BrowserPanelView } from "./BrowserPanel";
@ -14,7 +14,7 @@ import type { BrowserViewModel } from "../hooks/useBrowserView";
import { Badge } from "./ui/badge"; import { Badge } from "./ui/badge";
import { Button } from "./ui/button"; import { Button } from "./ui/button";
import { cn } from "../lib/utils"; import { cn } from "../lib/utils";
import { PRAttentionPanel, PRSummaryMeta } from "./PRSummaryDisplay"; import { PRSummaryMeta, PRSummaryParts } from "./PRSummaryDisplay";
type ProjectConfig = components["schemas"]["ProjectConfig"]; type ProjectConfig = components["schemas"]["ProjectConfig"];
type PRReviewState = components["schemas"]["PRReviewState"]; type PRReviewState = components["schemas"]["PRReviewState"];
@ -231,40 +231,11 @@ function PRSummaryCard({ pr }: { pr: SessionPRSummary }) {
</div> </div>
{pr.title ? <div className="mt-2 text-[12px] font-medium leading-snug text-foreground">{pr.title}</div> : null} {pr.title ? <div className="mt-2 text-[12px] font-medium leading-snug text-foreground">{pr.title}</div> : null}
<PRSummaryMeta className="mt-1.5" pr={pr} /> <PRSummaryMeta className="mt-1.5" pr={pr} />
<PRStatusStack className="mt-2" pr={pr} /> <PRSummaryParts className="mt-2" pr={pr} variant="stacked" />
<PRAttentionPanel pr={pr} />
</div> </div>
); );
} }
function PRStatusStack({ className, pr }: { className?: string; pr: SessionPRSummary }) {
return (
<div className={cn("flex flex-col gap-0.5 font-mono text-[10.5px] leading-4", className)}>
{prStatusRows(pr).map((row) => (
<div key={row.key} className="min-w-0 truncate">
<span className="text-passive">{row.label}</span>{" "}
<span className={cn("font-medium", inspectorStatusToneClass(row.tone))}>{row.value}</span>
</div>
))}
</div>
);
}
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"; type TimelineTone = "now" | "good" | "warn" | "neutral";
function ActivityTimeline({ session }: { session: WorkspaceSession }) { function ActivityTimeline({ session }: { session: WorkspaceSession }) {

View File

@ -1,6 +1,7 @@
import { useQuery } from "@tanstack/react-query"; import { useQuery } from "@tanstack/react-query";
import type { components } from "../../api/schema"; import type { components } from "../../api/schema";
import { apiClient } from "../lib/api-client"; import { apiClient } from "../lib/api-client";
import { mockSessionScmSummaries } from "../lib/mock-data";
export type SessionPRSummary = components["schemas"]["SessionPRSummary"]; export type SessionPRSummary = components["schemas"]["SessionPRSummary"];
@ -20,8 +21,9 @@ export async function fetchSessionScmSummary(sessionId: string): Promise<Session
export function sessionScmSummaryQueryOptions(sessionId: string) { export function sessionScmSummaryQueryOptions(sessionId: string) {
return { return {
queryKey: sessionScmSummaryQueryKey(sessionId), queryKey: sessionScmSummaryQueryKey(sessionId),
enabled: Boolean(sessionId) && !usePreviewData, enabled: Boolean(sessionId),
queryFn: () => fetchSessionScmSummary(sessionId), queryFn: () =>
usePreviewData ? Promise.resolve(mockSessionScmSummaries[sessionId] ?? []) : fetchSessionScmSummary(sessionId),
retry: 1, retry: 1,
}; };
} }
@ -29,8 +31,9 @@ export function sessionScmSummaryQueryOptions(sessionId: string) {
export function useSessionScmSummary(sessionId?: string) { export function useSessionScmSummary(sessionId?: string) {
return useQuery({ return useQuery({
queryKey: sessionScmSummaryQueryKey(sessionId), queryKey: sessionScmSummaryQueryKey(sessionId),
enabled: Boolean(sessionId) && !usePreviewData, enabled: Boolean(sessionId),
queryFn: () => fetchSessionScmSummary(sessionId!), queryFn: () =>
usePreviewData ? Promise.resolve(mockSessionScmSummaries[sessionId!] ?? []) : fetchSessionScmSummary(sessionId!),
retry: 1, retry: 1,
}); });
} }

View File

@ -1,4 +1,5 @@
import type { PRState, PullRequestFacts, WorkspaceSummary } from "../types/workspace"; import type { PRState, PullRequestFacts, WorkspaceSummary } from "../types/workspace";
import type { SessionPRSummary } from "../hooks/useSessionScmSummary";
const now = new Date().toISOString(); const now = new Date().toISOString();
const hoursAgo = (hours: number) => new Date(Date.now() - hours * 60 * 60 * 1000).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> = {}): 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<string, SessionPRSummary[]> = {
"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: [],
},
}),
],
};

View File

@ -1,6 +1,6 @@
import { describe, expect, it } from "vitest"; import { describe, expect, it } from "vitest";
import type { SessionPRSummary } from "../hooks/useSessionScmSummary"; 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> = {}): SessionPRSummary => ({ const summary = (overrides: Partial<SessionPRSummary> = {}): SessionPRSummary => ({
url: "https://github.com/acme/repo/pull/7", 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", () => { it("includes minimal diff detail on the merge row", () => {
@ -69,13 +69,13 @@ describe("prBrowserUrl", () => {
}); });
}); });
describe("prAttentionItems", () => { describe("prSummaryParts", () => {
it("returns no attention for clean open PRs", () => { it("always returns CI, Merge, and Review parts", () => {
expect(prAttentionItems(summary())).toEqual([]); expect(prSummaryParts(summary()).map((part) => part.label)).toEqual(["CI", "Merge", "Review"]);
}); });
it("details active CI, review, and merge blockers", () => { it("details active CI, merge, and review blockers under their parts", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
ci: { ci: {
state: "failing", state: "failing",
@ -102,19 +102,34 @@ describe("prAttentionItems", () => {
}), }),
); );
expect(items.map((item) => item.kind)).toEqual(["merge_blocked", "ci_failing", "review_changes_requested"]); expect(parts.map((part) => part.key)).toEqual(["ci", "merge", "review"]);
expect(items.find((item) => item.kind === "ci_failing")?.links[0]).toMatchObject({ 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", label: "copy-check",
href: "https://checks.example/copy", 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", label: "alice +5",
href: "https://github.com/acme/repo/pull/7#discussion_r1", href: "https://github.com/acme/repo/pull/7#discussion_r1",
}); });
}); });
it("links failing CI checks to their provider URLs", () => { it("links failing CI checks to their provider URLs", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
ci: { ci: {
state: "failing", state: "failing",
@ -128,17 +143,17 @@ describe("prAttentionItems", () => {
}), }),
); );
const ciItem = items.find((item) => item.kind === "ci_failing"); const ciPart = parts.find((part) => part.key === "ci");
expect(ciItem?.links).toEqual([ expect(ciPart?.links).toEqual([
{ label: "unit", href: "https://checks.example/unit", title: "failure" }, { label: "unit", href: "https://checks.example/unit", title: "failure" },
{ label: "lint", href: "https://checks.example/lint", title: "failure" }, { label: "lint", href: "https://checks.example/lint", title: "failure" },
{ label: "build", href: "https://checks.example/build", 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", () => { it("prefers the submitted review summary over inline comments", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
review: { review: {
decision: "changes_requested", 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", label: "alice +1",
href: "https://github.com/acme/repo/pull/7#pullrequestreview-1", href: "https://github.com/acme/repo/pull/7#pullrequestreview-1",
title: "Open requested-changes review from alice", 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", () => { it("falls back to the first inline comment when no review summary exists", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
review: { review: {
decision: "changes_requested", 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", label: "alice +1",
href: "https://github.com/acme/repo/pull/7#discussion_r1", href: "https://github.com/acme/repo/pull/7#discussion_r1",
title: "2 unresolved comments from alice", 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", () => { it("falls back to the PR page when review summary and inline comment URLs are missing", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
url: "https://github.com/acme/repo/issues/7", url: "https://github.com/acme/repo/issues/7",
htmlUrl: "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", label: "alice",
href: "https://github.com/acme/repo/pull/7", href: "https://github.com/acme/repo/pull/7",
title: "Open pull request for alice", title: "Open pull request for alice",
@ -213,7 +228,7 @@ describe("prAttentionItems", () => {
}); });
it("shows bot reviewers with a bot label", () => { it("shows bot reviewers with a bot label", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
review: { review: {
decision: "changes_requested", 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", label: "copilot bot",
href: "https://github.com/acme/repo/pull/7#pullrequestreview-2", href: "https://github.com/acme/repo/pull/7#pullrequestreview-2",
title: "Open requested-changes review from copilot bot", title: "Open requested-changes review from copilot bot",
@ -239,7 +254,7 @@ describe("prAttentionItems", () => {
}); });
it("links merge conflicts to GitHub's conflict resolution page", () => { it("links merge conflicts to GitHub's conflict resolution page", () => {
const items = prAttentionItems( const parts = prSummaryParts(
summary({ summary({
url: "https://github.com/acme/repo/issues/7", url: "https://github.com/acme/repo/issues/7",
htmlUrl: "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", label: "conflicts",
href: "https://github.com/acme/repo/pull/7/conflicts", href: "https://github.com/acme/repo/pull/7/conflicts",
}); });
}); });
it("suppresses attention once the PR is closed or merged", () => { it("keeps closed or merged PR summaries to the three status parts", () => {
expect( const parts = prSummaryParts(
prAttentionItems( summary({
summary({ state: "merged",
state: "merged", ci: { state: "failing", failingChecks: [{ name: "unit", status: "failed", conclusion: "failure" }] },
ci: { state: "failing", failingChecks: [{ name: "unit", status: "failed", conclusion: "failure" }] }, review: { decision: "changes_requested", hasUnresolvedHumanComments: true, unresolvedBy: [] },
review: { decision: "changes_requested", hasUnresolvedHumanComments: true, unresolvedBy: [] }, mergeability: { state: "conflicting", reasons: ["conflicts"], prUrl: "https://github.com/acme/repo/pull/7" },
mergeability: { state: "conflicting", reasons: ["conflicts"], prUrl: "https://github.com/acme/repo/pull/7" }, }),
}), );
),
).toEqual([]); 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",
});
}); });
}); });

View File

@ -27,18 +27,23 @@ export type PRStatusRow = {
tone: PRDisplayTone; tone: PRDisplayTone;
}; };
export type PRAttentionLink = { export type PRSummaryPartKey = "ci" | "review" | "merge";
export type PRSummaryLink = {
label: string; label: string;
href?: string; href?: string;
title?: string; title?: string;
}; };
export type PRAttentionItem = { export type PRSummaryPart = {
kind: "draft" | "ci_failing" | "review_changes_requested" | "review_pending" | "merge_conflict" | "merge_blocked"; key: PRSummaryPartKey;
title: string; label: string;
status: string;
summary?: string; summary?: string;
links: PRAttentionLink[]; links: PRSummaryLink[];
linkTotal?: number;
overflowLabel?: string; overflowLabel?: string;
overflowNoun?: string;
tone: PRDisplayTone; tone: PRDisplayTone;
}; };
@ -103,26 +108,53 @@ function sessionPRFactToSummary(session: WorkspaceSession, pr: PullRequestFacts)
} }
export function prStatusRows(pr: SessionPRSummary): PRStatusRow[] { 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 [ return [
{ {
key: "ci", key: "ci",
label: "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), tone: ciTone(pr.ci.state),
}, },
{
key: "review",
label: "Review",
value: reviewLabel(pr.review.decision),
tone: reviewTone(pr.review.decision, pr.review.hasUnresolvedHumanComments),
},
{ {
key: "merge", key: "merge",
label: "Merge", label: "Merge",
value: mergeabilityLabel(pr.mergeability.state), status: mergeabilityLabel(pr.mergeability.state),
detail: formatDiffSummary(pr), summary: mergeSummary(pr),
links: mergeLinks(pr),
linkTotal: mergeLinkTotal(pr),
overflowLabel: mergeOverflowLabel(pr),
overflowNoun: mergeOverflowNoun(pr),
tone: mergeabilityTone(pr.mergeability.state), 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; 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") { if (pr.state === "merged" || pr.state === "closed") {
return []; 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") { if (pr.mergeability.state === "conflicting") {
items.push( return mergeAttentionLinks(pr, "merge_conflict");
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"));
} }
if (pr.ci.state === "failing") { if (pr.mergeability.state === "blocked" || pr.mergeability.state === "unstable") {
const links = pr.ci.failingChecks.slice(0, 3).map((check) => ({ return mergeAttentionLinks(pr, "merge_blocked");
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.review.decision === "changes_requested" || pr.review.hasUnresolvedHumanComments) { return [];
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") { function mergeOverflowLabel(pr: SessionPRSummary): string | undefined {
links.push({ label: "PR", href: prBrowserUrl(pr), title: "Open pull request" }); if (pr.state === "merged" || pr.state === "closed") {
} return undefined;
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 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"] { 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; return parts.length > 0 ? parts.join(" ") : undefined;
} }
function mergeAttention( function mergeAttentionLinks(pr: SessionPRSummary, kind: "merge_conflict" | "merge_blocked"): PRSummaryLink[] {
pr: SessionPRSummary,
kind: Extract<PRAttentionItem["kind"], "merge_conflict" | "merge_blocked">,
title: string,
fallback: string,
tone: PRDisplayTone,
): PRAttentionItem {
const href = const href =
kind === "merge_conflict" ? mergeConflictUrl(pr) : pr.mergeability.prUrl || pr.htmlUrl || pr.url || undefined; kind === "merge_conflict" ? mergeConflictUrl(pr) : pr.mergeability.prUrl || pr.htmlUrl || pr.url || undefined;
const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({ const fileLinks = (pr.mergeability.conflictFiles ?? []).slice(0, 3).map((file) => ({
@ -350,18 +427,7 @@ function mergeAttention(
})); }));
const fallbackLink = const fallbackLink =
kind === "merge_conflict" && href ? [{ label: "conflicts", href, title: "Open merge conflicts" }] : []; kind === "merge_conflict" && href ? [{ label: "conflicts", href, title: "Open merge conflicts" }] : [];
const links = fileLinks.length > 0 ? fileLinks : reasonLinks.length > 0 ? reasonLinks : fallbackLink; return 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,
};
} }
function mergeConflictUrl(pr: SessionPRSummary): string | undefined { function mergeConflictUrl(pr: SessionPRSummary): string | undefined {
@ -412,7 +478,7 @@ function reviewerDisplayName(reviewer: SessionPRSummary["review"]["unresolvedBy"
function reviewAttentionLink( function reviewAttentionLink(
pr: SessionPRSummary, pr: SessionPRSummary,
reviewer: SessionPRSummary["review"]["unresolvedBy"][number], reviewer: SessionPRSummary["review"]["unresolvedBy"][number],
): PRAttentionLink { ): PRSummaryLink {
const inlineURL = reviewer.links.find((link) => link.url)?.url; const inlineURL = reviewer.links.find((link) => link.url)?.url;
if (reviewer.reviewUrl) { if (reviewer.reviewUrl) {
return { return {