From 5752fd37f584aa644acaacecdcd05e9a1b04b44e Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 17:54:46 +0200 Subject: [PATCH 01/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20themed=20stats=20o?= =?UTF-8?q?verview=20and=20live=20top-right=20review=20badge?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Summary-first /pair-stats overlay with detail and refresh controls, a non-capturing top-right status badge that survives hidden footers, and a quieter accepted-finding layout. --- README.md | 4 +- package-lock.json | 38 +++++++ package.json | 1 + src/index.ts | 51 +++++++--- src/stats-view.ts | 134 ++++++++++++++++++++---- src/status-overlay.ts | 105 +++++++++++++++++++ test/extension.test.ts | 103 ++++++++++++++----- test/stats-view.test.ts | 166 +++++++++++++++++++++++++----- test/status-overlay.test.ts | 197 ++++++++++++++++++++++++++++++++++++ 9 files changed, 716 insertions(+), 83 deletions(-) create mode 100644 src/status-overlay.ts create mode 100644 test/status-overlay.test.ts diff --git a/README.md b/README.md index 67d7ae1..8ec4a92 100644 --- a/README.md +++ b/README.md @@ -65,5 +65,7 @@ The default reviewer uses your current model and asks: “Does it add entropy? Use `current` or a `provider/model` identifier. File patterns are relative to your working directory; exclusions take precedence. Add entries for more reviewers. -- `/pair-stats`: view review activity, findings, tokens, and estimated costs on demand; missing usage stays unknown. +A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. + +- `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. - Diagnostics stay in files, never the terminal: OMP uses its native logs; Pi uses `~/.pi/agent/logs/pair-programmer/` (or `$PI_CODING_AGENT_DIR/logs/pair-programmer/`). diff --git a/package-lock.json b/package-lock.json index fa52a5f..3ba7d5f 100644 --- a/package-lock.json +++ b/package-lock.json @@ -9,6 +9,7 @@ "version": "0.1.0", "license": "MIT", "dependencies": { + "@earendil-works/pi-tui": "^0.87.1", "@typesafe-ai/sdk": "^0.6.0", "minimatch": "^10.2.6", "pino": "^10.3.1", @@ -2175,6 +2176,19 @@ "url": "https://github.com/sponsors/eemeli" } }, + "node_modules/@earendil-works/pi-tui": { + "version": "0.87.1", + "resolved": "https://registry.npmjs.org/@earendil-works/pi-tui/-/pi-tui-0.87.1.tgz", + "integrity": "sha512-YEH2vRyOeiO7hhN6j6AE6YwKSq2Kz2f3XR8bj1TbR+aGE/JsnY1hLPMI2pvaZfRM1n9Y00tejxFQ4zbzvF7nkQ==", + "license": "MIT", + "dependencies": { + "get-east-asian-width": "1.6.0", + "marked": "18.0.11" + }, + "engines": { + "node": ">=22.19.0" + } + }, "node_modules/@emnapi/core": { "version": "1.10.0", "resolved": "https://registry.npmjs.org/@emnapi/core/-/core-1.10.0.tgz", @@ -4444,6 +4458,18 @@ "dev": true, "license": "MIT" }, + "node_modules/get-east-asian-width": { + "version": "1.6.0", + "resolved": "https://registry.npmjs.org/get-east-asian-width/-/get-east-asian-width-1.6.0.tgz", + "integrity": "sha512-QRbvDIbx6YklUe6RxeTeleMR0yv3cYH6PsPZHcnVn7xv7zO1BHN8r0XETu8n6Ye3Q+ahtSarc3WgtNWmehIBfA==", + "license": "MIT", + "engines": { + "node": ">=18" + }, + "funding": { + "url": "https://github.com/sponsors/sindresorhus" + } + }, "node_modules/get-tsconfig": { "version": "4.14.3", "resolved": "https://registry.npmjs.org/get-tsconfig/-/get-tsconfig-4.14.3.tgz", @@ -5033,6 +5059,18 @@ "source-map-js": "^1.2.1" } }, + "node_modules/marked": { + "version": "18.0.11", + "resolved": "https://registry.npmjs.org/marked/-/marked-18.0.11.tgz", + "integrity": "sha512-HnslJfsZkRPBDJRHvVtAaWlZHEpSu7u8LgQuJCELjRKuWR+hpq4A7sLq3p8HaI9ypVoXDXxV34CsQJEe1+J5Aw==", + "license": "MIT", + "bin": { + "marked": "bin/marked.js" + }, + "engines": { + "node": ">= 20" + } + }, "node_modules/mdn-data": { "version": "2.35.0", "resolved": "https://registry.npmjs.org/mdn-data/-/mdn-data-2.35.0.tgz", diff --git a/package.json b/package.json index 78a56bc..be6166a 100644 --- a/package.json +++ b/package.json @@ -63,6 +63,7 @@ "node": ">=22.19.0" }, "dependencies": { + "@earendil-works/pi-tui": "^0.87.1", "@typesafe-ai/sdk": "^0.6.0", "minimatch": "^10.2.6", "pino": "^10.3.1", diff --git a/src/index.ts b/src/index.ts index 794086f..e1863ac 100644 --- a/src/index.ts +++ b/src/index.ts @@ -21,7 +21,8 @@ import { type PairStats, type ReviewOutcome, } from "./pair-stats.js"; -import { StatsView, statsLines } from "./stats-view.js"; +import { StatsView, statsLines, statsSummary } from "./stats-view.js"; +import { StatusOverlay, type StatusTone } from "./status-overlay.js"; import { isInherited, reviewFile, @@ -131,7 +132,7 @@ function describe(findings: readonly Finding[]): string { } function acceptedReview(finding: Finding, reason: string): string { - return `### Accepted review: ${finding.title}\n\n**${finding.file}:${String(finding.line)}**\n\n${finding.evidence}\n\n**Reason:** ${reason}`; + return `**Accepted review · ${finding.title}**\n\n\`${finding.file}:${String(finding.line)}\`\n\n${finding.evidence}\n\n**Why accepted:** ${reason}`; } export default function pairProgrammer(pi: ExtensionAPI): void { @@ -143,6 +144,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { logger = await createPairLogger(pi); })(); const statsView = new StatsView(); + const statusOverlay = new StatusOverlay(); const accounting = new SessionAccounting(); let activeContext: ExtensionContext | undefined; const cancelJobs = new Map void>(); @@ -165,11 +167,26 @@ export default function pairProgrammer(pi: ExtensionAPI): void { let resetTimer: NodeJS.Timeout | undefined; let checkReset: (() => void) | undefined; - function showState(ctx: ExtensionContext): void { - ctx.ui.setStatus( - "pair-programmer", - `Pair Programmer: ${store.enabled ? "on" : "off"}`, - ); + function showState(ctx = activeContext): void { + if (ctx === undefined) return; + const findings = store.summary(); + const running = accounting.stats.snapshot().reviews.running; + let tone: StatusTone = "success"; + let state = "watching"; + if (!store.enabled) { + tone = "dim"; + state = "paused"; + } else if (findings.outstanding > 0) { + tone = "warning"; + state = `${String(findings.outstanding)} awaiting decision`; + } else if (running > 0) { + tone = "accent"; + state = `reviewing · ${String(running)}`; + } else if (findings.pending > 0) { + tone = "accent"; + state = `${String(findings.pending)} queued`; + } + statusOverlay.show(ctx, { tone, text: state }); } function stopWatchingResets(): void { @@ -259,6 +276,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { ); const ids = batch.map((finding) => finding.id); store.deliver(ids); + showState(); logger?.log("delivery.sent", { sessionId: accounting.stats.sessionId, count: ids.length, @@ -378,6 +396,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { finish("cancelled", false); }); job.stats.start(job.id); + showState(job.ctx); logger?.log("review.started", fields); void (async () => { try { @@ -394,6 +413,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { accounting.retain(job.stats); controllers.delete(controller); cancelJobs.delete(controller); + if (job.session === generation) showState(job.ctx); } })(); } @@ -738,11 +758,13 @@ export default function pairProgrammer(pi: ExtensionAPI): void { pi.on("session_before_switch", beforeSessionChange); pi.on("session_before_fork", beforeSessionChange); pi.on("session_before_tree", beforeSessionChange); + // SAFETY: OMP emits this event; Pi's event map omits it and never calls the handler. const onOmpBeforeBranch = pi.on.bind(pi) as unknown as ( name: "session_before_branch", handler: () => void, ) => void; onOmpBeforeBranch("session_before_branch", beforeSessionChange); + // SAFETY: OMP emits these session events with Pi-compatible contexts; Pi never calls them. const onOmpSession = pi.on.bind(pi) as unknown as ( name: "session_switch" | "session_branch", handler: ( @@ -760,7 +782,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { pi.on("session_shutdown", () => { stopWatchingResets(); stop("shutdown"); - activeContext?.ui.setStatus("pair-programmer", undefined); + statusOverlay.dispose(); activeContext = undefined; accounting.suspend(); replaceBaseline(undefined); @@ -882,6 +904,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { params.decision, params.reason, ); + showState(); logger?.log("finding.verdict", { sessionId: accounting.stats.sessionId, outcome: saved ? params.decision : "invalid", @@ -933,6 +956,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { checkReset?.(); stop("cleared"); store.clear(); + showState(ctx); ctx.ui.notify("Pair Programmer reviews cleared.", "info"); return Promise.resolve(); }, @@ -943,10 +967,13 @@ export default function pairProgrammer(pi: ExtensionAPI): void { "Show on-demand Pair Programmer activity, findings and extension usage", handler: (_args, ctx) => { checkReset?.(); - return statsView.open( - ctx, - statsLines(accounting.stats.snapshot(), store), - ); + return statsView.open(ctx, () => { + const snapshot = accounting.stats.snapshot(); + return { + summary: statsSummary(snapshot, store), + details: statsLines(snapshot, store), + }; + }); }, }); } diff --git a/src/stats-view.ts b/src/stats-view.ts index 2fb2d99..26ee173 100644 --- a/src/stats-view.ts +++ b/src/stats-view.ts @@ -1,4 +1,9 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { + truncateToWidth, + visibleWidth, + wrapTextWithAnsi, +} from "@earendil-works/pi-tui"; import type { MeasuredTotal, StatsSnapshot } from "./pair-stats.js"; import type { ReviewStore } from "./review-store.js"; @@ -29,8 +34,7 @@ export function statsLines( const review = stats.reviews; const findings = store.summary(); const lines = [ - `Pair Programmer statistics - ${store.enabled ? "on" : "off"}`, - "Snapshot on open; close and run /pair-stats again to refresh.", + `Background review: ${store.enabled ? "on" : "off"}`, "", "Review activity: incurred in this session, across all branches", `Running ${String(review.running)} | completed ${String(review.success)} | failed ${String(review.failed)}`, @@ -82,6 +86,48 @@ export function statsLines( return lines; } +export function statsSummary( + stats: StatsSnapshot, + store: Pick, +): string[] { + const review = stats.reviews; + const findings = store.summary(); + const cost = stats.usage.reduce( + (total, group) => ({ + value: total.value + group.costUsd.value, + measured: total.measured + group.costUsd.measured, + }), + { value: 0, measured: 0 }, + ); + const calls = stats.usage.reduce((total, group) => total + group.calls, 0); + return [ + store.enabled ? "Watching your changes" : "Background review is paused", + "", + "REVIEWS / this session, all branches", + `${String(review.success)} completed · ${String(review.running)} running`, + `${String(review.failed + review.timeout)} failed or timed out · ${String(review.interrupted)} interrupted`, + "", + "FINDINGS / selected branch", + `${String(findings.accepted)} accepted · ${String(findings.rejected)} rejected`, + `${String(findings.outstanding)} awaiting decision · ${String(findings.pending)} queued`, + "", + "USAGE / extension only", + `Estimated cost ${measured(cost, calls, true, stats.incompleteJobs === 0)}`, + `${String(calls)} recorded model calls · USD estimate, not a bill`, + ...(stats.incompleteJobs > 0 + ? ["! Accounting is incomplete; missing usage is not zero."] + : []), + ...(stats.persistenceFailures > 0 || stats.pendingWrites > 0 + ? ["! Some records could not be saved. See details."] + : []), + ]; +} + +export interface StatsReport { + summary: readonly string[]; + details: readonly string[]; +} + export class StatsView { private closeCurrent: (() => void) | undefined; @@ -90,7 +136,7 @@ export class StatsView { this.closeCurrent = undefined; } - async open(ctx: ExtensionContext, lines: readonly string[]): Promise { + async open(ctx: ExtensionContext, read: () => StatsReport): Promise { this.close(); const unavailable = "/pair-stats requires an interactive terminal (TUI)."; if (!ctx.hasUI) throw new Error(unavailable); @@ -104,8 +150,10 @@ export class StatsView { let close: (() => void) | undefined; try { await ctx.ui.custom( - (tui, _theme, keys, done) => { + (tui, theme, keys, done) => { state.mounted = true; + let report = read(); + let detailed = false; let offset = 0; let pageSize = 1; let disposed = false; @@ -117,23 +165,65 @@ export class StatsView { this.closeCurrent = close; return { render(width: number): string[] { - const wrapped: string[] = []; const columns = Math.max(1, width); - for (const line of lines) { - if (line.length === 0) wrapped.push(""); - for (let index = 0; index < line.length; index += columns) - wrapped.push(line.slice(index, index + columns)); - } - pageSize = Math.max(1, tui.terminal.rows - 4); + const framed = columns >= 12; + const inner = Math.max(1, columns - (framed ? 6 : 0)); + const lines = detailed ? report.details : report.summary; + const wrapped = lines.flatMap((line) => { + let tone: "warning" | "accent" | "text" = "text"; + if (line.startsWith("!")) tone = "warning"; + else if (/^[A-Z]+ {2}\//u.test(line)) tone = "accent"; + return wrapTextWithAnsi(line, inner).map((part) => + theme.fg(tone, part), + ); + }); + const compact = tui.terminal.rows < 8; + pageSize = Math.max( + 1, + compact + ? tui.terminal.rows + : Math.floor(tui.terminal.rows * 0.8) - 7, + ); offset = Math.min(offset, Math.max(0, wrapped.length - pageSize)); - return [ - ...wrapped.slice(offset, offset + pageSize), - "", - "Up/Down/PgUp/PgDn scroll | Enter/Esc/q close".slice( - 0, - columns, + if (compact) return wrapped.slice(offset, offset + pageSize); + const row = (text: string): string => { + const clipped = truncateToWidth(text, inner, "…"); + return framed + ? theme.fg("borderMuted", "│") + + " " + + clipped + + " ".repeat(Math.max(0, inner - visibleWidth(clipped))) + + " " + + theme.fg("borderMuted", "│") + : clipped; + }; + const title = theme.bold(theme.fg("accent", "Pair Programmer")); + const position = `${String(offset + 1)}–${String(Math.min(offset + pageSize, wrapped.length))}/${String(wrapped.length)}`; + const mode = detailed ? "overview" : "details"; + const hint = + inner >= 52 + ? `↑↓ scroll d ${mode} r refresh esc close ${position}` + : `↑↓ d ${mode} r refresh q close`; + const body = [ + row(title), + row( + theme.fg( + "dim", + detailed ? "Detailed accounting" : "Session overview", + ), ), + row(""), + ...wrapped.slice(offset, offset + pageSize).map(row), + row(""), + row(theme.fg("dim", hint)), ]; + return framed + ? [ + theme.fg("borderMuted", `╭${"─".repeat(columns - 2)}╮`), + ...body, + theme.fg("borderMuted", `╰${"─".repeat(columns - 2)}╯`), + ] + : body; }, handleInput(data: string): void { if ( @@ -144,7 +234,13 @@ export class StatsView { ) close?.(); else { - if (keys.matches(data, "tui.select.up")) + if (data === "d") { + detailed = !detailed; + offset = 0; + } else if (data === "r") { + report = read(); + offset = 0; + } else if (keys.matches(data, "tui.select.up")) offset = Math.max(0, offset - 1); else if (keys.matches(data, "tui.select.down")) offset += 1; else if (keys.matches(data, "tui.select.pageUp")) @@ -162,7 +258,7 @@ export class StatsView { }, }; }, - { overlay: true }, + { overlay: true, overlayOptions: { width: 78 } }, ); if (!state.mounted) throw new Error(unavailable); } finally { diff --git a/src/status-overlay.ts b/src/status-overlay.ts new file mode 100644 index 0000000..15b34c5 --- /dev/null +++ b/src/status-overlay.ts @@ -0,0 +1,105 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { truncateToWidth, visibleWidth } from "@earendil-works/pi-tui"; + +export type StatusTone = "dim" | "accent" | "warning" | "success"; +export interface StatusState { + tone: StatusTone; + text: string; +} + +const KEY = "pair-programmer"; + +interface Painter { + fg(color: StatusTone | "muted", text: string): string; +} + +function styled(theme: Painter, state: StatusState): string { + const label = "pair · " + state.text; + return `${theme.fg(state.tone, "◆")} ${theme.fg("muted", label)}`; +} + +/** Permanent, non-focusable top-right badge; widget fallback outside Pi's TUI. */ +export class StatusOverlay { + private state: StatusState = { tone: "success", text: "watching" }; + private ctx: ExtensionContext | undefined; + private close: (() => void) | undefined; + private requestRender: (() => void) | undefined; + + show(ctx: ExtensionContext, state: StatusState): void { + this.state = state; + if (ctx !== this.ctx) this.mount(ctx); + if (this.close === undefined) this.widget(); + else this.requestRender?.(); + } + + dispose(): void { + this.close?.(); + this.close = undefined; + this.requestRender = undefined; + this.ctx?.ui.setWidget(KEY, undefined); + this.ctx = undefined; + } + + private label(): string { + return `◆ pair · ${this.state.text}`; + } + + private widget(): void { + const ui = this.ctx?.ui; + // SAFETY: OMP and test hosts may omit Pi's theme; plain text remains valid. + const theme = (ui as Partial | undefined)?.theme; + ui?.setWidget(KEY, [ + theme === undefined ? this.label() : styled(theme, this.state), + ]); + } + + private mount(ctx: ExtensionContext): void { + this.dispose(); + this.ctx = ctx; + if (!ctx.hasUI || (ctx as Partial).mode !== "tui") return; + let open = true; + let mounted = false; + this.close = () => { + open = false; + }; + const fallback = (): void => { + if (mounted || this.ctx !== ctx) return; + this.close = undefined; + this.widget(); + }; + void Promise.resolve( + ctx.ui.custom( + (tui, theme, _keys, done) => { + mounted = true; + this.requestRender = () => { + tui.requestRender(); + }; + const finish = (): void => { + done(undefined); + }; + if (open) this.close = finish; + else queueMicrotask(finish); + return { + render: (width: number): string[] => { + const body = " " + styled(theme, this.state) + " "; + return [truncateToWidth(body, Math.max(1, width), "…")]; + }, + invalidate(): void { + return; + }, + }; + }, + { + overlay: true, + overlayOptions: () => ({ + anchor: "top-right", + width: visibleWidth(this.label()) + 2, + margin: { top: 0, right: 1 }, + nonCapturing: true, + visible: (columns: number) => columns >= 40, + }), + }, + ), + ).then(fallback, fallback); + } +} diff --git a/test/extension.test.ts b/test/extension.test.ts index 3649eb8..d13674f 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -212,7 +212,8 @@ async function setup( sendMessage: Mock; isIdle: Mock<() => boolean>; notify: ReturnType; - setStatus: Mock; + setWidget: Mock<(key: string, content: string[] | undefined) => void>; + status: () => string | undefined; entries: JournalEntry[]; statsEntries: unknown[]; activateStatsJournal: (sessionId: string) => unknown[]; @@ -263,7 +264,8 @@ async function setup( let entryError: Error | undefined; let entryAttempts = 0; const notify = vi.fn(); - const setStatus = vi.fn(); + const setWidget = + vi.fn<(key: string, content: string[] | undefined) => void>(); const sendMessage = vi.fn(); const isIdle = vi.fn<() => boolean>(() => false); let decide: DecisionTool["execute"] | undefined; @@ -316,7 +318,7 @@ async function setup( }, hasUI: true, mode: host === "pi" ? "tui" : undefined, - ui: { notify, custom, setStatus }, + ui: { notify, custom, setWidget }, } as unknown as ExtensionContext; const emit = async (name: string, event: unknown = {}): Promise => { const handler = hooks.get(name); @@ -338,7 +340,8 @@ async function setup( sendMessage, isIdle, notify, - setStatus, + setWidget, + status: () => setWidget.mock.lastCall?.[1]?.[0], entries, statsEntries, activateStatsJournal: (sessionId: string) => { @@ -408,11 +411,16 @@ async function openStatistics(environment: { terminal: { rows: 100 }, requestRender: vi.fn(), } as unknown as Parameters[0], - {} as Parameters[1], - {} as Parameters[2], + { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, + } as unknown as Parameters[1], + { matches: () => false } as unknown as Parameters[2], vi.fn(), ); lines = component.render(240); + component.handleInput?.("d"); + lines = [...lines, ...component.render(240)]; component.dispose?.(); }, ); @@ -488,7 +496,11 @@ it.each(["success", "failed", "timeout", "cancelled"] as const)( costUsd: { value: 0.003, measured: 1 }, }); expect(environment.notify).not.toHaveBeenCalled(); - expect(environment.custom).not.toHaveBeenCalled(); + expect( + environment.custom.mock.calls.every( + ([, options]) => options?.overlayOptions !== undefined, + ), + ).toBe(true); expect(environment.sendMessage).not.toHaveBeenCalled(); expect(JSON.stringify(lifecycleLog.mock.calls)).not.toContain( "private-file", @@ -567,29 +579,17 @@ it("keeps attribution fail-open while recording its failed call separately from it("shows the current review state across toggles and session restoration", async () => { const environment = await setup(); - expect(environment.setStatus).toHaveBeenLastCalledWith( - "pair-programmer", - "Pair Programmer: on", - ); + expect(environment.status()).toBe("◆ pair · watching"); await environment.command("pair-programmer"); - expect(environment.setStatus).toHaveBeenLastCalledWith( - "pair-programmer", - "Pair Programmer: off", - ); + expect(environment.status()).toBe("◆ pair · paused"); await environment.emit("session_start", { reason: "startup" }); - expect(environment.setStatus).toHaveBeenLastCalledWith( - "pair-programmer", - "Pair Programmer: off", - ); + expect(environment.status()).toBe("◆ pair · paused"); await environment.command("pair-programmer"); - expect(environment.setStatus).toHaveBeenLastCalledWith( - "pair-programmer", - "Pair Programmer: on", - ); + expect(environment.status()).toBe("◆ pair · watching"); await environment.emit("session_shutdown"); - expect(environment.setStatus).toHaveBeenLastCalledWith( + expect(environment.setWidget).toHaveBeenLastCalledWith( "pair-programmer", undefined, ); @@ -2705,6 +2705,61 @@ it("starts fresh reviews after on even when an aborted reviewer never settles", await environment.emit("session_shutdown"); }); +it("styles the status widget with the active host theme", async () => { + const environment = await setup(); + Object.assign(environment.ctx.ui, { + theme: { fg: (color: string, text: string) => `<${color}>${text}` }, + }); + await environment.command("pair-programmer"); + expect(environment.status()).toBe("\u{25C6} pair \u{B7} paused"); + await environment.command("pair-programmer"); + expect(environment.status()).toBe( + "\u{25C6} pair \u{B7} watching", + ); + await environment.emit("session_shutdown"); +}); + +it("narrates review progress above the editor without surfacing finding contents", async () => { + const environment = await setup(); + const file = path.join(environment.cwd, "change.ts"); + await fsPromises.writeFile(file, "export const broken = true;\n"); + const pending = Promise.withResolvers(); + vi.mocked(reviewFile).mockReturnValue(pending.promise); + await environment.emit("tool_result", { + toolName: "write", + input: { path: file }, + isError: false, + }); + await advanceReviews(() => { + expect(environment.status()).toBe("◆ pair · reviewing · 2"); + }); + pending.resolve([ + { line: 1, title: "Secret title", quote: "broken", evidence: "Issue" }, + ]); + await advanceReviews(() => { + expect(environment.status()).toBe("◆ pair · 2 queued"); + }); + await environment.emit("turn_end"); + expect(environment.status()).toBe("◆ pair · 2 awaiting decision"); + for (const finding of findings(environment.entries)) + await environment.decide(finding.id, { + findingId: finding.id, + decision: "reject", + reason: "Intentional", + }); + expect(environment.status()).toBe("◆ pair · watching"); + await environment.emit("session_shutdown"); + await environment.decide("late", { + findingId: "late", + decision: "reject", + reason: "After shutdown", + }); + expect(environment.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); +}); + it("restores delivered decisions after session start and removes decided findings from model context", async () => { const environment = await setup(); const file = path.join(environment.cwd, "change.ts"); diff --git a/test/stats-view.test.ts b/test/stats-view.test.ts index 4242f26..d699e60 100644 --- a/test/stats-view.test.ts +++ b/test/stats-view.test.ts @@ -1,8 +1,21 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { visibleWidth } from "@earendil-works/pi-tui"; import { describe, expect, it, vi } from "vitest"; import { PairStats } from "../src/pair-stats.js"; import { ReviewStore } from "../src/review-store.js"; -import { StatsView, statsLines } from "../src/stats-view.js"; +import { + StatsView, + statsLines, + statsSummary, + type StatsReport, +} from "../src/stats-view.js"; + +const report = + (lines: readonly string[]): (() => StatsReport) => + () => ({ + summary: lines, + details: [`detail ${lines.join(" ")}`], + }); type Factory = Parameters[0]; interface ViewComponent { @@ -11,7 +24,10 @@ interface ViewComponent { invalidate(): void; dispose?(): void; } -function ui(): { +function ui( + rows = 20, + fg = (_color: string, text: string): string => text, +): { ctx: ExtensionContext; component: () => ViewComponent; mounted: Promise; @@ -26,10 +42,13 @@ function ui(): { const custom = async (factory: Factory): Promise => { component = await factory( { - terminal: { rows: 9 }, + terminal: { rows }, requestRender, } as unknown as Parameters[0], - {} as Parameters[1], + { + fg, + bold: (text: string) => text, + } as unknown as Parameters[1], { matches(data: string, action: string): boolean { const bindings: Record = { @@ -72,6 +91,34 @@ function ui(): { const emptyStore = new ReviewStore(vi.fn()); describe("statistics presentation", () => { + it("summarizes branch findings and measured session cost without hiding incomplete accounting", () => { + const stats = new PairStats("session", () => { + throw new Error("disk full"); + }); + expect(statsSummary(stats.snapshot(), emptyStore).join("\n")).toContain( + "Estimated cost unavailable", + ); + stats.start("job"); + stats.observe("job", { + stage: "review", + requestedModel: "model", + outcome: "success", + durationMs: 1, + usage: { costUsd: 0.002 }, + }); + const store = new ReviewStore(vi.fn()); + store.setEnabled(false); + const text = statsSummary(stats.snapshot(), store).join("\n"); + expect(text).toContain("Background review is paused"); + expect(text).toContain("$0.002000 (partial 1/1)"); + expect(text).toContain("! Accounting is incomplete"); + expect(text).toContain("! Some records could not be saved"); + const snapshot = stats.snapshot(); + snapshot.persistenceFailures = 0; + expect(statsSummary(snapshot, store).join("\n")).toContain( + "! Some records could not be saved", + ); + }); it("labels unknown identity, complete and partial subtotals without combining main-agent costs", () => { const stats = new PairStats("session", vi.fn()); stats.start("job"); @@ -144,7 +191,7 @@ describe("statistics presentation", () => { const store = new ReviewStore(vi.fn()); store.setEnabled(false); const rendered = statsLines(stats.snapshot(), store).join("\n"); - expect(rendered).toContain("statistics - off"); + expect(rendered).toContain("Background review: off"); expect(rendered).toContain("Review latency: unavailable"); expect(rendered).toContain("No finalized extension model calls"); expect(rendered).toContain("Storage unavailable for 1 record(s)"); @@ -163,15 +210,31 @@ describe("StatsView", () => { ])("closes explicitly on %j without retaining resources", async (key) => { const host = ui(); const view = new StatsView(); - const opened = view.open(host.ctx, ["Review counts", "extension usage"]); + const opened = view.open( + host.ctx, + report(["Review counts", "extension usage", "! Warning", "USAGE / now"]), + ); await host.mounted; const component = host.component(); - expect(component.render(12).slice(0, 4)).toEqual([ - "Review count", - "s", - "extension us", - "age", + expect(component.render(11)).toEqual([ + "Pair Progr\u{1B}[0m…\u{1B}[0m", + "Session ov\u{1B}[0m…\u{1B}[0m", + "", + "Review", + "counts", + "extension", + "usage", + "! Warning", + "USAGE /", + "now", + "", + "↑↓ d deta\u{1B}[0m…\u{1B}[0m", ]); + const framed = component.render(24); + expect(framed[0]).toBe(`╭${"─".repeat(22)}╮`); + expect(framed[4]).toBe(`│ Review counts${" ".repeat(5)} │`); + expect(framed.at(-2)).toContain("↑↓ d details r "); + expect(framed.at(-1)).toBe(`╰${"─".repeat(22)}╯`); component.invalidate(); component.handleInput?.(key); component.handleInput?.(key); @@ -187,22 +250,69 @@ describe("StatsView", () => { { length: 12 }, (_, index) => `Row ${String(index)}`, ); - const opened = view.open(host.ctx, lines); + const opened = view.open(host.ctx, report(lines)); await host.mounted; const component = host.component(); - expect(component.render(80)[0]).toBe("Row 0"); + const first = (): string | undefined => component.render(80)[4]; + expect(first()).toContain("Row 0"); + expect(component.render(80).at(-2)).toContain("esc close 1–9/12"); component.handleInput?.("\u{1B}[B"); - expect(component.render(80)[0]).toBe("Row 1"); + expect(first()).toContain("Row 1"); component.handleInput?.("\u{1B}[A"); - expect(component.render(80)[0]).toBe("Row 0"); - component.handleInput?.("\u{1B}[6~"); - expect(component.render(80)[0]).toBe("Row 5"); + expect(first()).toContain("Row 0"); component.handleInput?.("\u{1B}[6~"); - expect(component.render(80)[0]).toBe("Row 7"); + expect(first()).toContain("Row 3"); component.handleInput?.("\u{1B}[5~"); - expect(component.render(80)[0]).toBe("Row 2"); + expect(first()).toContain("Row 0"); + component.handleInput?.("d"); + expect(component.render(80)[2]).toContain("Detailed accounting"); + expect(first()).toContain("detail Row 0"); + expect(component.render(80).at(-2)).toContain("d overview"); + component.handleInput?.("\u{1B}[B"); + component.handleInput?.("r"); + expect(first()).toContain("detail Row 0"); + component.handleInput?.("d"); + expect(first()).toContain("Row 0"); component.handleInput?.("ignored"); - expect(host.renderRequests()).toBe(6); + expect(host.renderRequests()).toBe(9); + view.close(); + await opened; + }); + + it("refreshes snapshots and recomputes themed, Unicode-safe layout on resize", async () => { + let color = "31"; + let value = "変更 e\u{301} 👩‍💻 " + "long-model-name".repeat(10); + const host = ui(40, (_tone, text) => `\u{1B}[${color}m${text}\u{1B}[0m`); + const read = vi.fn(() => ({ summary: [value], details: [value] })); + const view = new StatsView(); + const opened = view.open(host.ctx, read); + await host.mounted; + const component = host.component(); + for (const width of [1, 11, 12, 24, 78, 160]) + for (const line of component.render(width)) + expect(visibleWidth(line)).toBeLessThanOrEqual(width); + expect(component.render(78).join("\n")).toContain("変更"); + value = "Refreshed"; + color = "32"; + component.invalidate(); + expect(component.render(78).join("\n")).toContain("\u{1B}[32m変更"); + expect(read).toHaveBeenCalledTimes(1); + component.handleInput?.("r"); + expect(component.render(78).join("\n")).toContain("Refreshed"); + expect(read).toHaveBeenCalledTimes(2); + view.close(); + await opened; + }); + + it("keeps tiny terminals to the scrollable report body", async () => { + const host = ui(3); + const view = new StatsView(); + const opened = view.open(host.ctx, report(["one", "two", "three", "four"])); + await host.mounted; + const component = host.component(); + expect(component.render(80)).toEqual(["one", "two", "three"]); + component.handleInput?.("\u{1B}[6~"); + expect(component.render(80)).toEqual(["two", "three", "four"]); view.close(); await opened; }); @@ -211,12 +321,12 @@ describe("StatsView", () => { const first = ui(); const second = ui(); const view = new StatsView(); - const previous = view.open(first.ctx, ["old"]); + const previous = view.open(first.ctx, report(["old"])); await first.mounted; - const current = view.open(second.ctx, ["current"]); + const current = view.open(second.ctx, report(["current"])); await second.mounted; await previous; - expect(second.component().render(80)[0]).toBe("current"); + expect(second.component().render(80)[4]).toContain("current"); view.close(); await current; }); @@ -225,12 +335,12 @@ describe("StatsView", () => { const host = ui(); const view = new StatsView(); host.ctx.hasUI = false; - await expect(view.open(host.ctx, [])).rejects.toThrow( + await expect(view.open(host.ctx, report([]))).rejects.toThrow( "interactive terminal", ); host.ctx.hasUI = true; host.ctx.mode = "rpc"; - await view.open(host.ctx, []); + await view.open(host.ctx, report([])); expect(host.notify).toHaveBeenCalledWith( expect.stringContaining("interactive terminal"), "warning", @@ -243,10 +353,12 @@ describe("StatsView", () => { hasUI: true, ui: { custom: () => Promise.resolve() }, } as unknown as ExtensionContext; - await expect(view.open(ctx, [])).rejects.toThrow("interactive terminal"); + await expect(view.open(ctx, report([]))).rejects.toThrow( + "interactive terminal", + ); const interactive = ui(); Reflect.deleteProperty(interactive.ctx, "mode"); - const opened = view.open(interactive.ctx, ["OMP view"]); + const opened = view.open(interactive.ctx, report(["OMP view"])); await interactive.mounted; view.close(); await opened; diff --git a/test/status-overlay.test.ts b/test/status-overlay.test.ts new file mode 100644 index 0000000..36ad7ab --- /dev/null +++ b/test/status-overlay.test.ts @@ -0,0 +1,197 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { describe, expect, it, vi, type Mock } from "vitest"; +import { StatusOverlay } from "../src/status-overlay.js"; + +type Factory = Parameters[0]; +interface Options { + overlayOptions: () => { + anchor: string; + width: number; + nonCapturing: boolean; + visible: (columns: number) => boolean; + }; +} + +interface Host { + ctx: ExtensionContext; + custom: Mock<(factory: Factory, opts: Options) => Promise>; + setWidget: ReturnType; + requestRender: ReturnType; + closed: ReturnType; + render: (width: number) => string[] | undefined; + options: () => ReturnType | undefined; +} + +function host(mode: string | undefined = "tui"): Host { + const requestRender = vi.fn(); + const setWidget = vi.fn(); + const closed = vi.fn(); + let render: ((width: number) => string[]) | undefined; + let options: Options | undefined; + const custom = vi.fn( + async (factory: Factory, opts: Options): Promise => { + options = opts; + const done = Promise.withResolvers(); + const component = await factory( + { requestRender } as unknown as Parameters[0], + { + fg: (color: string, text: string) => `<${color}>${text}`, + } as unknown as Parameters[1], + {} as Parameters[2], + () => { + closed(); + done.resolve(undefined); + }, + ); + render = (width) => component.render(width); + component.invalidate(); + return done.promise; + }, + ); + const ctx = { + hasUI: true, + mode, + ui: { custom, setWidget }, + } as unknown as ExtensionContext; + return { + ctx, + custom, + setWidget, + requestRender, + closed, + render: (width: number) => render?.(width), + options: () => options?.overlayOptions(), + }; +} + +describe("StatusOverlay", () => { + it("pins a non-capturing badge to the top-right and updates in place", async () => { + const tui = host(); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "success", text: "watching" }); + await Promise.resolve(); + expect(tui.options()).toMatchObject({ + anchor: "top-right", + width: 19, + nonCapturing: true, + }); + expect(tui.options()?.visible(39)).toBe(false); + expect(tui.options()?.visible(40)).toBe(true); + expect(tui.render(80)).toEqual([" ◆ pair · watching "]); + overlay.show(tui.ctx, { tone: "warning", text: "2 awaiting decision" }); + expect(tui.custom).toHaveBeenCalledTimes(1); + expect(tui.requestRender).toHaveBeenCalledTimes(2); + expect(tui.render(80)?.[0]).toContain("◆"); + expect(tui.options()?.width).toBe(30); + expect(tui.render(5)?.[0]).toContain("…"); + expect(tui.setWidget).not.toHaveBeenCalledWith( + "pair-programmer", + expect.anything(), + ); + overlay.dispose(); + expect(tui.closed).toHaveBeenCalledTimes(1); + expect(tui.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); + }); + + it("closes a badge disposed before the host mounts it", async () => { + const tui = host(); + let mount: (() => void) | undefined; + tui.custom.mockImplementationOnce( + (factory: Factory): Promise => { + return new Promise((resolve) => { + mount = () => { + const component = factory( + { requestRender: vi.fn() } as unknown as Parameters[0], + {} as Parameters[1], + {} as Parameters[2], + () => { + resolve(undefined); + }, + ); + expect(component).toBeDefined(); + }; + }); + }, + ); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + overlay.dispose(); + mount?.(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(tui.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); + }); + + it.each(["rpc", "print"])( + "falls back to a widget in %s mode without mounting an overlay", + (mode) => { + const tui = host(mode); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + expect(tui.custom).not.toHaveBeenCalled(); + expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · paused", + ]); + Object.assign(tui.ctx.ui, { + theme: { fg: (color: string, text: string) => `<${color}>${text}` }, + }); + overlay.show(tui.ctx, { tone: "accent", text: "1 queued" }); + expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · 1 queued", + ]); + }, + ); + + it("falls back to a widget when the host cannot keep the overlay", async () => { + for (const fail of [false, true]) { + const tui = host(); + tui.custom.mockImplementationOnce((): Promise => + fail ? Promise.reject(new Error("no")) : Promise.resolve(undefined), + ); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "success", text: "watching" }); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · watching", + ]); + overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · paused", + ]); + } + }); + + it("ignores stale host completions after moving to a new session", async () => { + const first = host(); + const pending = Promise.withResolvers(); + first.custom.mockReturnValueOnce(pending.promise); + const overlay = new StatusOverlay(); + overlay.show(first.ctx, { tone: "success", text: "watching" }); + const second = host("rpc"); + overlay.show(second.ctx, { tone: "success", text: "watching" }); + pending.resolve(undefined); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(first.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); + }); + + it("does nothing on dispose before any session", () => { + expect(() => { + new StatusOverlay().dispose(); + }).not.toThrow(); + }); + + it("skips overlays for headless contexts", () => { + const tui = host(); + tui.ctx.hasUI = false; + new StatusOverlay().show(tui.ctx, { tone: "dim", text: "paused" }); + expect(tui.custom).not.toHaveBeenCalled(); + }); +}); From 1ed416b980ac6ebaf4bbed0ee1a419ab6df4b672 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:22:52 +0200 Subject: [PATCH 02/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20expose=20live=20fi?= =?UTF-8?q?nding=20status=20from=20the=20review=20store?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReviewStore.lookup reports a finding's title, line, queued/awaiting/accepted/rejected/discarded status and decision reason. --- src/review-store.ts | 26 ++++++++++++++++++++++++++ test/review-store.test.ts | 28 ++++++++++++++++++++++++++++ 2 files changed, 54 insertions(+) diff --git a/src/review-store.ts b/src/review-store.ts index 30ea72d..d9f8274 100644 --- a/src/review-store.ts +++ b/src/review-store.ts @@ -44,6 +44,16 @@ export interface StoredFinding { reason?: string; } +export type FindingStatus = + "queued" | "awaiting" | "accepted" | "rejected" | "discarded"; + +export interface FindingView { + title: string; + line: number; + status: FindingStatus; + reason?: string; +} + interface FindingState extends StoredFinding { delivered: boolean; discarded: boolean; @@ -163,6 +173,22 @@ export class ReviewStore { return result; } + lookup(id: string): FindingView | undefined { + const state = this.findings.get(id); + if (state === undefined) return undefined; + let status: FindingStatus = "queued"; + if (state.verdict === "accept") status = "accepted"; + else if (state.verdict === "reject") status = "rejected"; + else if (state.discarded) status = "discarded"; + else if (state.delivered) status = "awaiting"; + return { + title: state.finding.title, + line: state.finding.line, + status, + ...(state.reason === undefined ? {} : { reason: state.reason }), + }; + } + deliveredFinding(id: string): Finding | undefined { const state = this.findings.get(id); return state?.delivered === true && state.verdict === undefined diff --git a/test/review-store.test.ts b/test/review-store.test.ts index 0f2de05..e659616 100644 --- a/test/review-store.test.ts +++ b/test/review-store.test.ts @@ -24,6 +24,34 @@ function session(): { } describe("ReviewStore", () => { + it("describes each finding's live status for the review feed", () => { + const { store } = session(); + for (const id of ["queued", "awaiting", "accepted", "rejected", "stale"]) + store.add(finding(id)); + store.deliver(["awaiting", "accepted", "rejected"]); + store.decide("accepted", "accept", "Confirmed"); + store.decide("rejected", "reject", "Intentional"); + store.discardStale("src/a.ts", "two"); + expect(store.lookup("missing")).toBeUndefined(); + expect(store.lookup("awaiting")).toEqual({ + title: "Issue awaiting", + line: 12, + status: "awaiting", + }); + expect(store.lookup("accepted")).toMatchObject({ + status: "accepted", + reason: "Confirmed", + }); + expect(store.lookup("rejected")).toMatchObject({ + status: "rejected", + reason: "Intentional", + }); + expect(store.lookup("queued")?.status).toBe("discarded"); + const fresh = session().store; + fresh.add(finding("new")); + expect(fresh.lookup("new")?.status).toBe("queued"); + }); + it.each([true, false])( "clears finding history durably while preserving enabled=%s", (enabled) => { From d87206606c9e209d46e15d3ac3caaab0a03d0046 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:23:20 +0200 Subject: [PATCH 03/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20record=20a=20sessi?= =?UTF-8?q?on-scoped=20feed=20of=20review=20jobs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReviewFeed keeps the latest 100 reviews newest first with file, short model name, outcome, duration and attributed finding ids. --- src/review-feed.ts | 57 ++++++++++++++++++++++++++++++++++++++++ test/review-feed.test.ts | 53 +++++++++++++++++++++++++++++++++++++ 2 files changed, 110 insertions(+) create mode 100644 src/review-feed.ts create mode 100644 test/review-feed.test.ts diff --git a/src/review-feed.ts b/src/review-feed.ts new file mode 100644 index 0000000..f31fc98 --- /dev/null +++ b/src/review-feed.ts @@ -0,0 +1,57 @@ +import type { ReviewOutcome } from "./pair-stats.js"; + +export type ReviewPhase = "running" | ReviewOutcome; + +export interface FeedEntry { + id: string; + file: string; + model: string; + startedAt: number; + phase: ReviewPhase; + durationMs?: number; + findingIds: readonly string[]; +} + +const LIMIT = 100; + +/** In-memory, session-scoped history of review jobs, newest first. */ +export class ReviewFeed { + private entries: FeedEntry[] = []; + + start( + id: string, + job: { file: string; model: string }, + now = Date.now(), + ): void { + this.entries.unshift({ + id, + file: job.file, + model: job.model.slice(job.model.lastIndexOf("/") + 1), + startedAt: now, + phase: "running", + findingIds: [], + }); + this.entries.length = Math.min(this.entries.length, LIMIT); + } + + finish(id: string, outcome: ReviewOutcome, durationMs: number): void { + const entry = this.entries.find((candidate) => candidate.id === id); + if (entry?.phase !== "running") return; + entry.phase = outcome; + entry.durationMs = durationMs; + } + + attach(id: string, findingIds: readonly string[]): void { + const entry = this.entries.find((candidate) => candidate.id === id); + if (entry !== undefined) + entry.findingIds = [...new Set([...entry.findingIds, ...findingIds])]; + } + + clear(): void { + this.entries = []; + } + + list(): readonly FeedEntry[] { + return this.entries; + } +} diff --git a/test/review-feed.test.ts b/test/review-feed.test.ts new file mode 100644 index 0000000..7bf7c6f --- /dev/null +++ b/test/review-feed.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; +import { ReviewFeed } from "../src/review-feed.js"; + +const job = { + file: "src/a.ts", + model: "openai/gpt-5", +}; + +describe("ReviewFeed", () => { + it("records reviews newest first with short model names", () => { + const feed = new ReviewFeed(); + feed.start("one", job, 1); + feed.start("two", { ...job, model: "local" }, 2); + expect(feed.list().map((entry) => entry.id)).toEqual(["two", "one"]); + expect(feed.list()[1]).toMatchObject({ + model: "gpt-5", + phase: "running", + startedAt: 1, + }); + expect(feed.list()[0]).toMatchObject({ model: "local" }); + expect(new ReviewFeed().list()).toEqual([]); + const defaulted = new ReviewFeed(); + defaulted.start("now", job); + expect(defaulted.list()[0]?.startedAt).toBeGreaterThan(0); + }); + + it("settles each review once and attributes findings without duplicates", () => { + const feed = new ReviewFeed(); + feed.start("one", job, 1); + feed.finish("one", "success", 40); + feed.finish("one", "failed", 90); + feed.finish("missing", "failed", 1); + feed.attach("one", ["a", "b"]); + feed.attach("one", ["b", "c"]); + feed.attach("missing", ["z"]); + expect(feed.list()[0]).toMatchObject({ + phase: "success", + durationMs: 40, + findingIds: ["a", "b", "c"], + }); + feed.clear(); + expect(feed.list()).toEqual([]); + }); + + it("keeps only the latest hundred reviews", () => { + const feed = new ReviewFeed(); + for (let index = 0; index < 105; index += 1) + feed.start(String(index), job, index); + expect(feed.list()).toHaveLength(100); + expect(feed.list()[0]?.id).toBe("104"); + expect(feed.list().at(-1)?.id).toBe("5"); + }); +}); From c34449c9e15e20b965d147398eaa021c46634d4e Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:23:47 +0200 Subject: [PATCH 04/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20add=20a=20non-capt?= =?UTF-8?q?uring=20review=20sidebar=20overlay?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Framed top-right card rendering each review's outcome and model, with live verdicts and reasons. Paths keep the file name and drop leading directories first; the card sizes to its content above the editor. --- src/review-sidebar.ts | 306 ++++++++++++++++++++++++++++++++ test/review-sidebar.test.ts | 339 ++++++++++++++++++++++++++++++++++++ 2 files changed, 645 insertions(+) create mode 100644 src/review-sidebar.ts create mode 100644 test/review-sidebar.test.ts diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts new file mode 100644 index 0000000..f7de825 --- /dev/null +++ b/src/review-sidebar.ts @@ -0,0 +1,306 @@ +import type { ExtensionContext, Theme } from "@earendil-works/pi-coding-agent"; +import { + sliceByColumn, + truncateToWidth, + visibleWidth, + wrapTextWithAnsi, +} from "@earendil-works/pi-tui"; +import type { FeedEntry } from "./review-feed.js"; +import type { FindingView } from "./review-store.js"; + +type Painter = Pick; +type Color = Parameters[0]; +export type Lookup = (id: string) => FindingView | undefined; + +const SPINNER = "◐◓◑◒"; +export const MIN_COLUMNS = 90; +// Rows kept clear under the card for the editor and footer. +const EDITOR_RESERVE = 8; + +/** Keeps the end of a path: drop whole leading directories first, then characters. */ +export function truncatePath(file: string, width: number): string { + if (visibleWidth(file) <= width) return file; + const parts = file.split("/"); + for (let index = 1; index < parts.length; index += 1) { + const tail = `…/${parts.slice(index).join("/")}`; + if (visibleWidth(tail) <= width) return tail; + } + const graphemes = Array.from( + new Intl.Segmenter(undefined, { granularity: "grapheme" }).segment(file), + ({ segment }) => segment, + ); + const start = graphemes.findIndex( + (_, index) => visibleWidth(`…${graphemes.slice(index).join("")}`) <= width, + ); + return start === -1 ? "…" : `…${graphemes.slice(start).join("")}`; +} + +/** Joins left and right text on one row, shrinking the right side first. */ +function spread(left: string, right: string, width: number): string { + const room = Math.max(0, width - visibleWidth(left) - 1); + let shown = right; + if (visibleWidth(right) > room) + shown = room === 0 ? "" : `${sliceByColumn(right, 0, room - 1).trimEnd()}…`; + const gap = Math.max(1, width - visibleWidth(left) - visibleWidth(shown)); + return `${left}${" ".repeat(gap)}${shown}`; +} + +function seconds(ms: number): string { + return ms < 10_000 + ? `${(ms / 1000).toFixed(1)}s` + : `${String(Math.round(ms / 1000))}s`; +} + +function outcome( + entry: FeedEntry, + findings: number, + now: number, +): { icon: string; color: Color; text: string } { + switch (entry.phase) { + case "running": { + const frame = SPINNER.charAt(Math.floor(now / 250) % SPINNER.length); + return { icon: frame, color: "accent", text: "reviewing…" }; + } + case "success": { + if (findings === 0) + return { icon: "✓", color: "success", text: "no findings" }; + const noun = findings === 1 ? "finding" : "findings"; + return { + icon: "◆", + color: "warning", + text: `${String(findings)} ${noun}`, + }; + } + case "failed": + return { icon: "✗", color: "error", text: "review failed" }; + case "timeout": + return { icon: "✗", color: "error", text: "timed out" }; + case "obsolete": + return { icon: "○", color: "dim", text: "superseded by a newer edit" }; + case "cancelled": + return { icon: "○", color: "dim", text: "cancelled" }; + } +} + +const FINDING: Record< + FindingView["status"], + { icon: string; color: Color; label: string } +> = { + queued: { icon: "·", color: "accent", label: "queued" }, + awaiting: { icon: "?", color: "warning", label: "awaiting decision" }, + accepted: { icon: "✓", color: "success", label: "accepted" }, + rejected: { icon: "✗", color: "muted", label: "rejected" }, + discarded: { icon: "○", color: "dim", label: "discarded" }, +}; + +function entryLines( + entry: FeedEntry, + lookup: Lookup, + theme: Painter, + width: number, + now: number, +): string[] { + const findings = entry.findingIds.flatMap((id) => { + const view = lookup(id); + return view === undefined ? [] : [view]; + }); + const state = outcome(entry, findings.length, now); + const time = theme.fg( + "dim", + seconds(entry.durationMs ?? Math.max(0, now - entry.startedAt)), + ); + const file = truncatePath( + entry.file, + Math.max(1, width - visibleWidth(time) - 3), + ); + const lines = [ + spread( + `${theme.fg(state.color, state.icon)} ${theme.bold(file)}`, + time, + width, + ), + spread( + ` ${theme.fg(state.color, state.text)}`, + theme.fg("dim", entry.model), + width, + ), + ]; + for (const finding of findings) { + const style = FINDING[finding.status]; + lines.push( + ` ${theme.fg(style.color, style.icon)} ${finding.title}${theme.fg("dim", " :" + String(finding.line))}`, + ` ${theme.fg(style.color, style.label)}`, + ); + if (finding.reason !== undefined) + lines.push( + ...wrapTextWithAnsi(`“${finding.reason}”`, Math.max(1, width - 4)) + .slice(0, 2) + .map((part) => ` ${theme.fg("dim", part)}`), + ); + } + return lines; +} + +/** Renders the framed sidebar at an exact width, at most `height` rows. */ +export function renderSidebar( + entries: readonly FeedEntry[], + lookup: Lookup, + theme: Painter, + width: number, + height: number, + now = Date.now(), +): string[] { + const inner = Math.max(1, width - 4); + const row = (text: string): string => { + const clipped = truncateToWidth(text, inner, "…"); + const pad = " ".repeat(Math.max(0, inner - visibleWidth(clipped))); + return `${theme.fg("borderMuted", "│")} ${clipped}${pad} ${theme.fg("borderMuted", "│")}`; + }; + const running = entries.filter((entry) => entry.phase === "running").length; + const header = [ + theme.bold(theme.fg("accent", "Pair Programmer")), + theme.fg( + "dim", + running > 0 + ? `Live reviews · ${String(running)} running` + : "Live reviews", + ), + "", + ]; + const footer = ["", theme.fg("dim", "alt+r hide · /pair-stats totals")]; + const room = Math.max(0, height - 2 - header.length - footer.length); + const body: string[] = []; + if (entries.length === 0) + body.push( + theme.fg("muted", "No reviews yet."), + theme.fg("dim", "They appear after each edit."), + ); + let shown = 0; + for (const entry of entries) { + const block = [ + ...(shown === 0 ? [] : [""]), + ...entryLines(entry, lookup, theme, inner, now), + ]; + const reserve = shown + 1 < entries.length ? 1 : 0; + if (body.length + block.length + reserve > room) break; + body.push(...block); + shown += 1; + } + if (shown < entries.length) { + const hidden = entries.length - shown; + body.push( + theme.fg( + "dim", + `+${String(hidden)} earlier review${hidden === 1 ? "" : "s"}`, + ), + ); + } + const content = [...header, ...body].slice(0, room + header.length); + return [ + theme.fg("borderMuted", `╭${"─".repeat(Math.max(0, width - 2))}╮`), + ...[...content, ...footer].map(row), + theme.fg("borderMuted", `╰${"─".repeat(Math.max(0, width - 2))}╯`), + ]; +} + +/** Togglable, non-capturing right-hand overlay showing the live review feed. */ +export class ReviewSidebar { + private readonly entries: () => readonly FeedEntry[]; + private readonly lookup: Lookup; + private ctx: ExtensionContext | undefined; + private close: (() => void) | undefined; + private requestRender: (() => void) | undefined; + private ticker: NodeJS.Timeout | undefined; + + constructor(entries: () => readonly FeedEntry[], lookup: Lookup) { + this.entries = entries; + this.lookup = lookup; + } + + get open(): boolean { + return this.ctx !== undefined; + } + + toggle(ctx: ExtensionContext): void { + if (this.open) { + this.dispose(); + return; + } + if (!ctx.hasUI || (ctx as Partial).mode !== "tui") { + ctx.ui.notify("The review sidebar requires Pi's terminal UI.", "warning"); + return; + } + this.mount(ctx); + } + + /** Re-render after feed or verdict changes; follows session replacement. */ + refresh(ctx?: ExtensionContext): void { + if (ctx !== undefined && this.open && ctx !== this.ctx) this.mount(ctx); + this.requestRender?.(); + } + + dispose(): void { + clearInterval(this.ticker); + this.ticker = undefined; + this.close?.(); + this.close = undefined; + this.requestRender = undefined; + this.ctx = undefined; + } + + private mount(ctx: ExtensionContext): void { + this.dispose(); + this.ctx = ctx; + let active = true; + this.close = () => { + active = false; + }; + const release = (): void => { + if (this.ctx === ctx) this.dispose(); + }; + void Promise.resolve( + ctx.ui.custom( + (tui, theme, _keys, done) => { + const finish = (): void => { + active = false; + done(undefined); + }; + if (active) this.close = finish; + else queueMicrotask(finish); + this.requestRender = () => { + tui.requestRender(); + }; + this.ticker = setInterval(() => { + if (this.entries().some((entry) => entry.phase === "running")) + tui.requestRender(); + }, 250); + this.ticker.unref(); + return { + render: (width: number): string[] => + renderSidebar( + this.entries(), + this.lookup, + theme, + width, + Math.max(8, tui.terminal.rows - EDITOR_RESERVE), + ), + invalidate(): void { + return; + }, + }; + }, + { + overlay: true, + overlayOptions: { + anchor: "top-right", + width: "34%", + minWidth: 38, + margin: { top: 1, right: 1 }, + nonCapturing: true, + visible: (columns: number) => columns >= MIN_COLUMNS, + }, + }, + ), + ).then(release, release); + } +} diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts new file mode 100644 index 0000000..44478e9 --- /dev/null +++ b/test/review-sidebar.test.ts @@ -0,0 +1,339 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import { visibleWidth } from "@earendil-works/pi-tui"; +import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; +import { ReviewFeed } from "../src/review-feed.js"; +import { + MIN_COLUMNS, + renderSidebar, + ReviewSidebar, + truncatePath, +} from "../src/review-sidebar.js"; +import type { FindingView } from "../src/review-store.js"; + +const plain = { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, +} as unknown as Parameters[2]; +const job = { + file: "src/a.ts", + model: "openai/gpt-5", +}; +const views: Record = { + accepted: { + title: "Duplicate helper", + line: 42, + status: "accepted", + reason: "Reuse the existing padding helper instead of adding another one.", + }, + awaiting: { title: "Magic interval", line: 7, status: "awaiting" }, + queued: { title: "Queued", line: 1, status: "queued" }, + rejected: { title: "Rejected", line: 2, status: "rejected", reason: "No" }, + discarded: { title: "Discarded", line: 3, status: "discarded" }, +}; +const viewMap = new Map(Object.entries(views)); +const lookup = (id: string): FindingView | undefined => viewMap.get(id); +const text = ( + feed: ReviewFeed, + width = 48, + height = 60, + now = 20_000, +): string[] => renderSidebar(feed.list(), lookup, plain, width, height, now); + +describe("renderSidebar", () => { + it("frames an empty feed at its natural height", () => { + const lines = text(new ReviewFeed(), 40, 12); + expect(lines).toHaveLength(9); + expect(lines[0]).toBe(`╭${"─".repeat(38)}╮`); + expect(lines.at(-1)).toBe(`╰${"─".repeat(38)}╯`); + expect(lines.join("\n")).toContain("No reviews yet."); + expect(lines.at(-2)).toContain("alt+r hide"); + for (const line of lines) expect(visibleWidth(line)).toBe(40); + }); + + it("narrates every review outcome and finding verdict", () => { + const feed = new ReviewFeed(); + const outcomes = [ + "failed", + "timeout", + "obsolete", + "cancelled", + "success", + ] as const; + for (const [index, outcome] of outcomes.entries()) { + feed.start(outcome, job, index); + feed.finish(outcome, outcome, 1500); + } + feed.start("one", job, 0); + feed.finish("one", "success", 12_400); + feed.attach("one", ["awaiting", "missing"]); + feed.start("many", job, 0); + feed.finish("many", "success", 900); + feed.attach("many", Object.keys(views)); + feed.start("live", { ...job, model: "" }, 19_000); + const rendered = text(feed, 60, 80).join("\n"); + for (const expected of [ + "Live reviews · 1 running", + "reviewing…", + "1.0s", + "review failed", + "timed out", + "superseded by a newer edit", + "cancelled", + "no findings", + "1 finding", + "12s", + "5 findings", + "Duplicate helper :42", + "awaiting decision", + "“Reuse the existing padding helper", + "queued", + "rejected", + "“No”", + "discarded", + "gpt-5", + ]) + expect(rendered).toContain(expected); + expect(renderSidebar([], lookup, plain, 40, 10).join("")).toContain( + "Live reviews", + ); + }); + + it("keeps the newest reviews that fit and counts the rest", () => { + const feed = new ReviewFeed(); + for (let index = 0; index < 6; index += 1) { + feed.start(String(index), { ...job, file: `f${String(index)}.ts` }, 0); + feed.finish(String(index), "success", 1); + } + const lines = text(feed, 40, 16); + const joined = lines.join("\n"); + expect(lines.length).toBeLessThanOrEqual(16); + expect(joined).toContain("f5.ts"); + expect(joined).not.toContain("f2.ts"); + expect(joined).toContain("f3.ts"); + expect(joined).toContain("+3 earlier reviews"); + const single = new ReviewFeed(); + single.start("a", job, 0); + single.start("b", job, 0); + expect(text(single, 40, 10).join("\n")).toContain( + "+1 earlier review\u{20}", + ); + for (const line of text(feed, 38, 5)) expect(visibleWidth(line)).toBe(38); + }); + + it("keeps file names and drops leading directories first", () => { + const file = "apps/hush/lib/pages/chat_detail/widgets/bubble.dart"; + expect(truncatePath(file, 80)).toBe(file); + expect(truncatePath(file, 28)).toBe("…/widgets/bubble.dart"); + expect(truncatePath(file, 16)).toBe("…/bubble.dart"); + expect(truncatePath(file, 8)).toBe("…le.dart"); + expect(truncatePath("變更變更變更.ts", 6)).toBe("…更.ts"); + expect(truncatePath("x.ts", 0)).toBe("…"); + }); + + it("shares a row between the outcome and a shortened model, without the prompt", () => { + const feed = new ReviewFeed(); + feed.start( + "a", + { + ...job, + model: "provider/a-very-long-model-name-that-cannot-fit-beside-it", + }, + 0, + ); + feed.finish("a", "success", 1); + const lines = text(feed, 40, 40).map((line) => line.slice(2, -2)); + expect(lines[5]).toContain("no findings"); + expect(lines[5]?.trimEnd()).toMatch(/nam…$/u); + expect(lines[6]?.trim()).toBe(""); + for (const width of [38, 12]) + for (const line of text(feed, width, 40)) + expect(visibleWidth(line)).toBe(width); + }); + + it("truncates long paths and animates running reviews", () => { + const feed = new ReviewFeed(); + feed.start("a", { ...job, file: `${"deep/".repeat(20)}file.ts` }, 0); + const lines = text(feed, 40, 20, 0); + for (const line of lines) expect(visibleWidth(line)).toBe(40); + expect(lines.join("\n")).toContain("…"); + const frames = [0, 250, 500, 750].map((now) => text(feed, 40, 20, now)[4]); + expect(new Set(frames).size).toBe(4); + }); +}); + +type Factory = Parameters[0]; +interface Host { + ctx: ExtensionContext; + custom: Mock; + notify: Mock; + requestRender: Mock; + closed: Mock; + options: () => { + anchor: string; + nonCapturing: boolean; + visible: (columns: number) => boolean; + }; + render: (width: number) => string[] | undefined; +} + +function host(mode = "tui"): Host { + const requestRender = vi.fn(); + const notify = vi.fn(); + const closed = vi.fn(); + let options: ReturnType | undefined; + let render: ((width: number) => string[]) | undefined; + const custom = vi.fn( + async ( + factory: Factory, + opts: { overlayOptions: ReturnType }, + ): Promise => { + options = opts.overlayOptions; + const done = Promise.withResolvers(); + const component = await factory( + { + requestRender, + terminal: { rows: 20 }, + } as unknown as Parameters[0], + plain as unknown as Parameters[1], + {} as Parameters[2], + () => { + closed(); + done.resolve(undefined); + }, + ); + component.invalidate(); + render = (width) => component.render(width); + return done.promise; + }, + ); + return { + ctx: { + hasUI: true, + mode, + ui: { custom, notify }, + } as unknown as ExtensionContext, + custom, + notify, + requestRender, + closed, + options: () => { + if (options === undefined) throw new Error("not mounted"); + return options; + }, + render: (width) => render?.(width), + }; +} + +afterEach(() => { + vi.useRealTimers(); +}); + +describe("ReviewSidebar", () => { + it("toggles a non-capturing right-hand overlay that renders the live feed", async () => { + vi.useFakeTimers(); + const feed = new ReviewFeed(); + const tui = host(); + const sidebar = new ReviewSidebar(() => feed.list(), lookup); + sidebar.refresh(tui.ctx); + expect(tui.custom).not.toHaveBeenCalled(); + sidebar.toggle(tui.ctx); + await Promise.resolve(); + expect(sidebar.open).toBe(true); + expect(tui.options()).toMatchObject({ + anchor: "top-right", + nonCapturing: true, + }); + expect(tui.options().visible(MIN_COLUMNS - 1)).toBe(false); + expect(tui.options().visible(MIN_COLUMNS)).toBe(true); + expect(tui.render(40)).toHaveLength(9); + vi.advanceTimersByTime(250); + expect(tui.requestRender).not.toHaveBeenCalled(); + feed.start("a", job, Date.now()); + vi.advanceTimersByTime(250); + expect(tui.requestRender).toHaveBeenCalledTimes(1); + sidebar.refresh(); + expect(tui.requestRender).toHaveBeenCalledTimes(2); + sidebar.toggle(tui.ctx); + expect(sidebar.open).toBe(false); + await Promise.resolve(); + expect(tui.closed).toHaveBeenCalledTimes(1); + vi.advanceTimersByTime(1000); + expect(tui.requestRender).toHaveBeenCalledTimes(2); + }); + + it("follows session replacement while open", async () => { + const first = host(); + const second = host(); + const sidebar = new ReviewSidebar(() => [], lookup); + sidebar.toggle(first.ctx); + sidebar.refresh(first.ctx); + expect(first.custom).toHaveBeenCalledTimes(1); + sidebar.refresh(second.ctx); + await Promise.resolve(); + expect(first.closed).toHaveBeenCalledTimes(1); + expect(second.custom).toHaveBeenCalledTimes(1); + expect(sidebar.open).toBe(true); + sidebar.dispose(); + }); + + it("explains that the sidebar needs Pi's terminal UI", () => { + const headless = host(); + headless.ctx.hasUI = false; + for (const unsupported of [host("rpc"), headless]) { + const sidebar = new ReviewSidebar(() => [], lookup); + sidebar.toggle(unsupported.ctx); + expect(sidebar.open).toBe(false); + expect(unsupported.custom).not.toHaveBeenCalled(); + expect(unsupported.notify).toHaveBeenCalledWith( + expect.stringContaining("terminal UI"), + "warning", + ); + } + }); + + it("closes when the host ends the overlay or disposes it before mounting", async () => { + const ended = host(); + ended.custom.mockReturnValueOnce(Promise.resolve(undefined)); + const sidebar = new ReviewSidebar(() => [], lookup); + sidebar.toggle(ended.ctx); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sidebar.open).toBe(false); + + const failed = host(); + failed.custom.mockReturnValueOnce(Promise.reject(new Error("no"))); + sidebar.toggle(failed.ctx); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sidebar.open).toBe(false); + + const late = host(); + let mount: (() => void) | undefined; + late.custom.mockImplementationOnce( + (factory: Factory): Promise => { + return new Promise((resolve) => { + mount = () => { + const component = factory( + { + requestRender: vi.fn(), + terminal: { rows: 20 }, + } as unknown as Parameters[0], + plain as unknown as Parameters[1], + {} as Parameters[2], + () => { + resolve(undefined); + }, + ); + expect(component).toBeDefined(); + }; + }); + }, + ); + sidebar.toggle(late.ctx); + sidebar.toggle(late.ctx); + mount?.(); + await new Promise((resolve) => setTimeout(resolve, 0)); + expect(sidebar.open).toBe(false); + sidebar.toggle(late.ctx); + expect(sidebar.open).toBe(true); + sidebar.dispose(); + }); +}); From e0d9809cb58fa5b71da33fc438c67d8d3d6d864f Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:24:15 +0200 Subject: [PATCH 05/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20toggle=20the=20liv?= =?UTF-8?q?e=20review=20sidebar=20with=20/pair-feed=20and=20alt+r?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Feeds review starts, outcomes and newly admitted findings into the sidebar, refreshes it on every state change, clears it with /pair-clear and session changes, and documents the controls. --- README.md | 1 + src/index.ts | 51 ++++++++++++++++++++++++++++++++- test/extension.test.ts | 65 ++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 116 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 8ec4a92..c54081f 100644 --- a/README.md +++ b/README.md @@ -67,5 +67,6 @@ Use `current` or a `provider/model` identifier. File patterns are relative to yo A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. +- `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. Newest reviews come first, each with its file, reviewer, model, duration and outcome (no findings, findings, failed, timed out, superseded or cancelled), plus every finding's decision and reason as it happens. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. - `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. - Diagnostics stay in files, never the terminal: OMP uses its native logs; Pi uses `~/.pi/agent/logs/pair-programmer/` (or `$PI_CODING_AGENT_DIR/logs/pair-programmer/`). diff --git a/src/index.ts b/src/index.ts index e1863ac..7e8f431 100644 --- a/src/index.ts +++ b/src/index.ts @@ -23,6 +23,8 @@ import { } from "./pair-stats.js"; import { StatsView, statsLines, statsSummary } from "./stats-view.js"; import { StatusOverlay, type StatusTone } from "./status-overlay.js"; +import { ReviewFeed } from "./review-feed.js"; +import { ReviewSidebar } from "./review-sidebar.js"; import { isInherited, reviewFile, @@ -145,6 +147,11 @@ export default function pairProgrammer(pi: ExtensionAPI): void { })(); const statsView = new StatsView(); const statusOverlay = new StatusOverlay(); + const feed = new ReviewFeed(); + const sidebar = new ReviewSidebar( + () => feed.list(), + (id) => store.lookup(id), + ); const accounting = new SessionAccounting(); let activeContext: ExtensionContext | undefined; const cancelJobs = new Map void>(); @@ -187,6 +194,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { state = `${String(findings.pending)} queued`; } statusOverlay.show(ctx, { tone, text: state }); + sidebar.refresh(ctx); } function stopWatchingResets(): void { @@ -246,6 +254,8 @@ export default function pairProgrammer(pi: ExtensionAPI): void { wakeTimer = undefined; wakePending = false; presented.clear(); + if (reasonCode === "session_change" || reasonCode === "cleared") + feed.clear(); } function deliverable(): readonly Finding[] { @@ -353,6 +363,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { finished = true; const durationMs = performance.now() - started; job.stats.finish(job.id, result, durationMs, settled); + feed.finish(job.id, result, durationMs); logger?.log("review.finished", { ...fields, outcome: result, @@ -396,6 +407,10 @@ export default function pairProgrammer(pi: ExtensionAPI): void { finish("cancelled", false); }); job.stats.start(job.id); + feed.start(job.id, { + file: job.file, + model: job.model, + }); showState(job.ctx); logger?.log("review.started", fields); void (async () => { @@ -493,6 +508,9 @@ export default function pairProgrammer(pi: ExtensionAPI): void { count: judged.filter(({ inherited }) => inherited).length, }); if (signal.aborted || !(await current(job))) return "obsolete"; + const before = new Set( + store.history(job.file).map((stored) => stored.finding.id), + ); const result = await admission.admit({ file: job.file, reviewer, @@ -510,7 +528,19 @@ export default function pairProgrammer(pi: ExtensionAPI): void { outcome: result, }); if (result === "obsolete") return "obsolete"; - if (result === "added") scheduleWake(job.ctx); + if (result === "added") { + feed.attach( + job.id, + store + .history(job.file) + .filter( + ({ finding }) => + finding.reviewer === reviewer && !before.has(finding.id), + ) + .map(({ finding }) => finding.id), + ); + scheduleWake(job.ctx); + } reviewed.set(`${job.key}:${reviewer}:${job.model}`, job.revision); return "success"; } @@ -783,6 +813,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { stopWatchingResets(); stop("shutdown"); statusOverlay.dispose(); + sidebar.dispose(); activeContext = undefined; accounting.suspend(); replaceBaseline(undefined); @@ -962,6 +993,24 @@ export default function pairProgrammer(pi: ExtensionAPI): void { }, }); + const toggleSidebar = (ctx: ExtensionContext): void => { + checkReset?.(); + sidebar.toggle(ctx); + }; + pi.registerCommand("pair-feed", { + description: "Toggle the live review sidebar", + handler: (_args, ctx) => { + toggleSidebar(ctx); + return Promise.resolve(); + }, + }); + // OMP may not expose shortcuts; the command remains the portable toggle. + if (typeof pi.registerShortcut === "function") + pi.registerShortcut("alt+r", { + description: "Toggle the Pair Programmer review sidebar", + handler: toggleSidebar, + }); + pi.registerCommand("pair-stats", { description: "Show on-demand Pair Programmer activity, findings and extension usage", diff --git a/test/extension.test.ts b/test/extension.test.ts index d13674f..b36f33b 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -212,6 +212,7 @@ async function setup( sendMessage: Mock; isIdle: Mock<() => boolean>; notify: ReturnType; + shortcuts: Map void>; setWidget: Mock<(key: string, content: string[] | undefined) => void>; status: () => string | undefined; entries: JournalEntry[]; @@ -249,6 +250,7 @@ async function setup( ); } const hooks = new Map(); + const shortcuts = new Map void>(); const commands = new Map< string, Parameters[1] @@ -283,6 +285,16 @@ async function setup( commands.set(name, command); }, sendMessage, + ...(host === "pi" + ? { + registerShortcut( + key: string, + options: { handler: (ctx: ExtensionContext) => void }, + ) { + shortcuts.set(key, options.handler); + }, + } + : {}), appendEntry(customType: string, data: unknown) { if (customType === "pair-programmer-stats") { if (entryError !== undefined) throw entryError; @@ -341,6 +353,7 @@ async function setup( isIdle, notify, setWidget, + shortcuts, status: () => setWidget.mock.lastCall?.[1]?.[0], entries, statsEntries, @@ -2719,6 +2732,58 @@ it("styles the status widget with the active host theme", async () => { await environment.emit("session_shutdown"); }); +it("feeds each review's outcome and attributed findings into the toggled sidebar", async () => { + const environment = await setup(); + const file = path.join(environment.cwd, "change.ts"); + await fsPromises.writeFile(file, "export const broken = true;\n"); + let render: ((width: number) => string[]) | undefined; + const plain = { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, + }; + environment.custom.mockImplementation(async (factory) => { + const component = await factory( + { + requestRender: vi.fn(), + terminal: { rows: 60 }, + } as unknown as Parameters[0], + plain as unknown as Parameters[1], + {} as Parameters[2], + vi.fn(), + ); + if (component.render(60).join("").includes("Live reviews")) + render = (width) => component.render(width); + return new Promise(() => { + return; + }); + }); + environment.shortcuts.get("alt+r")?.(environment.ctx); + await Promise.resolve(); + expect(render?.(60).join("\n")).toContain("No reviews yet."); + vi.mocked(reviewFile).mockResolvedValue([ + { line: 1, title: "Sidebar issue", quote: "broken", evidence: "Why" }, + ]); + await environment.emit("tool_result", { + toolName: "write", + input: { path: file }, + isError: false, + }); + await advanceReviews(() => { + expect(findings(environment.entries)).toHaveLength(2); + }); + await vi.advanceTimersByTimeAsync(0); + const feed = render?.(60).join("\n") ?? ""; + expect(feed).toContain("change.ts"); + expect(feed).toContain("1 finding"); + expect(feed).toContain("Sidebar issue :1"); + expect(feed).toContain("queued"); + await environment.command("pair-clear"); + expect(render?.(60).join("\n")).toContain("No reviews yet."); + await environment.command("pair-feed"); + await environment.command("pair-feed"); + await environment.emit("session_shutdown"); +}); + it("narrates review progress above the editor without surfacing finding contents", async () => { const environment = await setup(); const file = path.join(environment.cwd, "change.ts"); From 53f9a71f52b22037f2780c8d8f1d9836465ce9da Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:24:28 +0200 Subject: [PATCH 06/21] chore: ignore generated vitest reports --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 25fbf5a..64b8408 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ node_modules/ coverage/ +.vitest/ From 5e756159a74ffa8152b885865618950b2e0fae90 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:32:14 +0200 Subject: [PATCH 07/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20show=20only=20runn?= =?UTF-8?q?ing=20and=20decided=20reviews=20in=20the=20sidebar?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviews appear while running or once they have accepted or rejected findings; only decided findings are listed, summarized as 'N accepted · N rejected'. No-finding, undecided, failed, timed-out, superseded and cancelled reviews are hidden. --- README.md | 2 +- src/review-sidebar.ts | 110 ++++++++++++++++++------------------ test/extension.test.ts | 22 ++++++-- test/review-sidebar.test.ts | 69 ++++++++++++---------- 4 files changed, 112 insertions(+), 91 deletions(-) diff --git a/README.md b/README.md index c54081f..413465e 100644 --- a/README.md +++ b/README.md @@ -67,6 +67,6 @@ Use `current` or a `provider/model` identifier. File patterns are relative to yo A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. -- `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. Newest reviews come first, each with its file, reviewer, model, duration and outcome (no findings, findings, failed, timed out, superseded or cancelled), plus every finding's decision and reason as it happens. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. +- `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first, each with its file, model and duration, plus every decided finding and the agent's reason. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. - `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. - Diagnostics stay in files, never the terminal: OMP uses its native logs; Pi uses `~/.pi/agent/logs/pair-programmer/` (or `$PI_CODING_AGENT_DIR/logs/pair-programmer/`). diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index f7de825..f0e8397 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -51,60 +51,56 @@ function seconds(ms: number): string { : `${String(Math.round(ms / 1000))}s`; } -function outcome( - entry: FeedEntry, - findings: number, - now: number, -): { icon: string; color: Color; text: string } { - switch (entry.phase) { - case "running": { - const frame = SPINNER.charAt(Math.floor(now / 250) % SPINNER.length); - return { icon: frame, color: "accent", text: "reviewing…" }; - } - case "success": { - if (findings === 0) - return { icon: "✓", color: "success", text: "no findings" }; - const noun = findings === 1 ? "finding" : "findings"; - return { - icon: "◆", - color: "warning", - text: `${String(findings)} ${noun}`, - }; - } - case "failed": - return { icon: "✗", color: "error", text: "review failed" }; - case "timeout": - return { icon: "✗", color: "error", text: "timed out" }; - case "obsolete": - return { icon: "○", color: "dim", text: "superseded by a newer edit" }; - case "cancelled": - return { icon: "○", color: "dim", text: "cancelled" }; - } -} +type Decided = FindingView & { status: "accepted" | "rejected" }; -const FINDING: Record< - FindingView["status"], - { icon: string; color: Color; label: string } -> = { - queued: { icon: "·", color: "accent", label: "queued" }, - awaiting: { icon: "?", color: "warning", label: "awaiting decision" }, - accepted: { icon: "✓", color: "success", label: "accepted" }, - rejected: { icon: "✗", color: "muted", label: "rejected" }, - discarded: { icon: "○", color: "dim", label: "discarded" }, +const FINDING: Record = { + accepted: { icon: "✓", color: "success" }, + rejected: { icon: "✗", color: "muted" }, }; +/** Only accepted and rejected findings are worth surfacing. */ +function decided(entry: FeedEntry, lookup: Lookup): Decided[] { + return entry.findingIds.flatMap((id) => { + const view = lookup(id); + return view?.status === "accepted" || view?.status === "rejected" + ? [view as Decided] + : []; + }); +} + +function summary(findings: readonly Decided[]): { + icon: string; + color: Color; + text: string; +} { + const accepted = findings.filter((f) => f.status === "accepted").length; + const rejected = findings.length - accepted; + const text = [ + accepted > 0 ? `${String(accepted)} accepted` : "", + rejected > 0 ? `${String(rejected)} rejected` : "", + ] + .filter(Boolean) + .join(" · "); + return accepted > 0 + ? { icon: "◆", color: "warning", text } + : { icon: "✗", color: "muted", text }; +} + function entryLines( entry: FeedEntry, - lookup: Lookup, + findings: readonly Decided[], theme: Painter, width: number, now: number, ): string[] { - const findings = entry.findingIds.flatMap((id) => { - const view = lookup(id); - return view === undefined ? [] : [view]; - }); - const state = outcome(entry, findings.length, now); + const state = + entry.phase === "running" + ? { + icon: SPINNER.charAt(Math.floor(now / 250) % SPINNER.length), + color: "accent" as const, + text: "reviewing…", + } + : summary(findings); const time = theme.fg( "dim", seconds(entry.durationMs ?? Math.max(0, now - entry.startedAt)), @@ -129,7 +125,6 @@ function entryLines( const style = FINDING[finding.status]; lines.push( ` ${theme.fg(style.color, style.icon)} ${finding.title}${theme.fg("dim", " :" + String(finding.line))}`, - ` ${theme.fg(style.color, style.label)}`, ); if (finding.reason !== undefined) lines.push( @@ -151,6 +146,12 @@ export function renderSidebar( now = Date.now(), ): string[] { const inner = Math.max(1, width - 4); + const visible = entries.flatMap((entry) => { + const findings = decided(entry, lookup); + return entry.phase === "running" || findings.length > 0 + ? [{ entry, findings }] + : []; + }); const row = (text: string): string => { const clipped = truncateToWidth(text, inner, "…"); const pad = " ".repeat(Math.max(0, inner - visibleWidth(clipped))); @@ -170,24 +171,25 @@ export function renderSidebar( const footer = ["", theme.fg("dim", "alt+r hide · /pair-stats totals")]; const room = Math.max(0, height - 2 - header.length - footer.length); const body: string[] = []; - if (entries.length === 0) + if (visible.length === 0) body.push( - theme.fg("muted", "No reviews yet."), - theme.fg("dim", "They appear after each edit."), + theme.fg("muted", "Nothing to show yet."), + theme.fg("dim", "Running reviews and decided"), + theme.fg("dim", "findings appear here."), ); let shown = 0; - for (const entry of entries) { + for (const { entry, findings } of visible) { const block = [ ...(shown === 0 ? [] : [""]), - ...entryLines(entry, lookup, theme, inner, now), + ...entryLines(entry, findings, theme, inner, now), ]; - const reserve = shown + 1 < entries.length ? 1 : 0; + const reserve = shown + 1 < visible.length ? 1 : 0; if (body.length + block.length + reserve > room) break; body.push(...block); shown += 1; } - if (shown < entries.length) { - const hidden = entries.length - shown; + if (shown < visible.length) { + const hidden = visible.length - shown; body.push( theme.fg( "dim", diff --git a/test/extension.test.ts b/test/extension.test.ts index b36f33b..d2b954b 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -2759,7 +2759,7 @@ it("feeds each review's outcome and attributed findings into the toggled sidebar }); environment.shortcuts.get("alt+r")?.(environment.ctx); await Promise.resolve(); - expect(render?.(60).join("\n")).toContain("No reviews yet."); + expect(render?.(60).join("\n")).toContain("Nothing to show yet."); vi.mocked(reviewFile).mockResolvedValue([ { line: 1, title: "Sidebar issue", quote: "broken", evidence: "Why" }, ]); @@ -2773,12 +2773,22 @@ it("feeds each review's outcome and attributed findings into the toggled sidebar }); await vi.advanceTimersByTimeAsync(0); const feed = render?.(60).join("\n") ?? ""; - expect(feed).toContain("change.ts"); - expect(feed).toContain("1 finding"); - expect(feed).toContain("Sidebar issue :1"); - expect(feed).toContain("queued"); + expect(feed).not.toContain("change.ts"); + await environment.emit("turn_end"); + const [finding] = findings(environment.entries); + if (finding === undefined) throw new Error("Missing finding"); + await environment.decide(finding.id, { + findingId: finding.id, + decision: "accept", + reason: "Real issue", + }); + const decidedFeed = render?.(60).join("\n") ?? ""; + expect(decidedFeed).toContain("change.ts"); + expect(decidedFeed).toContain("1 accepted"); + expect(decidedFeed).toContain("Sidebar issue :1"); + expect(decidedFeed).toContain("“Real issue”"); await environment.command("pair-clear"); - expect(render?.(60).join("\n")).toContain("No reviews yet."); + expect(render?.(60).join("\n")).toContain("Nothing to show yet."); await environment.command("pair-feed"); await environment.command("pair-feed"); await environment.emit("session_shutdown"); diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index 44478e9..b155473 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -29,6 +29,7 @@ const views: Record = { queued: { title: "Queued", line: 1, status: "queued" }, rejected: { title: "Rejected", line: 2, status: "rejected", reason: "No" }, discarded: { title: "Discarded", line: 3, status: "discarded" }, + bare: { title: "Bare accept", line: 4, status: "accepted" }, }; const viewMap = new Map(Object.entries(views)); const lookup = (id: string): FindingView | undefined => viewMap.get(id); @@ -42,15 +43,15 @@ const text = ( describe("renderSidebar", () => { it("frames an empty feed at its natural height", () => { const lines = text(new ReviewFeed(), 40, 12); - expect(lines).toHaveLength(9); + expect(lines).toHaveLength(10); expect(lines[0]).toBe(`╭${"─".repeat(38)}╮`); expect(lines.at(-1)).toBe(`╰${"─".repeat(38)}╯`); - expect(lines.join("\n")).toContain("No reviews yet."); + expect(lines.join("\n")).toContain("Nothing to show yet."); expect(lines.at(-2)).toContain("alt+r hide"); for (const line of lines) expect(visibleWidth(line)).toBe(40); }); - it("narrates every review outcome and finding verdict", () => { + it("shows only running reviews and those with accepted or rejected findings", () => { const feed = new ReviewFeed(); const outcomes = [ "failed", @@ -59,40 +60,48 @@ describe("renderSidebar", () => { "cancelled", "success", ] as const; - for (const [index, outcome] of outcomes.entries()) { - feed.start(outcome, job, index); + for (const outcome of outcomes) { + feed.start(outcome, { ...job, file: `${outcome}.ts` }, 0); feed.finish(outcome, outcome, 1500); } - feed.start("one", job, 0); - feed.finish("one", "success", 12_400); - feed.attach("one", ["awaiting", "missing"]); - feed.start("many", job, 0); - feed.finish("many", "success", 900); - feed.attach("many", Object.keys(views)); - feed.start("live", { ...job, model: "" }, 19_000); + feed.start("undecided", { ...job, file: "undecided.ts" }, 0); + feed.finish("undecided", "success", 1); + feed.attach("undecided", ["awaiting", "queued", "discarded", "missing"]); + feed.start("rejected", { ...job, file: "rejected.ts" }, 0); + feed.finish("rejected", "success", 12_400); + feed.attach("rejected", ["rejected"]); + feed.start("mixed", { ...job, file: "mixed.ts" }, 0); + feed.finish("mixed", "success", 900); + feed.attach("mixed", Object.keys(views)); + feed.start("live", { ...job, file: "live.ts", model: "" }, 19_000); const rendered = text(feed, 60, 80).join("\n"); for (const expected of [ "Live reviews · 1 running", + "live.ts", "reviewing…", "1.0s", - "review failed", - "timed out", - "superseded by a newer edit", - "cancelled", - "no findings", - "1 finding", + "rejected.ts", "12s", - "5 findings", + "1 rejected", + "mixed.ts", + "2 accepted · 1 rejected", + "Bare accept :4", "Duplicate helper :42", - "awaiting decision", "“Reuse the existing padding helper", - "queued", - "rejected", + "Rejected :2", "“No”", - "discarded", "gpt-5", ]) expect(rendered).toContain(expected); + for (const hidden of [ + ...outcomes.map((outcome) => `${outcome}.ts`), + "undecided.ts", + "Magic interval", + "Queued :1", + "Discarded", + "no findings", + ]) + expect(rendered).not.toContain(hidden); expect(renderSidebar([], lookup, plain, 40, 10).join("")).toContain( "Live reviews", ); @@ -103,14 +112,14 @@ describe("renderSidebar", () => { for (let index = 0; index < 6; index += 1) { feed.start(String(index), { ...job, file: `f${String(index)}.ts` }, 0); feed.finish(String(index), "success", 1); + feed.attach(String(index), ["accepted"]); } const lines = text(feed, 40, 16); const joined = lines.join("\n"); expect(lines.length).toBeLessThanOrEqual(16); expect(joined).toContain("f5.ts"); - expect(joined).not.toContain("f2.ts"); - expect(joined).toContain("f3.ts"); - expect(joined).toContain("+3 earlier reviews"); + expect(joined).not.toContain("f0.ts"); + expect(joined).toMatch(/\+\d earlier reviews/u); const single = new ReviewFeed(); single.start("a", job, 0); single.start("b", job, 0); @@ -140,10 +149,10 @@ describe("renderSidebar", () => { }, 0, ); - feed.finish("a", "success", 1); const lines = text(feed, 40, 40).map((line) => line.slice(2, -2)); - expect(lines[5]).toContain("no findings"); - expect(lines[5]?.trimEnd()).toMatch(/nam…$/u); + expect(lines[5]).toContain("reviewing…"); + expect(lines[5]).toContain("a-very-long"); + expect(lines[5]?.trimEnd()).toMatch(/…$/u); expect(lines[6]?.trim()).toBe(""); for (const width of [38, 12]) for (const line of text(feed, width, 40)) @@ -245,7 +254,7 @@ describe("ReviewSidebar", () => { }); expect(tui.options().visible(MIN_COLUMNS - 1)).toBe(false); expect(tui.options().visible(MIN_COLUMNS)).toBe(true); - expect(tui.render(40)).toHaveLength(9); + expect(tui.render(40)).toHaveLength(10); vi.advanceTimersByTime(250); expect(tui.requestRender).not.toHaveBeenCalled(); feed.start("a", job, Date.now()); From e47cda512f66dca43846adf13d641278fcf7ebd5 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 18:37:27 +0200 Subject: [PATCH 08/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20render=20accepted?= =?UTF-8?q?=20findings=20as=20compact=20chat=20cards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Accepted findings carry structured details and render as a title plus file-name-first location; Pi's expand toggle reveals the evidence and the agent's reason. Older messages and hosts without message renderers keep the default rendering. --- src/accepted-card.ts | 75 ++++++++++++++++++++++++++++++++++ src/index.ts | 18 ++++++++- test/accepted-card.test.ts | 82 ++++++++++++++++++++++++++++++++++++++ test/extension.test.ts | 16 ++++++++ 4 files changed, 190 insertions(+), 1 deletion(-) create mode 100644 src/accepted-card.ts create mode 100644 test/accepted-card.test.ts diff --git a/src/accepted-card.ts b/src/accepted-card.ts new file mode 100644 index 0000000..6d94233 --- /dev/null +++ b/src/accepted-card.ts @@ -0,0 +1,75 @@ +import type { MessageRenderer, Theme } from "@earendil-works/pi-coding-agent"; +import { + sliceByColumn, + visibleWidth, + wrapTextWithAnsi, +} from "@earendil-works/pi-tui"; +import { z } from "zod"; +import { truncatePath } from "./review-sidebar.js"; + +export const ACCEPTED_MESSAGE = "pair-programmer-accepted"; + +const AcceptedDetails = z.object({ + title: z.string(), + file: z.string(), + line: z.number(), + evidence: z.string(), + reason: z.string(), +}); +export type AcceptedDetails = z.infer; + +type Painter = Pick; + +/** Collapsed: title and location. Expanded: also evidence and the agent's reason. */ +export function acceptedLines( + details: AcceptedDetails, + theme: Painter, + width: number, + expanded: boolean, + pad = 0, +): string[] { + const indent = " ".repeat(Math.max(0, Math.min(pad, width - 4))); + const inner = Math.max(1, width - visibleWidth(indent) - 2); + const location = `:${String(details.line)}`; + const path = truncatePath( + details.file, + Math.max(1, inner - visibleWidth(location)), + ); + const title = wrapTextWithAnsi(details.title, inner); + const lines = [ + ...title.map( + (part, index) => + `${index === 0 ? theme.fg("warning", "◆") : " "} ${theme.bold(part)}`, + ), + ` ${theme.fg("dim", path + location)}`, + ]; + if (expanded) + lines.push( + ...wrapTextWithAnsi(details.evidence, inner).map( + (part) => ` ${theme.fg("muted", part)}`, + ), + ...wrapTextWithAnsi(`“${details.reason}”`, inner).map( + (part) => ` ${theme.fg("dim", part)}`, + ), + ); + return lines.map((line) => sliceByColumn(indent + line, 0, width)); +} + +/** Renders accepted findings as a compact card; unknown payloads fall back to Pi's default. */ +export const renderAccepted: MessageRenderer = (message, options, theme) => { + const parsed = AcceptedDetails.safeParse(message.details); + if (!parsed.success) return; + return { + render: (width: number): string[] => + acceptedLines( + parsed.data, + theme, + width, + options.expanded, + options.outputPad, + ), + invalidate(): void { + return; + }, + }; +}; diff --git a/src/index.ts b/src/index.ts index 7e8f431..f5d4dbc 100644 --- a/src/index.ts +++ b/src/index.ts @@ -12,6 +12,11 @@ import { captureBaseline, type TaskBaseline, } from "./change-evidence.js"; +import { + ACCEPTED_MESSAGE, + renderAccepted, + type AcceptedDetails, +} from "./accepted-card.js"; import { FindingAdmission } from "./finding-admission.js"; import { createPairLogger, type PairLogger } from "./logger.js"; import type { ModelCallObserver } from "./model-usage.js"; @@ -943,9 +948,16 @@ export default function pairProgrammer(pi: ExtensionAPI): void { if (saved && params.decision === "accept" && finding !== undefined) { pi.sendMessage( { - customType: "pair-programmer-accepted", + customType: ACCEPTED_MESSAGE, content: acceptedReview(finding, params.reason), display: true, + details: { + title: finding.title, + file: finding.file, + line: finding.line, + evidence: finding.evidence, + reason: params.reason, + } satisfies AcceptedDetails, }, { triggerTurn: false }, ); @@ -997,6 +1009,10 @@ export default function pairProgrammer(pi: ExtensionAPI): void { checkReset?.(); sidebar.toggle(ctx); }; + // OMP may not expose message renderers; its default rendering stays readable. + if (typeof pi.registerMessageRenderer === "function") + pi.registerMessageRenderer(ACCEPTED_MESSAGE, renderAccepted); + pi.registerCommand("pair-feed", { description: "Toggle the live review sidebar", handler: (_args, ctx) => { diff --git a/test/accepted-card.test.ts b/test/accepted-card.test.ts new file mode 100644 index 0000000..359c1a8 --- /dev/null +++ b/test/accepted-card.test.ts @@ -0,0 +1,82 @@ +import type { MessageRenderer } from "@earendil-works/pi-coding-agent"; +import { visibleWidth } from "@earendil-works/pi-tui"; +import { describe, expect, it } from "vitest"; +import { acceptedLines, renderAccepted } from "../src/accepted-card.js"; + +const theme = { + fg: (color: string, text: string) => `<${color}>${text}`, + bold: (text: string) => `*${text}`, +}; +const plain = { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, +}; +const details = { + title: "Imperative loop used to construct collection", + file: "apps/hush/lib/pages/chat_detail/widgets/message_bubble/linkified_message_text.dart", + line: 91, + evidence: "The changed code builds TextSpan children with a collection-for.", + reason: "Reworking it for the unsafe-index warning.", +}; +type Args = Parameters; +const render = ( + message: Partial, + expanded: boolean, + width = 120, +): string[] | undefined => + renderAccepted( + message as Args[0], + { expanded, outputPad: 1 }, + theme as unknown as Args[2], + )?.render(width); + +describe("accepted finding card", () => { + it("collapses to the title and a file-name-first location", () => { + expect(render({ details }, false)).toEqual([ + " ◆ *Imperative loop used to construct collection", + ` ${details.file}:91`, + ]); + expect(acceptedLines(details, plain, 70, false, 1)).toEqual([ + " ◆ Imperative loop used to construct collection", + " …/chat_detail/widgets/message_bubble/linkified_message_text.dart:91", + ]); + }); + + it("reveals the evidence and quoted reason when expanded", () => { + const lines = render({ details }, true) ?? []; + expect(lines).toHaveLength(4); + expect(lines[2]).toBe( + " The changed code builds TextSpan children with a collection-for.", + ); + expect(lines[3]).toBe( + " “Reworking it for the unsafe-index warning.”", + ); + }); + + it("wraps within narrow widths and keeps the file name", () => { + for (const width of [3, 12, 24, 40]) { + const lines = acceptedLines(details, plain, width, true, 4); + for (const line of lines) + expect(visibleWidth(line)).toBeLessThanOrEqual(width); + } + const narrow = acceptedLines(details, plain, 24, false); + expect(narrow.at(-1)).toContain("…"); + expect(narrow.at(-1)).toContain(":91"); + expect(narrow.slice(1, -1).every((line) => line.startsWith(" "))).toBe( + true, + ); + }); + + it("falls back to Pi's default rendering for older or malformed messages", () => { + expect(render({}, false)).toBeUndefined(); + expect( + render({ details: { ...details, line: "91" } }, true), + ).toBeUndefined(); + const component = renderAccepted( + { details } as Args[0], + { expanded: false, outputPad: 0 }, + theme as unknown as Args[2], + ); + expect(() => component?.invalidate()).not.toThrow(); + }); +}); diff --git a/test/extension.test.ts b/test/extension.test.ts index d2b954b..2e55e60 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -213,6 +213,7 @@ async function setup( isIdle: Mock<() => boolean>; notify: ReturnType; shortcuts: Map void>; + renderers: Map; setWidget: Mock<(key: string, content: string[] | undefined) => void>; status: () => string | undefined; entries: JournalEntry[]; @@ -251,6 +252,7 @@ async function setup( } const hooks = new Map(); const shortcuts = new Map void>(); + const renderers = new Map(); const commands = new Map< string, Parameters[1] @@ -287,6 +289,9 @@ async function setup( sendMessage, ...(host === "pi" ? { + registerMessageRenderer(customType: string, renderer: unknown) { + renderers.set(customType, renderer); + }, registerShortcut( key: string, options: { handler: (ctx: ExtensionContext) => void }, @@ -354,6 +359,7 @@ async function setup( notify, setWidget, shortcuts, + renderers, status: () => setWidget.mock.lastCall?.[1]?.[0], entries, statsEntries, @@ -2885,6 +2891,16 @@ it("restores delivered decisions after session start and removes decided finding expect(published[0]?.[0].content).toContain("Confirmed by caller"); expect(published[0]?.[0].content).toContain("First issue"); expect(published[0]?.[0].content).toContain("change.ts:1"); + expect(published[0]?.[0].details).toEqual({ + title: first.title, + file: "change.ts", + line: 1, + evidence: "broken — First issue", + reason: "Confirmed by caller", + }); + expect(environment.renderers.get("pair-programmer-accepted")).toBeTypeOf( + "function", + ); expect( ( await environment.decide("decision", { From ad6c2f0504db0c39c183cef68ce5a9b9b55deee2 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sat, 26 Sep 2026 20:27:51 +0200 Subject: [PATCH 09/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20expand=20accepted?= =?UTF-8?q?=20finding=20cards=20by=20click?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Left-clicking a card toggles its evidence and reason, with a ▸/▾ affordance. The toggle survives Pi's component rebuilds and yields to a later global ctrl+o. Requires fullscreen mode, where Pi routes mouse events. --- src/accepted-card.ts | 29 +++++++++++++++------- test/accepted-card.test.ts | 51 +++++++++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 12 deletions(-) diff --git a/src/accepted-card.ts b/src/accepted-card.ts index 6d94233..3e0f5b7 100644 --- a/src/accepted-card.ts +++ b/src/accepted-card.ts @@ -35,11 +35,12 @@ export function acceptedLines( details.file, Math.max(1, inner - visibleWidth(location)), ); - const title = wrapTextWithAnsi(details.title, inner); + const title = wrapTextWithAnsi(details.title, Math.max(1, inner - 2)); + const marker = theme.fg("dim", expanded ? " ▾" : " ▸"); const lines = [ ...title.map( (part, index) => - `${index === 0 ? theme.fg("warning", "◆") : " "} ${theme.bold(part)}`, + `${index === 0 ? theme.fg("warning", "◆") : " "} ${theme.bold(part)}${index === title.length - 1 ? marker : ""}`, ), ` ${theme.fg("dim", path + location)}`, ]; @@ -55,19 +56,29 @@ export function acceptedLines( return lines.map((line) => sliceByColumn(indent + line, 0, width)); } +/** + * A card's own click toggle, remembered per message because Pi rebuilds the + * component on every expand or theme change. `base` records the global expand + * state when clicked, so a later ctrl+o overrides the local toggle. + */ +const toggled = new WeakMap(); + /** Renders accepted findings as a compact card; unknown payloads fall back to Pi's default. */ export const renderAccepted: MessageRenderer = (message, options, theme) => { const parsed = AcceptedDetails.safeParse(message.details); if (!parsed.success) return; + const open = (): boolean => { + const state = toggled.get(message); + return state?.base === options.expanded ? state.open : options.expanded; + }; return { render: (width: number): string[] => - acceptedLines( - parsed.data, - theme, - width, - options.expanded, - options.outputPad, - ), + acceptedLines(parsed.data, theme, width, open(), options.outputPad), + handleMouse(event) { + if (event.type !== "click" || event.button !== "left") return; + toggled.set(message, { open: !open(), base: options.expanded }); + return { handled: true, render: true }; + }, invalidate(): void { return; }, diff --git a/test/accepted-card.test.ts b/test/accepted-card.test.ts index 359c1a8..d9fdc60 100644 --- a/test/accepted-card.test.ts +++ b/test/accepted-card.test.ts @@ -1,5 +1,9 @@ import type { MessageRenderer } from "@earendil-works/pi-coding-agent"; -import { visibleWidth } from "@earendil-works/pi-tui"; +import { + visibleWidth, + type Component, + type TuiMouseEvent, +} from "@earendil-works/pi-tui"; import { describe, expect, it } from "vitest"; import { acceptedLines, renderAccepted } from "../src/accepted-card.js"; @@ -33,11 +37,11 @@ const render = ( describe("accepted finding card", () => { it("collapses to the title and a file-name-first location", () => { expect(render({ details }, false)).toEqual([ - " ◆ *Imperative loop used to construct collection", + " ◆ *Imperative loop used to construct collection ▸", ` ${details.file}:91`, ]); expect(acceptedLines(details, plain, 70, false, 1)).toEqual([ - " ◆ Imperative loop used to construct collection", + " ◆ Imperative loop used to construct collection ▸", " …/chat_detail/widgets/message_bubble/linkified_message_text.dart:91", ]); }); @@ -67,6 +71,47 @@ describe("accepted finding card", () => { ); }); + it("toggles by left click, survives rebuilds, and yields to the global expand", () => { + const message = { details } as Args[0]; + const card = (expanded: boolean): Component | undefined => + renderAccepted( + message, + { expanded, outputPad: 0 }, + plain as unknown as Args[2], + ); + const click = { type: "click", button: "left" } as TuiMouseEvent; + const collapsed = card(false); + expect(collapsed?.render(120)).toHaveLength(2); + const ignored: TuiMouseEvent[] = [ + { ...click, button: "right" }, + { ...click, type: "press" }, + { ...click, type: "wheel" }, + ]; + for (const event of ignored) + expect(collapsed?.handleMouse?.(event)).toBeUndefined(); + expect(collapsed?.handleMouse?.(click)).toEqual({ + handled: true, + render: true, + }); + expect(collapsed?.render(120)).toHaveLength(4); + expect(collapsed?.render(120)[0]).toContain("▾"); + expect(card(false)?.render(120)).toHaveLength(4); + card(false)?.handleMouse?.(click); + expect(card(false)?.render(120)).toHaveLength(2); + card(false)?.handleMouse?.(click); + expect(card(true)?.render(120)).toHaveLength(4); + card(true)?.handleMouse?.(click); + expect(card(true)?.render(120)).toHaveLength(2); + expect(card(false)?.render(120)).toHaveLength(2); + expect( + renderAccepted( + { details } as Args[0], + { expanded: true, outputPad: 0 }, + plain as unknown as Args[2], + )?.render(120), + ).toHaveLength(4); + }); + it("falls back to Pi's default rendering for older or malformed messages", () => { expect(render({}, false)).toBeUndefined(); expect( From 112fbda7060ba40a9989c5d14b32bda9aa503198 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sun, 27 Sep 2026 01:37:35 +0200 Subject: [PATCH 10/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20name=20reviewers?= =?UTF-8?q?=20in=20the=20sidebar=20and=20chat=20cards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reviewers accept an optional 1-24 character name (default reviewer: entropy) that labels sidebar rows and accepted cards, falling back to the short model name. Names are not part of reviewer identity. --- README.md | 3 ++- src/accepted-card.ts | 6 ++++-- src/index.ts | 12 ++++++++++++ src/review-feed.ts | 4 +++- src/review-sidebar.ts | 2 +- src/reviewers.ts | 19 +++++++++++++++++++ test/accepted-card.test.ts | 3 +++ test/extension.test.ts | 37 +++++++++++++++++++++++++++++++++++++ test/review-feed.test.ts | 1 + test/review-sidebar.test.ts | 8 +++++--- test/reviewers.test.ts | 33 +++++++++++++++++++++++++++++++++ 11 files changed, 120 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 413465e..630855c 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ The default reviewer uses your current model and asks: “Does it add entropy? { "reviewers": [ { + "name": "entropy", "model": "current", "prompt": "Does it add entropy?", "include": ["src/**/*.ts"], @@ -63,7 +64,7 @@ The default reviewer uses your current model and asks: “Does it add entropy? } ``` -Use `current` or a `provider/model` identifier. File patterns are relative to your working directory; exclusions take precedence. Add entries for more reviewers. +The optional `name` (1–24 characters) labels the reviewer in the sidebar and chat cards; unnamed reviewers show their model name, and renaming a reviewer keeps its review history. Use `current` or a `provider/model` identifier. File patterns are relative to your working directory; exclusions take precedence. Add entries for more reviewers. A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. diff --git a/src/accepted-card.ts b/src/accepted-card.ts index 3e0f5b7..6268063 100644 --- a/src/accepted-card.ts +++ b/src/accepted-card.ts @@ -15,6 +15,7 @@ const AcceptedDetails = z.object({ line: z.number(), evidence: z.string(), reason: z.string(), + reviewer: z.string().optional(), }); export type AcceptedDetails = z.infer; @@ -31,9 +32,10 @@ export function acceptedLines( const indent = " ".repeat(Math.max(0, Math.min(pad, width - 4))); const inner = Math.max(1, width - visibleWidth(indent) - 2); const location = `:${String(details.line)}`; + const by = details.reviewer === undefined ? "" : ` · ${details.reviewer}`; const path = truncatePath( details.file, - Math.max(1, inner - visibleWidth(location)), + Math.max(1, inner - visibleWidth(location + by)), ); const title = wrapTextWithAnsi(details.title, Math.max(1, inner - 2)); const marker = theme.fg("dim", expanded ? " ▾" : " ▸"); @@ -42,7 +44,7 @@ export function acceptedLines( (part, index) => `${index === 0 ? theme.fg("warning", "◆") : " "} ${theme.bold(part)}${index === title.length - 1 ? marker : ""}`, ), - ` ${theme.fg("dim", path + location)}`, + ` ${theme.fg("dim", path + location + by)}`, ]; if (expanded) lines.push( diff --git a/src/index.ts b/src/index.ts index f5d4dbc..6f2c1a1 100644 --- a/src/index.ts +++ b/src/index.ts @@ -42,6 +42,7 @@ import { loadReviewers, matchingReviewers, reviewerKey, + reviewerLabel, type ReviewerConfig, } from "./reviewers.js"; @@ -138,6 +139,10 @@ function describe(findings: readonly Finding[]): string { return `Review findings:\n${lines.join("\n")}\nAccept or reject every finding with pair_programmer_decide(findingId, decision, reason) before using another tool. Give a concrete reason for each decision.`; } +function optionalReviewer(name: string | undefined): { reviewer?: string } { + return name === undefined ? {} : { reviewer: name }; +} + function acceptedReview(finding: Finding, reason: string): string { return `**Accepted review · ${finding.title}**\n\n\`${finding.file}:${String(finding.line)}\`\n\n${finding.evidence}\n\n**Why accepted:** ${reason}`; } @@ -153,6 +158,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { const statsView = new StatsView(); const statusOverlay = new StatusOverlay(); const feed = new ReviewFeed(); + const reviewerNames = new Map(); const sidebar = new ReviewSidebar( () => feed.list(), (id) => store.lookup(id), @@ -412,8 +418,13 @@ export default function pairProgrammer(pi: ExtensionAPI): void { finish("cancelled", false); }); job.stats.start(job.id); + reviewerNames.set( + reviewerKey(job.reviewer), + reviewerLabel(job.reviewer, job.model), + ); feed.start(job.id, { file: job.file, + reviewer: reviewerLabel(job.reviewer, job.model), model: job.model, }); showState(job.ctx); @@ -957,6 +968,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { line: finding.line, evidence: finding.evidence, reason: params.reason, + ...optionalReviewer(reviewerNames.get(finding.reviewer)), } satisfies AcceptedDetails, }, { triggerTurn: false }, diff --git a/src/review-feed.ts b/src/review-feed.ts index f31fc98..f29a78c 100644 --- a/src/review-feed.ts +++ b/src/review-feed.ts @@ -5,6 +5,7 @@ export type ReviewPhase = "running" | ReviewOutcome; export interface FeedEntry { id: string; file: string; + reviewer: string; model: string; startedAt: number; phase: ReviewPhase; @@ -20,12 +21,13 @@ export class ReviewFeed { start( id: string, - job: { file: string; model: string }, + job: { file: string; reviewer: string; model: string }, now = Date.now(), ): void { this.entries.unshift({ id, file: job.file, + reviewer: job.reviewer, model: job.model.slice(job.model.lastIndexOf("/") + 1), startedAt: now, phase: "running", diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index f0e8397..b4e631d 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -117,7 +117,7 @@ function entryLines( ), spread( ` ${theme.fg(state.color, state.text)}`, - theme.fg("dim", entry.model), + theme.fg("dim", entry.reviewer), width, ), ]; diff --git a/src/reviewers.ts b/src/reviewers.ts index 646b47c..fce7850 100644 --- a/src/reviewers.ts +++ b/src/reviewers.ts @@ -5,6 +5,8 @@ import { minimatch } from "minimatch"; import { z } from "zod"; export interface ReviewerConfig { + /** Short display label; not part of the reviewer's identity. */ + name?: string | undefined; model: string; prompt: string; include: readonly string[]; @@ -13,6 +15,7 @@ export interface ReviewerConfig { export const DEFAULT_REVIEWERS: readonly ReviewerConfig[] = [ { + name: "entropy", model: "current", prompt: "Does it add entropy ?", include: ["**/*"], @@ -70,6 +73,17 @@ const PatternSchema = z.string().refine(validPattern, { "must be a nonempty project-relative POSIX glob with balanced syntax", }); const ReviewerSchema = z.strictObject({ + name: z + .string() + .refine( + (value) => + value.trim() === value && + value.length > 0 && + value.length <= 24 && + !/\p{Cc}/u.test(value), + { message: "must be 1-24 characters without surrounding spaces" }, + ) + .optional(), model: z.string().refine((value) => value.trim().length > 0), prompt: z.string().refine((value) => value.trim().length > 0), include: z.array(PatternSchema).min(1), @@ -151,6 +165,11 @@ export function matchingReviewers( ); } +/** The reviewer's name, or the short model name when unnamed. */ +export function reviewerLabel(reviewer: ReviewerConfig, model: string): string { + return reviewer.name ?? model.slice(model.lastIndexOf("/") + 1); +} + export function reviewerKey(reviewer: ReviewerConfig): string { return createHash("sha256") .update( diff --git a/test/accepted-card.test.ts b/test/accepted-card.test.ts index d9fdc60..46c4e10 100644 --- a/test/accepted-card.test.ts +++ b/test/accepted-card.test.ts @@ -55,6 +55,9 @@ describe("accepted finding card", () => { expect(lines[3]).toBe( " “Reworking it for the unsafe-index warning.”", ); + expect( + acceptedLines({ ...details, reviewer: "entropy" }, plain, 60, false)[1], + ).toBe(" …/message_bubble/linkified_message_text.dart:91 · entropy"); }); it("wraps within narrow widths and keeps the file name", () => { diff --git a/test/extension.test.ts b/test/extension.test.ts index 2e55e60..6f59247 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -2800,6 +2800,42 @@ it("feeds each review's outcome and attributed findings into the toggled sidebar await environment.emit("session_shutdown"); }); +it("omits the reviewer name from cards for findings restored from an earlier process", async () => { + const environment = await setup(); + const restored: Finding = { + id: "restored", + reviewer: "unknown-reviewer", + file: "change.ts", + revision: "r1", + line: 3, + title: "Restored", + evidence: "From before a reload", + }; + environment.entries.push( + { + type: "custom", + customType: "pair-programmer", + data: { action: "add", finding: restored }, + }, + { + type: "custom", + customType: "pair-programmer", + data: { action: "deliver", ids: ["restored"] }, + }, + ); + await environment.emit("session_start", { reason: "startup" }); + await environment.decide("restored", { + findingId: "restored", + decision: "accept", + reason: "Still valid", + }); + const card = environment.sendMessage.mock.calls.find( + ([message]) => message.customType === "pair-programmer-accepted", + ); + expect(card?.[0].details).not.toHaveProperty("reviewer"); + await environment.emit("session_shutdown"); +}); + it("narrates review progress above the editor without surfacing finding contents", async () => { const environment = await setup(); const file = path.join(environment.cwd, "change.ts"); @@ -2897,6 +2933,7 @@ it("restores delivered decisions after session start and removes decided finding line: 1, evidence: "broken — First issue", reason: "Confirmed by caller", + reviewer: "gpt-5", }); expect(environment.renderers.get("pair-programmer-accepted")).toBeTypeOf( "function", diff --git a/test/review-feed.test.ts b/test/review-feed.test.ts index 7bf7c6f..88998d5 100644 --- a/test/review-feed.test.ts +++ b/test/review-feed.test.ts @@ -3,6 +3,7 @@ import { ReviewFeed } from "../src/review-feed.js"; const job = { file: "src/a.ts", + reviewer: "entropy", model: "openai/gpt-5", }; diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index b155473..c5b96b9 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -16,6 +16,7 @@ const plain = { } as unknown as Parameters[2]; const job = { file: "src/a.ts", + reviewer: "entropy", model: "openai/gpt-5", }; const views: Record = { @@ -90,7 +91,7 @@ describe("renderSidebar", () => { "“Reuse the existing padding helper", "Rejected :2", "“No”", - "gpt-5", + "entropy", ]) expect(rendered).toContain(expected); for (const hidden of [ @@ -139,19 +140,20 @@ describe("renderSidebar", () => { expect(truncatePath("x.ts", 0)).toBe("…"); }); - it("shares a row between the outcome and a shortened model, without the prompt", () => { + it("shares a row between the outcome and a shortened reviewer name", () => { const feed = new ReviewFeed(); feed.start( "a", { ...job, - model: "provider/a-very-long-model-name-that-cannot-fit-beside-it", + reviewer: "a-very-long-reviewer-name-that-cannot-fit-beside-it", }, 0, ); const lines = text(feed, 40, 40).map((line) => line.slice(2, -2)); expect(lines[5]).toContain("reviewing…"); expect(lines[5]).toContain("a-very-long"); + expect(lines[5]).not.toContain("gpt-5"); expect(lines[5]?.trimEnd()).toMatch(/…$/u); expect(lines[6]?.trim()).toBe(""); for (const width of [38, 12]) diff --git a/test/reviewers.test.ts b/test/reviewers.test.ts index 325fa1a..14edfc6 100644 --- a/test/reviewers.test.ts +++ b/test/reviewers.test.ts @@ -7,6 +7,7 @@ import { loadReviewers, matchingReviewers, reviewerKey, + reviewerLabel, type ReviewerConfig, } from "../src/reviewers.js"; @@ -275,3 +276,35 @@ it("uses semantic reviewer identity regardless of pattern order", () => { expect(key).not.toBe(reviewerKey({ ...first, include: ["src/**/*.ts"] })); expect(key).not.toBe(reviewerKey({ ...first, exclude: [] })); }); + +it("accepts short reviewer names that label without changing identity", async () => { + const directory = await temporaryDirectory(); + const file = path.join(directory, "pair-programmer.reviewers.json"); + const named: ReviewerConfig = { + name: "security", + model: "provider/model", + prompt: "Is it exploitable?", + include: ["**/*"], + exclude: [], + }; + await writeFile(file, JSON.stringify({ reviewers: [named] })); + expect(await loadReviewers(directory)).toEqual([named]); + for (const name of ["", " padded ", "x".repeat(25), "bad\u{7}"]) { + await writeFile(file, JSON.stringify({ reviewers: [{ ...named, name }] })); + await expect(loadReviewers(directory)).rejects.toThrow( + "reviewers[0].name must be 1-24 characters", + ); + } + const unnamed: ReviewerConfig = { + model: named.model, + prompt: named.prompt, + include: named.include, + exclude: named.exclude, + }; + expect(reviewerKey(named)).toBe(reviewerKey(unnamed)); + expect(reviewerKey({ ...named, name: "renamed" })).toBe(reviewerKey(named)); + expect(reviewerLabel(named, "provider/model")).toBe("security"); + expect(reviewerLabel(unnamed, "openai/gpt-5")).toBe("gpt-5"); + expect(reviewerLabel(unnamed, "local")).toBe("local"); + expect(DEFAULT_REVIEWERS[0]?.name).toBe("entropy"); +}); From 2d8d56a53dee26edbd7f6ca9f9034df672be9b98 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sun, 27 Sep 2026 01:42:29 +0200 Subject: [PATCH 11/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20collapse=20sidebar?= =?UTF-8?q?=20reviews=20and=20expand=20them=20by=20click?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Each review collapses to one row (file, reviewer, verdict counts ▸); a left click on any of its rows toggles duration, findings and reasons. Expanded reviews that no longer fit fall back to their collapsed row. --- README.md | 2 +- src/review-sidebar.ts | 150 +++++++++++++++++++++++++----------- test/extension.test.ts | 4 +- test/review-sidebar.test.ts | 83 ++++++++++++++++---- 4 files changed, 172 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index 630855c..feec145 100644 --- a/README.md +++ b/README.md @@ -68,6 +68,6 @@ The optional `name` (1–24 characters) labels the reviewer in the sidebar and c A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. -- `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first, each with its file, model and duration, plus every decided finding and the agent's reason. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. +- `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first. Each review is one row with its file, reviewer and verdict counts; click it (fullscreen mode) to show its duration, decided findings and the agent's reasons. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. - `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. - Diagnostics stay in files, never the terminal: OMP uses its native logs; Pi uses `~/.pi/agent/logs/pair-programmer/` (or `$PI_CODING_AGENT_DIR/logs/pair-programmer/`). diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index b4e631d..8854335 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -35,12 +35,15 @@ export function truncatePath(file: string, width: number): string { return start === -1 ? "…" : `…${graphemes.slice(start).join("")}`; } +/** Cuts text to `width` columns with a plain ellipsis (no stray reset codes). */ +function clip(text: string, width: number): string { + if (visibleWidth(text) <= width) return text; + return width <= 0 ? "" : `${sliceByColumn(text, 0, width - 1).trimEnd()}…`; +} + /** Joins left and right text on one row, shrinking the right side first. */ function spread(left: string, right: string, width: number): string { - const room = Math.max(0, width - visibleWidth(left) - 1); - let shown = right; - if (visibleWidth(right) > room) - shown = room === 0 ? "" : `${sliceByColumn(right, 0, room - 1).trimEnd()}…`; + const shown = clip(right, Math.max(0, width - visibleWidth(left) - 1)); const gap = Math.max(1, width - visibleWidth(left) - visibleWidth(shown)); return `${left}${" ".repeat(gap)}${shown}`; } @@ -72,6 +75,8 @@ function summary(findings: readonly Decided[]): { icon: string; color: Color; text: string; + accepted: number; + rejected: number; } { const accepted = findings.filter((f) => f.status === "accepted").length; const rejected = findings.length - accepted; @@ -82,45 +87,50 @@ function summary(findings: readonly Decided[]): { .filter(Boolean) .join(" · "); return accepted > 0 - ? { icon: "◆", color: "warning", text } - : { icon: "✗", color: "muted", text }; + ? { icon: "◆", color: "warning", text, accepted, rejected } + : { icon: "✗", color: "muted", text, accepted, rejected }; +} + +/** Compact verdict counts for the collapsed row, e.g. `1✓ 2✗`. */ +function counts(theme: Painter, accepted: number, rejected: number): string { + return [ + accepted > 0 ? theme.fg("success", `${String(accepted)}✓`) : "", + rejected > 0 ? theme.fg("muted", `${String(rejected)}✗`) : "", + ] + .filter(Boolean) + .join(" "); } function entryLines( entry: FeedEntry, findings: readonly Decided[], + open: boolean, theme: Painter, width: number, now: number, ): string[] { - const state = - entry.phase === "running" - ? { - icon: SPINNER.charAt(Math.floor(now / 250) % SPINNER.length), - color: "accent" as const, - text: "reviewing…", - } - : summary(findings); + const running = entry.phase === "running"; + const state = summary(findings); + const icon = running + ? theme.fg("accent", SPINNER.charAt(Math.floor(now / 250) % SPINNER.length)) + : theme.fg(state.color, state.icon); const time = theme.fg( "dim", seconds(entry.durationMs ?? Math.max(0, now - entry.startedAt)), ); + const marker = theme.fg("dim", open ? "▾" : "▸"); + const tail = running + ? time + : `${counts(theme, state.accepted, state.rejected)} ${marker}`; + const name = clip(entry.reviewer, Math.max(1, Math.floor(width / 3))); + const right = `${theme.fg("dim", name)} ${tail}`; const file = truncatePath( entry.file, - Math.max(1, width - visibleWidth(time) - 3), + Math.max(1, width - visibleWidth(right) - 3), ); - const lines = [ - spread( - `${theme.fg(state.color, state.icon)} ${theme.bold(file)}`, - time, - width, - ), - spread( - ` ${theme.fg(state.color, state.text)}`, - theme.fg("dim", entry.reviewer), - width, - ), - ]; + const lines = [spread(`${icon} ${theme.bold(file)}`, right, width)]; + if (running || !open) return lines; + lines.push(spread(` ${theme.fg(state.color, state.text)}`, time, width)); for (const finding of findings) { const style = FINDING[finding.status]; lines.push( @@ -136,15 +146,22 @@ function entryLines( return lines; } -/** Renders the framed sidebar at an exact width, at most `height` rows. */ -export function renderSidebar( +export interface SidebarLayout { + lines: string[]; + /** Review id owning each rendered row, for click targeting. */ + rows: (string | undefined)[]; +} + +/** Lays out the framed sidebar at an exact width, at most `height` rows. */ +export function layoutSidebar( entries: readonly FeedEntry[], lookup: Lookup, theme: Painter, width: number, height: number, now = Date.now(), -): string[] { + expanded: ReadonlySet = new Set(), +): SidebarLayout { const inner = Math.max(1, width - 4); const visible = entries.flatMap((entry) => { const findings = decided(entry, lookup); @@ -168,24 +185,32 @@ export function renderSidebar( ), "", ]; - const footer = ["", theme.fg("dim", "alt+r hide · /pair-stats totals")]; + const footer = ["", theme.fg("dim", "click to expand · alt+r hide")]; const room = Math.max(0, height - 2 - header.length - footer.length); const body: string[] = []; - if (visible.length === 0) + const owners: (string | undefined)[] = []; + if (visible.length === 0) { body.push( theme.fg("muted", "Nothing to show yet."), theme.fg("dim", "Running reviews and decided"), theme.fg("dim", "findings appear here."), ); + owners.push(undefined, undefined, undefined); + } let shown = 0; for (const { entry, findings } of visible) { - const block = [ - ...(shown === 0 ? [] : [""]), - ...entryLines(entry, findings, theme, inner, now), - ]; + const gap = shown === 0 ? [] : [""]; const reserve = shown + 1 < visible.length ? 1 : 0; - if (body.length + block.length + reserve > room) break; - body.push(...block); + const fits = (lines: readonly string[]): boolean => + body.length + gap.length + lines.length + reserve <= room; + const open = expanded.has(entry.id); + let lines = entryLines(entry, findings, open, theme, inner, now); + // An expanded review that no longer fits falls back to its collapsed row. + if (open && !fits(lines)) + lines = entryLines(entry, findings, false, theme, inner, now); + if (!fits(lines)) break; + body.push(...gap, ...lines); + owners.push(...gap.map(() => ""), ...lines.map(() => entry.id)); shown += 1; } if (shown < visible.length) { @@ -196,13 +221,28 @@ export function renderSidebar( `+${String(hidden)} earlier review${hidden === 1 ? "" : "s"}`, ), ); + owners.push(undefined); } const content = [...header, ...body].slice(0, room + header.length); - return [ - theme.fg("borderMuted", `╭${"─".repeat(Math.max(0, width - 2))}╮`), - ...[...content, ...footer].map(row), - theme.fg("borderMuted", `╰${"─".repeat(Math.max(0, width - 2))}╯`), - ]; + return { + lines: [ + theme.fg("borderMuted", `╭${"─".repeat(Math.max(0, width - 2))}╮`), + ...[...content, ...footer].map(row), + theme.fg("borderMuted", `╰${"─".repeat(Math.max(0, width - 2))}╯`), + ], + rows: [ + ...Array.from({ length: 1 + header.length }), + ...owners.slice(0, content.length - header.length), + ...Array.from({ length: footer.length + 1 }), + ], + }; +} + +/** Renders the framed sidebar at an exact width, at most `height` rows. */ +export function renderSidebar( + ...args: Parameters +): string[] { + return layoutSidebar(...args).lines; } /** Togglable, non-capturing right-hand overlay showing the live review feed. */ @@ -213,6 +253,8 @@ export class ReviewSidebar { private close: (() => void) | undefined; private requestRender: (() => void) | undefined; private ticker: NodeJS.Timeout | undefined; + /** Reviews the user expanded by click; ids are unique per review. */ + private readonly expanded = new Set(); constructor(entries: () => readonly FeedEntry[], lookup: Lookup) { this.entries = entries; @@ -277,15 +319,31 @@ export class ReviewSidebar { tui.requestRender(); }, 250); this.ticker.unref(); + let rows: SidebarLayout["rows"] = []; return { - render: (width: number): string[] => - renderSidebar( + render: (width: number): string[] => { + const layout = layoutSidebar( this.entries(), this.lookup, theme, width, Math.max(8, tui.terminal.rows - EDITOR_RESERVE), - ), + Date.now(), + this.expanded, + ); + rows = layout.rows; + return layout.lines; + }, + handleMouse: (event) => { + if (event.type !== "click" || event.button !== "left") return; + const id = rows[event.y]; + const entry = this.entries().find( + (candidate) => candidate.id === id, + ); + if (entry === undefined || entry.phase === "running") return; + if (!this.expanded.delete(entry.id)) this.expanded.add(entry.id); + return { handled: true, render: true }; + }, invalidate(): void { return; }, diff --git a/test/extension.test.ts b/test/extension.test.ts index 6f59247..56be024 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -2790,9 +2790,7 @@ it("feeds each review's outcome and attributed findings into the toggled sidebar }); const decidedFeed = render?.(60).join("\n") ?? ""; expect(decidedFeed).toContain("change.ts"); - expect(decidedFeed).toContain("1 accepted"); - expect(decidedFeed).toContain("Sidebar issue :1"); - expect(decidedFeed).toContain("“Real issue”"); + expect(decidedFeed).toContain("gpt-5 1✓ ▸"); await environment.command("pair-clear"); expect(render?.(60).join("\n")).toContain("Nothing to show yet."); await environment.command("pair-feed"); diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index c5b96b9..47c9083 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -1,5 +1,5 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; -import { visibleWidth } from "@earendil-works/pi-tui"; +import { visibleWidth, type TuiMouseEvent } from "@earendil-works/pi-tui"; import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; import { ReviewFeed } from "../src/review-feed.js"; import { @@ -39,7 +39,9 @@ const text = ( width = 48, height = 60, now = 20_000, -): string[] => renderSidebar(feed.list(), lookup, plain, width, height, now); + expanded: ReadonlySet = new Set(feed.list().map(({ id }) => id)), +): string[] => + renderSidebar(feed.list(), lookup, plain, width, height, now, expanded); describe("renderSidebar", () => { it("frames an empty feed at its natural height", () => { @@ -79,7 +81,6 @@ describe("renderSidebar", () => { for (const expected of [ "Live reviews · 1 running", "live.ts", - "reviewing…", "1.0s", "rejected.ts", "12s", @@ -124,9 +125,7 @@ describe("renderSidebar", () => { const single = new ReviewFeed(); single.start("a", job, 0); single.start("b", job, 0); - expect(text(single, 40, 10).join("\n")).toContain( - "+1 earlier review\u{20}", - ); + expect(text(single, 40, 9).join("\n")).toContain("+1 earlier review\u{20}"); for (const line of text(feed, 38, 5)) expect(visibleWidth(line)).toBe(38); }); @@ -140,22 +139,38 @@ describe("renderSidebar", () => { expect(truncatePath("x.ts", 0)).toBe("…"); }); - it("shares a row between the outcome and a shortened reviewer name", () => { + it("collapses each review to one row of file, reviewer and verdict counts", () => { const feed = new ReviewFeed(); + feed.start("done", { ...job, file: "src/deep/done.ts" }, 0); + feed.finish("done", "success", 8400); + feed.attach("done", ["accepted", "rejected"]); feed.start( - "a", + "live", { ...job, + file: "src/live.ts", reviewer: "a-very-long-reviewer-name-that-cannot-fit-beside-it", }, - 0, + 19_000, + ); + const collapsed = text(feed, 44, 40, 20_000, new Set()).map((line) => + line.slice(2, -2), + ); + expect(collapsed[4]).toMatch(/^◐ src\/live\.ts +a-very-long-… {2}1\.0s$/u); + expect(collapsed[6]).toMatch( + /^◆ src\/deep\/done\.ts +entropy {2}1✓ 1✗ ▸$/u, ); - const lines = text(feed, 40, 40).map((line) => line.slice(2, -2)); - expect(lines[5]).toContain("reviewing…"); - expect(lines[5]).toContain("a-very-long"); - expect(lines[5]).not.toContain("gpt-5"); - expect(lines[5]?.trimEnd()).toMatch(/…$/u); - expect(lines[6]?.trim()).toBe(""); + expect(collapsed[7]?.trim()).toBe(""); + const open = text(feed, 44, 40).map((line) => line.slice(2, -2)); + expect(open[4]).toContain("1.0s"); + expect(open[5]?.trim()).toBe(""); + expect(open[6]).toMatch(/1✓ 1✗ ▾$/u); + expect(open[7]).toMatch(/^ {2}1 accepted · 1 rejected +8\.4s$/u); + const rejectedOnly = new ReviewFeed(); + rejectedOnly.start("r", job, 0); + rejectedOnly.finish("r", "success", 1); + rejectedOnly.attach("r", ["rejected"]); + expect(text(rejectedOnly, 44, 40, 0, new Set())[4]).toMatch(/✗ ▸/u); for (const width of [38, 12]) for (const line of text(feed, width, 40)) expect(visibleWidth(line)).toBe(width); @@ -185,14 +200,16 @@ interface Host { visible: (columns: number) => boolean; }; render: (width: number) => string[] | undefined; + click: (y: number, button?: string, type?: string) => unknown; } -function host(mode = "tui"): Host { +function host(mode = "tui", rows = 20): Host { const requestRender = vi.fn(); const notify = vi.fn(); const closed = vi.fn(); let options: ReturnType | undefined; let render: ((width: number) => string[]) | undefined; + let mouse: ((event: TuiMouseEvent) => unknown) | undefined; const custom = vi.fn( async ( factory: Factory, @@ -203,7 +220,7 @@ function host(mode = "tui"): Host { const component = await factory( { requestRender, - terminal: { rows: 20 }, + terminal: { rows }, } as unknown as Parameters[0], plain as unknown as Parameters[1], {} as Parameters[2], @@ -214,6 +231,7 @@ function host(mode = "tui"): Host { ); component.invalidate(); render = (width) => component.render(width); + mouse = (event) => component.handleMouse?.(event); return done.promise; }, ); @@ -232,6 +250,8 @@ function host(mode = "tui"): Host { return options; }, render: (width) => render?.(width), + click: (y, button = "left", type = "click") => + mouse?.({ type, button, y } as TuiMouseEvent), }; } @@ -272,6 +292,35 @@ describe("ReviewSidebar", () => { expect(tui.requestRender).toHaveBeenCalledTimes(2); }); + it("expands and collapses a finished review by left click on any of its rows", async () => { + const feed = new ReviewFeed(); + feed.start("done", { ...job, file: "done.ts" }, 0); + feed.finish("done", "success", 1); + feed.attach("done", ["accepted"]); + feed.start("live", { ...job, file: "live.ts" }, Date.now()); + const tui = host("tui", 40); + const sidebar = new ReviewSidebar(() => feed.list(), lookup); + sidebar.toggle(tui.ctx); + await Promise.resolve(); + const rows = (): string[] => tui.render(40) ?? []; + const at = (name: string): number => + rows().findIndex((line) => line.includes(name)); + expect(rows().join("\n")).not.toContain("Duplicate helper"); + for (const ignored of [ + tui.click(0), + tui.click(at("live.ts")), + tui.click(at("done.ts"), "right"), + tui.click(at("done.ts"), "left", "press"), + tui.click(99), + ]) + expect(ignored).toBeUndefined(); + expect(tui.click(at("done.ts"))).toEqual({ handled: true, render: true }); + expect(rows().join("\n")).toContain("Duplicate helper :42"); + expect(tui.click(at("Duplicate helper"))).toMatchObject({ handled: true }); + expect(rows().join("\n")).not.toContain("Duplicate helper"); + sidebar.dispose(); + }); + it("follows session replacement while open", async () => { const first = host(); const second = host(); From 81651e946329019ecf24d14959f5a9d7905eff04 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sun, 27 Sep 2026 01:46:38 +0200 Subject: [PATCH 12/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20show=20rejected=20?= =?UTF-8?q?findings=20as=20folded=20chat=20cards?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Rejections are appended as transcript-only custom entries, so they never reach model context, and render as one dimmed line that expands by click or ctrl+o to location, evidence and reason. Accepted and rejected cards share one finding-card module. --- src/accepted-card.ts | 88 ------------ src/finding-card.ts | 130 ++++++++++++++++++ src/index.ts | 51 +++---- test/extension.test.ts | 31 +++++ ...pted-card.test.ts => finding-card.test.ts} | 54 +++++++- 5 files changed, 238 insertions(+), 116 deletions(-) delete mode 100644 src/accepted-card.ts create mode 100644 src/finding-card.ts rename test/{accepted-card.test.ts => finding-card.test.ts} (72%) diff --git a/src/accepted-card.ts b/src/accepted-card.ts deleted file mode 100644 index 6268063..0000000 --- a/src/accepted-card.ts +++ /dev/null @@ -1,88 +0,0 @@ -import type { MessageRenderer, Theme } from "@earendil-works/pi-coding-agent"; -import { - sliceByColumn, - visibleWidth, - wrapTextWithAnsi, -} from "@earendil-works/pi-tui"; -import { z } from "zod"; -import { truncatePath } from "./review-sidebar.js"; - -export const ACCEPTED_MESSAGE = "pair-programmer-accepted"; - -const AcceptedDetails = z.object({ - title: z.string(), - file: z.string(), - line: z.number(), - evidence: z.string(), - reason: z.string(), - reviewer: z.string().optional(), -}); -export type AcceptedDetails = z.infer; - -type Painter = Pick; - -/** Collapsed: title and location. Expanded: also evidence and the agent's reason. */ -export function acceptedLines( - details: AcceptedDetails, - theme: Painter, - width: number, - expanded: boolean, - pad = 0, -): string[] { - const indent = " ".repeat(Math.max(0, Math.min(pad, width - 4))); - const inner = Math.max(1, width - visibleWidth(indent) - 2); - const location = `:${String(details.line)}`; - const by = details.reviewer === undefined ? "" : ` · ${details.reviewer}`; - const path = truncatePath( - details.file, - Math.max(1, inner - visibleWidth(location + by)), - ); - const title = wrapTextWithAnsi(details.title, Math.max(1, inner - 2)); - const marker = theme.fg("dim", expanded ? " ▾" : " ▸"); - const lines = [ - ...title.map( - (part, index) => - `${index === 0 ? theme.fg("warning", "◆") : " "} ${theme.bold(part)}${index === title.length - 1 ? marker : ""}`, - ), - ` ${theme.fg("dim", path + location + by)}`, - ]; - if (expanded) - lines.push( - ...wrapTextWithAnsi(details.evidence, inner).map( - (part) => ` ${theme.fg("muted", part)}`, - ), - ...wrapTextWithAnsi(`“${details.reason}”`, inner).map( - (part) => ` ${theme.fg("dim", part)}`, - ), - ); - return lines.map((line) => sliceByColumn(indent + line, 0, width)); -} - -/** - * A card's own click toggle, remembered per message because Pi rebuilds the - * component on every expand or theme change. `base` records the global expand - * state when clicked, so a later ctrl+o overrides the local toggle. - */ -const toggled = new WeakMap(); - -/** Renders accepted findings as a compact card; unknown payloads fall back to Pi's default. */ -export const renderAccepted: MessageRenderer = (message, options, theme) => { - const parsed = AcceptedDetails.safeParse(message.details); - if (!parsed.success) return; - const open = (): boolean => { - const state = toggled.get(message); - return state?.base === options.expanded ? state.open : options.expanded; - }; - return { - render: (width: number): string[] => - acceptedLines(parsed.data, theme, width, open(), options.outputPad), - handleMouse(event) { - if (event.type !== "click" || event.button !== "left") return; - toggled.set(message, { open: !open(), base: options.expanded }); - return { handled: true, render: true }; - }, - invalidate(): void { - return; - }, - }; -}; diff --git a/src/finding-card.ts b/src/finding-card.ts new file mode 100644 index 0000000..cae9d2f --- /dev/null +++ b/src/finding-card.ts @@ -0,0 +1,130 @@ +import type { + EntryRenderer, + MessageRenderer, + Theme, +} from "@earendil-works/pi-coding-agent"; +import { + sliceByColumn, + visibleWidth, + wrapTextWithAnsi, + type Component, +} from "@earendil-works/pi-tui"; +import { z } from "zod"; +import { truncatePath } from "./review-sidebar.js"; + +export const ACCEPTED_MESSAGE = "pair-programmer-accepted"; +/** Custom entry type: rendered in the transcript, never sent to the model. */ +export const REJECTED_ENTRY = "pair-programmer-rejected"; + +const CardDetails = z.object({ + title: z.string(), + file: z.string(), + line: z.number(), + evidence: z.string(), + reason: z.string(), + reviewer: z.string().optional(), +}); +export type CardDetails = z.infer; +export type Verdict = "accepted" | "rejected"; + +type Painter = Pick; +/** The transcript object a card renders; stable across Pi's rebuilds. */ +type CardKey = Parameters[0] | Parameters[0]; + +/** + * Accepted: bold title plus location; rejected: one dimmed line. Expanding + * either reveals the location, evidence and the agent's reason. + */ +export function cardLines( + details: CardDetails, + verdict: Verdict, + theme: Painter, + width: number, + expanded: boolean, + pad = 0, +): string[] { + const indent = " ".repeat(Math.max(0, Math.min(pad, width - 4))); + const inner = Math.max(1, width - visibleWidth(indent) - 2); + const accepted = verdict === "accepted"; + const suffix = accepted ? "" : " · rejected"; + const marker = theme.fg("dim", `${suffix}${expanded ? " ▾" : " ▸"}`); + const title = wrapTextWithAnsi( + details.title, + Math.max(1, inner - visibleWidth(suffix) - 2), + ); + const icon = accepted ? theme.fg("warning", "◆") : theme.fg("muted", "✗"); + const lines = title.map((part, index) => { + const text = accepted ? theme.bold(part) : theme.fg("muted", part); + const first = index === 0 ? icon : " "; + return `${first} ${text}${index === title.length - 1 ? marker : ""}`; + }); + if (accepted || expanded) { + const location = `:${String(details.line)}`; + const by = details.reviewer === undefined ? "" : ` · ${details.reviewer}`; + const path = truncatePath( + details.file, + Math.max(1, inner - visibleWidth(location + by)), + ); + lines.push(` ${theme.fg("dim", path + location + by)}`); + } + if (expanded) + lines.push( + ...wrapTextWithAnsi(details.evidence, inner).map( + (part) => ` ${theme.fg("muted", part)}`, + ), + ...wrapTextWithAnsi(`“${details.reason}”`, inner).map( + (part) => ` ${theme.fg("dim", part)}`, + ), + ); + return lines.map((line) => sliceByColumn(indent + line, 0, width)); +} + +/** + * A card's own click toggle, remembered per message or entry because Pi + * rebuilds the component on every expand or theme change. `base` records the + * global expand state when clicked, so a later ctrl+o overrides the toggle. + */ +const toggled = new WeakMap(); + +function card( + key: CardKey, + payload: unknown, + verdict: Verdict, + theme: Painter, + expanded: boolean, + pad: number, +): Component | undefined { + const parsed = CardDetails.safeParse(payload); + if (!parsed.success) return; + const open = (): boolean => { + const state = toggled.get(key); + return state?.base === expanded ? state.open : expanded; + }; + return { + render: (width: number): string[] => + cardLines(parsed.data, verdict, theme, width, open(), pad), + handleMouse(event) { + if (event.type !== "click" || event.button !== "left") return; + toggled.set(key, { open: !open(), base: expanded }); + return { handled: true, render: true }; + }, + invalidate(): void { + return; + }, + }; +} + +/** Accepted findings; unknown payloads fall back to Pi's default rendering. */ +export const renderAccepted: MessageRenderer = (message, options, theme) => + card( + message, + message.details, + "accepted", + theme, + options.expanded, + options.outputPad, + ); + +/** Rejected findings, stored as transcript-only entries. */ +export const renderRejected: EntryRenderer = (entry, options, theme) => + card(entry, entry.data, "rejected", theme, options.expanded, 1); diff --git a/src/index.ts b/src/index.ts index 6f2c1a1..f49facb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,9 +14,11 @@ import { } from "./change-evidence.js"; import { ACCEPTED_MESSAGE, + REJECTED_ENTRY, renderAccepted, - type AcceptedDetails, -} from "./accepted-card.js"; + renderRejected, + type CardDetails, +} from "./finding-card.js"; import { FindingAdmission } from "./finding-admission.js"; import { createPairLogger, type PairLogger } from "./logger.js"; import type { ModelCallObserver } from "./model-usage.js"; @@ -942,10 +944,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { details: { saved: false }, }); } - const finding = - params.decision === "accept" - ? store.deliveredFinding(params.findingId) - : undefined; + const finding = store.deliveredFinding(params.findingId); const saved = store.decide( params.findingId, params.decision, @@ -956,23 +955,27 @@ export default function pairProgrammer(pi: ExtensionAPI): void { sessionId: accounting.stats.sessionId, outcome: saved ? params.decision : "invalid", }); - if (saved && params.decision === "accept" && finding !== undefined) { - pi.sendMessage( - { - customType: ACCEPTED_MESSAGE, - content: acceptedReview(finding, params.reason), - display: true, - details: { - title: finding.title, - file: finding.file, - line: finding.line, - evidence: finding.evidence, - reason: params.reason, - ...optionalReviewer(reviewerNames.get(finding.reviewer)), - } satisfies AcceptedDetails, - }, - { triggerTurn: false }, - ); + if (saved && finding !== undefined) { + const details = { + title: finding.title, + file: finding.file, + line: finding.line, + evidence: finding.evidence, + reason: params.reason, + ...optionalReviewer(reviewerNames.get(finding.reviewer)), + } satisfies CardDetails; + if (params.decision === "accept") + pi.sendMessage( + { + customType: ACCEPTED_MESSAGE, + content: acceptedReview(finding, params.reason), + display: true, + details, + }, + { triggerTurn: false }, + ); + // Rejections are transcript-only: rendered for the user, never sent to the model. + else pi.appendEntry(REJECTED_ENTRY, details); } return Promise.resolve({ content: [ @@ -1024,6 +1027,8 @@ export default function pairProgrammer(pi: ExtensionAPI): void { // OMP may not expose message renderers; its default rendering stays readable. if (typeof pi.registerMessageRenderer === "function") pi.registerMessageRenderer(ACCEPTED_MESSAGE, renderAccepted); + if (typeof pi.registerEntryRenderer === "function") + pi.registerEntryRenderer(REJECTED_ENTRY, renderRejected); pi.registerCommand("pair-feed", { description: "Toggle the live review sidebar", diff --git a/test/extension.test.ts b/test/extension.test.ts index 56be024..1622cd2 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -214,6 +214,7 @@ async function setup( notify: ReturnType; shortcuts: Map void>; renderers: Map; + rejectedCards: unknown[]; setWidget: Mock<(key: string, content: string[] | undefined) => void>; status: () => string | undefined; entries: JournalEntry[]; @@ -253,6 +254,7 @@ async function setup( const hooks = new Map(); const shortcuts = new Map void>(); const renderers = new Map(); + const rejectedCards: unknown[] = []; const commands = new Map< string, Parameters[1] @@ -292,6 +294,9 @@ async function setup( registerMessageRenderer(customType: string, renderer: unknown) { renderers.set(customType, renderer); }, + registerEntryRenderer(customType: string, renderer: unknown) { + renderers.set(customType, renderer); + }, registerShortcut( key: string, options: { handler: (ctx: ExtensionContext) => void }, @@ -308,6 +313,10 @@ async function setup( transcript.push(entry); return; } + if (customType === "pair-programmer-rejected") { + rejectedCards.push(data); + return; + } entryAttempts += 1; if (entryError !== undefined) throw entryError; if (customType !== "pair-programmer") @@ -360,6 +369,7 @@ async function setup( setWidget, shortcuts, renderers, + rejectedCards, status: () => setWidget.mock.lastCall?.[1]?.[0], entries, statsEntries, @@ -2862,6 +2872,27 @@ it("narrates review progress above the editor without surfacing finding contents decision: "reject", reason: "Intentional", }); + expect( + environment.rejectedCards.map((card) => + JSON.stringify(card, ["title", "reason", "reviewer"]), + ), + ).toEqual( + findings(environment.entries).map((finding) => + JSON.stringify({ + title: finding.title, + reason: "Intentional", + reviewer: "gpt-5", + }), + ), + ); + expect( + environment.sendMessage.mock.calls.some( + ([message]) => message.customType === "pair-programmer-accepted", + ), + ).toBe(false); + expect(environment.renderers.get("pair-programmer-rejected")).toBeTypeOf( + "function", + ); expect(environment.status()).toBe("◆ pair · watching"); await environment.emit("session_shutdown"); await environment.decide("late", { diff --git a/test/accepted-card.test.ts b/test/finding-card.test.ts similarity index 72% rename from test/accepted-card.test.ts rename to test/finding-card.test.ts index 46c4e10..cd126b1 100644 --- a/test/accepted-card.test.ts +++ b/test/finding-card.test.ts @@ -5,7 +5,11 @@ import { type TuiMouseEvent, } from "@earendil-works/pi-tui"; import { describe, expect, it } from "vitest"; -import { acceptedLines, renderAccepted } from "../src/accepted-card.js"; +import { + cardLines, + renderAccepted, + renderRejected, +} from "../src/finding-card.js"; const theme = { fg: (color: string, text: string) => `<${color}>${text}`, @@ -34,13 +38,47 @@ const render = ( theme as unknown as Args[2], )?.render(width); +describe("rejected finding card", () => { + const render = ( + expanded: boolean, + data: unknown = details, + ): string[] | undefined => + renderRejected( + { data } as Parameters[0], + { expanded }, + plain as unknown as Args[2], + )?.render(120); + + it("folds to one dimmed line and expands to location, evidence and reason", () => { + expect(render(false)).toEqual([ + " ✗ Imperative loop used to construct collection · rejected ▸", + ]); + const open = render(true) ?? []; + expect(open[0]).toMatch(/· rejected ▾$/u); + expect(open[1]).toContain(`${details.file}:91`); + expect(open.at(-1)).toBe(" “Reworking it for the unsafe-index warning.”"); + }); + + it("toggles by click and ignores malformed entries", () => { + const entry = { data: details } as Parameters[0]; + const card = renderRejected( + entry, + { expanded: false }, + plain as unknown as Args[2], + ); + card?.handleMouse?.({ type: "click", button: "left" } as TuiMouseEvent); + expect(card?.render(120).length).toBeGreaterThan(1); + expect(render(false, { title: "only" })).toBeUndefined(); + }); +}); + describe("accepted finding card", () => { it("collapses to the title and a file-name-first location", () => { expect(render({ details }, false)).toEqual([ " ◆ *Imperative loop used to construct collection ▸", ` ${details.file}:91`, ]); - expect(acceptedLines(details, plain, 70, false, 1)).toEqual([ + expect(cardLines(details, "accepted", plain, 70, false, 1)).toEqual([ " ◆ Imperative loop used to construct collection ▸", " …/chat_detail/widgets/message_bubble/linkified_message_text.dart:91", ]); @@ -56,17 +94,23 @@ describe("accepted finding card", () => { " “Reworking it for the unsafe-index warning.”", ); expect( - acceptedLines({ ...details, reviewer: "entropy" }, plain, 60, false)[1], + cardLines( + { ...details, reviewer: "entropy" }, + "accepted", + plain, + 60, + false, + )[1], ).toBe(" …/message_bubble/linkified_message_text.dart:91 · entropy"); }); it("wraps within narrow widths and keeps the file name", () => { for (const width of [3, 12, 24, 40]) { - const lines = acceptedLines(details, plain, width, true, 4); + const lines = cardLines(details, "accepted", plain, width, true, 4); for (const line of lines) expect(visibleWidth(line)).toBeLessThanOrEqual(width); } - const narrow = acceptedLines(details, plain, 24, false); + const narrow = cardLines(details, "accepted", plain, 24, false); expect(narrow.at(-1)).toContain("…"); expect(narrow.at(-1)).toContain(":91"); expect(narrow.slice(1, -1).every((line) => line.startsWith(" "))).toBe( From 3e3545fb0397ed0312f637f8e96a172bc81daab4 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Sun, 27 Sep 2026 01:51:11 +0200 Subject: [PATCH 13/21] =?UTF-8?q?feat:=20=F0=9F=8E=B8=20add=20a=20hotspots?= =?UTF-8?q?=20tab=20to=20/pair-stats?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Press h to rank files by accepted findings on the selected branch, with width-scaled bars and rejected counts; o, d and h switch tabs. README documents hotspots and rejected cards. --- README.md | 4 +-- src/index.ts | 8 ++++- src/review-store.ts | 20 ++++++++++++ src/stats-view.ts | 69 +++++++++++++++++++++++++++++++-------- test/extension.test.ts | 2 ++ test/review-store.test.ts | 30 +++++++++++++++++ test/stats-view.test.ts | 55 +++++++++++++++++++++++++++---- 7 files changed, 165 insertions(+), 23 deletions(-) diff --git a/README.md b/README.md index feec145..e632306 100644 --- a/README.md +++ b/README.md @@ -42,7 +42,7 @@ Without it, reviews still run, but only exact duplicate findings are filtered. ## Use -Reviews are on by default. After each successful `write` or `edit`, reviewers run in the background. The coding agent must accept or reject each finding with a reason before continuing with other tools. Only accepted findings appear in your transcript. +Reviews are on by default. After each successful `write` or `edit`, reviewers run in the background. The coding agent must accept or reject each finding with a reason before continuing with other tools. Accepted findings appear in your transcript as compact cards; rejected ones appear as folded one-line cards that are never sent to the model. Click a card (fullscreen mode) or press `ctrl+o` to expand it. Run `/pair-programmer` to toggle reviews. @@ -69,5 +69,5 @@ The optional `name` (1–24 characters) labels the reviewer in the sidebar and c A small badge pinned to the top-right corner shows whether reviews are watching, running, queued, awaiting a decision, or paused. It never takes keyboard focus, stays visible when extensions such as zentui hide the footer, and hides in terminals narrower than 40 columns. Outside Pi's terminal UI (RPC, OMP), it falls back to a line above the editor. Accepted findings appear in the transcript with their location, evidence, and rationale. - `/pair-feed` or `alt+r`: toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first. Each review is one row with its file, reviewer and verdict counts; click it (fullscreen mode) to show its duration, decided findings and the agent's reasons. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. -- `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. +- `/pair-stats`: open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `h` for hotspots (files ranked by accepted findings on the selected branch), `o` to return to the overview, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. - Diagnostics stay in files, never the terminal: OMP uses its native logs; Pi uses `~/.pi/agent/logs/pair-programmer/` (or `$PI_CODING_AGENT_DIR/logs/pair-programmer/`). diff --git a/src/index.ts b/src/index.ts index f49facb..50b3203 100644 --- a/src/index.ts +++ b/src/index.ts @@ -28,7 +28,12 @@ import { type PairStats, type ReviewOutcome, } from "./pair-stats.js"; -import { StatsView, statsLines, statsSummary } from "./stats-view.js"; +import { + StatsView, + statsHotspots, + statsLines, + statsSummary, +} from "./stats-view.js"; import { StatusOverlay, type StatusTone } from "./status-overlay.js"; import { ReviewFeed } from "./review-feed.js"; import { ReviewSidebar } from "./review-sidebar.js"; @@ -1054,6 +1059,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { return { summary: statsSummary(snapshot, store), details: statsLines(snapshot, store), + hotspots: (width: number) => statsHotspots(store, width), }; }); }, diff --git a/src/review-store.ts b/src/review-store.ts index d9f8274..f0fb59b 100644 --- a/src/review-store.ts +++ b/src/review-store.ts @@ -173,6 +173,26 @@ export class ReviewStore { return result; } + /** Decided findings per file, most accepted first (selected branch). */ + hotspots(): { file: string; accepted: number; rejected: number }[] { + const files = new Map(); + for (const { finding, verdict } of this.findings.values()) { + if (verdict === undefined) continue; + const counts = files.get(finding.file) ?? { accepted: 0, rejected: 0 }; + if (verdict === "accept") counts.accepted += 1; + else counts.rejected += 1; + files.set(finding.file, counts); + } + return [...files] + .map(([file, counts]) => ({ file, ...counts })) + .toSorted( + (left, right) => + right.accepted - left.accepted || + right.rejected - left.rejected || + left.file.localeCompare(right.file), + ); + } + lookup(id: string): FindingView | undefined { const state = this.findings.get(id); if (state === undefined) return undefined; diff --git a/src/stats-view.ts b/src/stats-view.ts index 26ee173..38ec272 100644 --- a/src/stats-view.ts +++ b/src/stats-view.ts @@ -6,6 +6,7 @@ import { } from "@earendil-works/pi-tui"; import type { MeasuredTotal, StatsSnapshot } from "./pair-stats.js"; import type { ReviewStore } from "./review-store.js"; +import { truncatePath } from "./review-sidebar.js"; function measured( total: MeasuredTotal, @@ -123,11 +124,49 @@ export function statsSummary( ]; } +/** Files ranked by accepted findings, with bars scaled to `width` columns. */ +export function statsHotspots( + store: Pick, + width: number, +): string[] { + const files = store.hotspots(); + if (files.length === 0) + return [ + "No decided findings on this branch yet.", + "Files with accepted findings will rank here.", + ]; + const most = Math.max(1, ...files.map(({ accepted }) => accepted)); + const count = String(most).length; + const bar = Math.max(4, Math.min(12, Math.floor(width / 6))); + const path = Math.max(8, width - bar - count - 16); + return [ + "ACCEPTED / by file, selected branch", + ...files.map(({ file, accepted, rejected }) => { + const name = truncatePath(file, path).padEnd(path); + const filled = "█".repeat(Math.ceil((accepted / most) * bar)); + const extra = rejected > 0 ? ` · ${String(rejected)} rejected` : ""; + return `${name} ${filled.padEnd(bar)} ${String(accepted).padStart(count)}${extra}`; + }), + ]; +} + export interface StatsReport { summary: readonly string[]; details: readonly string[]; + hotspots: (width: number) => readonly string[]; } +type Tab = "overview" | "details" | "hotspots"; +type TabSpec = readonly [key: string, tab: Tab, subtitle: string]; +const OVERVIEW: TabSpec = ["o", "overview", "Session overview"]; +/** Tabs in hint order. */ +const TABS: readonly TabSpec[] = [ + OVERVIEW, + ["d", "details", "Detailed accounting"], + ["h", "hotspots", "Hotspots · accepted findings by file"], +]; +const TAB_BY_KEY = new Map(TABS.map((spec) => [spec[0], spec])); + export class StatsView { private closeCurrent: (() => void) | undefined; @@ -153,7 +192,7 @@ export class StatsView { (tui, theme, keys, done) => { state.mounted = true; let report = read(); - let detailed = false; + let spec = OVERVIEW; let offset = 0; let pageSize = 1; let disposed = false; @@ -168,7 +207,10 @@ export class StatsView { const columns = Math.max(1, width); const framed = columns >= 12; const inner = Math.max(1, columns - (framed ? 6 : 0)); - const lines = detailed ? report.details : report.summary; + const tab = spec[1]; + let lines = report.summary; + if (tab === "details") lines = report.details; + else if (tab === "hotspots") lines = report.hotspots(inner); const wrapped = lines.flatMap((line) => { let tone: "warning" | "accent" | "text" = "text"; if (line.startsWith("!")) tone = "warning"; @@ -199,19 +241,17 @@ export class StatsView { }; const title = theme.bold(theme.fg("accent", "Pair Programmer")); const position = `${String(offset + 1)}–${String(Math.min(offset + pageSize, wrapped.length))}/${String(wrapped.length)}`; - const mode = detailed ? "overview" : "details"; + const others = TABS.filter(([, name]) => name !== tab); + const tabs = others + .map(([key, name]) => `${key} ${name}`) + .join(" "); const hint = - inner >= 52 - ? `↑↓ scroll d ${mode} r refresh esc close ${position}` - : `↑↓ d ${mode} r refresh q close`; + inner >= 64 + ? `↑↓ scroll ${tabs} r refresh esc ${position}` + : `↑↓ ${others.map(([key]) => key).join(" ")} r q close`; const body = [ row(title), - row( - theme.fg( - "dim", - detailed ? "Detailed accounting" : "Session overview", - ), - ), + row(theme.fg("dim", spec[2])), row(""), ...wrapped.slice(offset, offset + pageSize).map(row), row(""), @@ -234,8 +274,9 @@ export class StatsView { ) close?.(); else { - if (data === "d") { - detailed = !detailed; + const next = TAB_BY_KEY.get(data); + if (next !== undefined) { + spec = next === spec ? OVERVIEW : next; offset = 0; } else if (data === "r") { report = read(); diff --git a/test/extension.test.ts b/test/extension.test.ts index 1622cd2..731ff3d 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -450,6 +450,8 @@ async function openStatistics(environment: { lines = component.render(240); component.handleInput?.("d"); lines = [...lines, ...component.render(240)]; + component.handleInput?.("h"); + lines = [...lines, ...component.render(240)]; component.dispose?.(); }, ); diff --git a/test/review-store.test.ts b/test/review-store.test.ts index e659616..72440b4 100644 --- a/test/review-store.test.ts +++ b/test/review-store.test.ts @@ -24,6 +24,36 @@ function session(): { } describe("ReviewStore", () => { + it("ranks files by accepted then rejected findings, ignoring undecided ones", () => { + const { store } = session(); + const ids = ["a1", "a2", "a3", "b1", "b2", "c1", "c2", "d1"]; + const files: Record = { + a: "src/a.ts", + b: "src/b.ts", + c: "src/c.ts", + d: "src/d.ts", + }; + for (const id of ids) + store.add(finding(id, new Map(Object.entries(files)).get(id.charAt(0)))); + store.deliver(ids); + for (const id of ["a1", "a2", "b1"]) store.decide(id, "accept", "Yes"); + for (const id of ["a3", "b2", "c1", "c2"]) store.decide(id, "reject", "No"); + expect(store.hotspots()).toEqual([ + { file: "src/a.ts", accepted: 2, rejected: 1 }, + { file: "src/b.ts", accepted: 1, rejected: 1 }, + { file: "src/c.ts", accepted: 0, rejected: 2 }, + ]); + const tie = session().store; + for (const id of ["z", "y"]) tie.add(finding(id, `src/${id}.ts`)); + tie.deliver(["z", "y"]); + tie.decide("z", "accept", "Yes"); + tie.decide("y", "accept", "Yes"); + expect(tie.hotspots().map(({ file }) => file)).toEqual([ + "src/y.ts", + "src/z.ts", + ]); + }); + it("describes each finding's live status for the review feed", () => { const { store } = session(); for (const id of ["queued", "awaiting", "accepted", "rejected", "stale"]) diff --git a/test/stats-view.test.ts b/test/stats-view.test.ts index d699e60..b62d5fd 100644 --- a/test/stats-view.test.ts +++ b/test/stats-view.test.ts @@ -6,6 +6,7 @@ import { ReviewStore } from "../src/review-store.js"; import { StatsView, statsLines, + statsHotspots, statsSummary, type StatsReport, } from "../src/stats-view.js"; @@ -15,6 +16,7 @@ const report = () => ({ summary: lines, details: [`detail ${lines.join(" ")}`], + hotspots: (width: number) => [`hot ${String(width)}`], }); type Factory = Parameters[0]; @@ -198,6 +200,35 @@ describe("statistics presentation", () => { }); }); +describe("hotspots", () => { + it("ranks files by accepted findings with scaled bars and rejected counts", () => { + const store = { + hotspots: () => [ + { + file: "apps/lib/pages/chat/controller.dart", + accepted: 6, + rejected: 2, + }, + { file: "src/utils.ts", accepted: 3, rejected: 0 }, + { file: "src/legacy.ts", accepted: 0, rejected: 1 }, + ], + }; + const lines = statsHotspots(store, 72); + expect(lines[0]).toBe("ACCEPTED / by file, selected branch"); + expect(lines[1]).toMatch( + /^apps\/lib\/pages\/chat\/controller\.dart +█{12} {2}6 {2}· 2 rejected$/u, + ); + expect(lines[2]).toMatch(/^src\/utils\.ts +█{6} +3$/u); + expect(lines[3]).toMatch(/^src\/legacy\.ts {2,}0 {2}· 1 rejected$/u); + const narrow = statsHotspots(store, 30); + expect(narrow[1]).toMatch(/^…/u); + expect(narrow[1]).toContain("████ "); + expect(statsHotspots({ hotspots: () => [] }, 72)[0]).toContain( + "No decided findings", + ); + }); +}); + describe("StatsView", () => { it.each([ "q", @@ -228,12 +259,12 @@ describe("StatsView", () => { "USAGE /", "now", "", - "↑↓ d deta\u{1B}[0m…\u{1B}[0m", + "↑↓ d h r\u{1B}[0m…\u{1B}[0m", ]); const framed = component.render(24); expect(framed[0]).toBe(`╭${"─".repeat(22)}╮`); expect(framed[4]).toBe(`│ Review counts${" ".repeat(5)} │`); - expect(framed.at(-2)).toContain("↑↓ d details r "); + expect(framed.at(-2)).toContain("↑↓ d h r q clo"); expect(framed.at(-1)).toBe(`╰${"─".repeat(22)}╯`); component.invalidate(); component.handleInput?.(key); @@ -255,7 +286,9 @@ describe("StatsView", () => { const component = host.component(); const first = (): string | undefined => component.render(80)[4]; expect(first()).toContain("Row 0"); - expect(component.render(80).at(-2)).toContain("esc close 1–9/12"); + expect(component.render(80).at(-2)).toContain( + "d details h hotspots r refresh esc 1–9/12", + ); component.handleInput?.("\u{1B}[B"); expect(first()).toContain("Row 1"); component.handleInput?.("\u{1B}[A"); @@ -267,14 +300,20 @@ describe("StatsView", () => { component.handleInput?.("d"); expect(component.render(80)[2]).toContain("Detailed accounting"); expect(first()).toContain("detail Row 0"); - expect(component.render(80).at(-2)).toContain("d overview"); + expect(component.render(80).at(-2)).toContain("o overview h hotspots"); component.handleInput?.("\u{1B}[B"); component.handleInput?.("r"); expect(first()).toContain("detail Row 0"); + component.handleInput?.("h"); + expect(component.render(80)[2]).toContain("Hotspots"); + expect(first()).toContain("hot 74"); + component.handleInput?.("o"); + expect(first()).toContain("Row 0"); + component.handleInput?.("d"); component.handleInput?.("d"); expect(first()).toContain("Row 0"); component.handleInput?.("ignored"); - expect(host.renderRequests()).toBe(9); + expect(host.renderRequests()).toBe(12); view.close(); await opened; }); @@ -283,7 +322,11 @@ describe("StatsView", () => { let color = "31"; let value = "変更 e\u{301} 👩‍💻 " + "long-model-name".repeat(10); const host = ui(40, (_tone, text) => `\u{1B}[${color}m${text}\u{1B}[0m`); - const read = vi.fn(() => ({ summary: [value], details: [value] })); + const read = vi.fn(() => ({ + summary: [value], + details: [value], + hotspots: () => [value], + })); const view = new StatsView(); const opened = view.open(host.ctx, read); await host.mounted; From 11b4ff625085ec6e34b6eae0f27c35117034edbb Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:03:22 +0200 Subject: [PATCH 14/21] refactor: share one finding status classifier in ReviewStore --- src/review-store.ts | 32 +++++++++++++++++++------------- 1 file changed, 19 insertions(+), 13 deletions(-) diff --git a/src/review-store.ts b/src/review-store.ts index f0fb59b..4e51dbd 100644 --- a/src/review-store.ts +++ b/src/review-store.ts @@ -59,6 +59,22 @@ interface FindingState extends StoredFinding { discarded: boolean; } +/** The single source of truth for a finding's lifecycle status. */ +function statusOf(state: FindingState): FindingStatus { + if (state.verdict === "accept") return "accepted"; + if (state.verdict === "reject") return "rejected"; + if (state.discarded) return "discarded"; + return state.delivered ? "awaiting" : "queued"; +} + +const SUMMARY_KEY = { + queued: "pending", + awaiting: "outstanding", + accepted: "accepted", + rejected: "rejected", + discarded: "discarded", +} as const satisfies Record; + export class ReviewStore { private readonly findings = new Map(); private readonly append: (data: unknown) => void; @@ -95,13 +111,8 @@ export class ReviewStore { rejected: 0, discarded: 0, }; - for (const state of this.findings.values()) { - if (state.verdict === "accept") counts.accepted += 1; - else if (state.verdict === "reject") counts.rejected += 1; - else if (state.discarded) counts.discarded += 1; - else if (state.delivered) counts.outstanding += 1; - else counts.pending += 1; - } + for (const state of this.findings.values()) + counts[SUMMARY_KEY[statusOf(state)]] += 1; return counts; } @@ -196,15 +207,10 @@ export class ReviewStore { lookup(id: string): FindingView | undefined { const state = this.findings.get(id); if (state === undefined) return undefined; - let status: FindingStatus = "queued"; - if (state.verdict === "accept") status = "accepted"; - else if (state.verdict === "reject") status = "rejected"; - else if (state.discarded) status = "discarded"; - else if (state.delivered) status = "awaiting"; return { title: state.finding.title, line: state.finding.line, - status, + status: statusOf(state), ...(state.reason === undefined ? {} : { reason: state.reason }), }; } From f04beddc9e361b9ff6cfa69b722b0b498d8bf973 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:03:25 +0200 Subject: [PATCH 15/21] refactor: return added finding IDs from admission --- src/finding-admission.ts | 22 +++++++----- src/index.ts | 20 +++-------- test/finding-admission.test.ts | 64 ++++++++++++++++++++-------------- 3 files changed, 54 insertions(+), 52 deletions(-) diff --git a/src/finding-admission.ts b/src/finding-admission.ts index e7905bd..f900d91 100644 --- a/src/finding-admission.ts +++ b/src/finding-admission.ts @@ -16,6 +16,11 @@ interface AdmissionRequest { type Candidate = Finding & { duplicateKey: string }; +export interface AdmissionResult { + outcome: "obsolete" | "unchanged" | "added"; + ids: readonly string[]; +} + let jevClient: TypeSafeClient | undefined; function hash(parts: readonly string[]): string { @@ -132,9 +137,8 @@ export class FindingAdmission { this.store = store; } - async admit( - request: AdmissionRequest, - ): Promise<"obsolete" | "unchanged" | "added"> { + /** Admits novel findings; `ids` lists exactly the findings this call added. */ + async admit(request: AdmissionRequest): Promise { const { file, signal } = request; let history = this.store.history(file); let novel = await novelFindings( @@ -145,7 +149,8 @@ export class FindingAdmission { request.onModelCall, ); for (;;) { - if (!(await request.isCurrent()) || signal.aborted) return "obsolete"; + if (!(await request.isCurrent()) || signal.aborted) + return { outcome: "obsolete", ids: [] }; const latest = this.store.history(file); const added = latest.slice(history.length); if (novel.length === 0 || added.length === 0) break; @@ -158,10 +163,9 @@ export class FindingAdmission { request.onModelCall, ); } - let result: "unchanged" | "added" = "unchanged"; - for (const finding of novel) { - if (this.store.add(finding)) result = "added"; - } - return result; + const ids = novel + .filter((finding) => this.store.add(finding)) + .map((finding) => finding.id); + return { outcome: ids.length > 0 ? "added" : "unchanged", ids }; } } diff --git a/src/index.ts b/src/index.ts index 50b3203..9b0b17d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -531,9 +531,6 @@ export default function pairProgrammer(pi: ExtensionAPI): void { count: judged.filter(({ inherited }) => inherited).length, }); if (signal.aborted || !(await current(job))) return "obsolete"; - const before = new Set( - store.history(job.file).map((stored) => stored.finding.id), - ); const result = await admission.admit({ file: job.file, reviewer, @@ -548,20 +545,11 @@ export default function pairProgrammer(pi: ExtensionAPI): void { logger?.log("finding.admission", { sessionId: job.stats.sessionId, jobId: job.id, - outcome: result, + outcome: result.outcome, }); - if (result === "obsolete") return "obsolete"; - if (result === "added") { - feed.attach( - job.id, - store - .history(job.file) - .filter( - ({ finding }) => - finding.reviewer === reviewer && !before.has(finding.id), - ) - .map(({ finding }) => finding.id), - ); + if (result.outcome === "obsolete") return "obsolete"; + if (result.outcome === "added") { + feed.attach(job.id, result.ids); scheduleWake(job.ctx); } reviewed.set(`${job.key}:${reviewer}:${job.model}`, job.revision); diff --git a/test/finding-admission.test.ts b/test/finding-admission.test.ts index ac2e643..0e5a596 100644 --- a/test/finding-admission.test.ts +++ b/test/finding-admission.test.ts @@ -98,7 +98,7 @@ it.each(["accept", "reject"] as const)( ); expect( await admission.admit({ ...request([changed]), revision: "revision-2" }), - ).toBe("added"); + ).toHaveProperty("outcome", "added"); const second = readyFinding(store); expect(second.id).not.toBe(first.id); store.deliver([second.id]); @@ -121,7 +121,7 @@ it.each(["accept", "reject"] as const)( ...request([changed]), revision: "revision-3", }), - ).toBe("unchanged"); + ).toHaveProperty("outcome", "unchanged"); expect(restored.store.ready()).toEqual([]); }, ); @@ -138,13 +138,13 @@ it("suppresses repeated evidence across revisions, lines, and a Jev outage", asy ...request([{ ...proposal, line: 20, title: " NULL DEREFERENCE " }]), revision: "revision-2", }), - ).toBe("unchanged"); + ).toHaveProperty("outcome", "unchanged"); expect( await admission.admit({ ...request([{ ...proposal, evidence: "Reworded nullable access" }]), revision: "revision-3", }), - ).toBe("unchanged"); + ).toHaveProperty("outcome", "unchanged"); expect(systemOne).toHaveBeenCalledOnce(); expect(store.ready()).toEqual([]); expect(store.history("src/example.ts")).toEqual([ @@ -214,9 +214,15 @@ it("retains legacy decisions and duplicate identity without rewriting historical const { admission, store } = session(entries); systemOne.mockRejectedValueOnce(new Error("Jev unavailable")); const changed = { ...proposal, evidence: "New caller triggers this access" }; - expect(await admission.admit(request([changed]))).toBe("unchanged"); + expect(await admission.admit(request([changed]))).toHaveProperty( + "outcome", + "unchanged", + ); expect(entries.map(({ data }) => data)).toEqual(events); - expect(await admission.admit(request([changed]))).toBe("added"); + expect(await admission.admit(request([changed]))).toHaveProperty( + "outcome", + "added", + ); const second = readyFinding(store); expect(second.id).not.toBe(legacy.id); expect(store.history(legacy.file)[0]).toEqual({ @@ -240,14 +246,14 @@ it("rechecks concurrent equivalent findings and never serializes unrelated files reviewer: "risks", isCurrent: () => secondCheck.promise, }); - expect(await admission.admit({ ...request(), file: "src/other.ts" })).toBe( - "added", - ); + expect( + await admission.admit({ ...request(), file: "src/other.ts" }), + ).toHaveProperty("outcome", "added"); firstCheck.resolve(true); - expect(await pendingFirst).toBe("added"); + expect(await pendingFirst).toHaveProperty("outcome", "added"); systemOne.mockResolvedValue(duplicate); secondCheck.resolve(true); - expect(await pendingSecond).toBe("unchanged"); + expect(await pendingSecond).toHaveProperty("outcome", "unchanged"); expect(store.ready().map(({ file, title }) => ({ file, title }))).toEqual([ { file: "src/other.ts", title: proposal.title }, { file: "src/example.ts", title: proposal.title }, @@ -264,7 +270,7 @@ it("keeps multiple distinct candidates after another review commits during judgm }); await admission.admit(request([{ ...proposal, title: "Missing timeout" }])); check.resolve(true); - expect(await pending).toBe("added"); + expect(await pending).toHaveProperty("outcome", "added"); expect(store.ready().map(({ title }) => title)).toEqual([ "Missing timeout", proposal.title, @@ -291,7 +297,8 @@ it.each(["obsolete", "abort", "disabled"] as const)( else store.setEnabled(false); const previous = [...entries]; judgment.resolve(novel); - expect(await pending).toBe( + expect(await pending).toHaveProperty( + "outcome", change === "disabled" ? "unchanged" : "obsolete", ); expect(entries).toEqual(previous); @@ -322,16 +329,19 @@ it("does not persist candidates after a judgment aborts and ignores already-abor signal: controller.signal, }); controller.abort(); - expect(await pending).toBe("obsolete"); + expect(await pending).toHaveProperty("outcome", "obsolete"); expect( await admission.admit({ ...request(), signal: controller.signal }), - ).toBe("obsolete"); + ).toHaveProperty("outcome", "obsolete"); expect(store.ready()).toEqual([first]); }); it("leaves the queue unchanged when no findings survive review", async () => { const { admission, store, entries } = session(); - expect(await admission.admit(request([]))).toBe("unchanged"); + expect(await admission.admit(request([]))).toHaveProperty( + "outcome", + "unchanged", + ); expect(store.ready()).toEqual([]); expect(entries).toEqual([]); }); @@ -339,12 +349,12 @@ it("leaves the queue unchanged when no findings survive review", async () => { it("accounts only real dedup calls and keeps deterministic duplicate fast paths silent", async () => { const { admission, store } = session(); const onModelCall = vi.fn(); - await expect(admission.admit({ ...request(), onModelCall })).resolves.toBe( - "added", - ); - await expect(admission.admit({ ...request(), onModelCall })).resolves.toBe( - "unchanged", - ); + await expect( + admission.admit({ ...request(), onModelCall }), + ).resolves.toHaveProperty("outcome", "added"); + await expect( + admission.admit({ ...request(), onModelCall }), + ).resolves.toHaveProperty("outcome", "unchanged"); expect(onModelCall).not.toHaveBeenCalled(); systemOne.mockResolvedValueOnce({ ...novel, @@ -356,7 +366,7 @@ it("accounts only real dedup calls and keeps deterministic duplicate fast paths ...request([{ ...proposal, title: "Other defect" }]), onModelCall, }), - ).resolves.toBe("added"); + ).resolves.toHaveProperty("outcome", "added"); expect(store.ready().map(({ title }) => title)).toEqual([ proposal.title, "Other defect", @@ -381,13 +391,13 @@ it("preserves deterministic fail-open decisions while recording unavailable judg ...request([{ ...proposal, evidence: "Changed wording" }]), onModelCall, }), - ).resolves.toBe("unchanged"); + ).resolves.toHaveProperty("outcome", "unchanged"); await expect( admission.admit({ ...request([{ ...proposal, title: "Distinct defect" }]), onModelCall, }), - ).resolves.toBe("added"); + ).resolves.toHaveProperty("outcome", "added"); expect(store.ready().map(({ title }) => title)).toEqual([ proposal.title, "Distinct defect", @@ -416,7 +426,7 @@ it("does not let an accounting exception alter a semantic duplicate decision", a throw new Error("journal offline"); }, }), - ).resolves.toBe("unchanged"); + ).resolves.toHaveProperty("outcome", "unchanged"); expect(store.ready()).toEqual([original]); }); @@ -443,7 +453,7 @@ it("records one cancelled dedup call without admitting its findings", async () = onModelCall, }); controller.abort(); - await expect(pending).resolves.toBe("obsolete"); + await expect(pending).resolves.toHaveProperty("outcome", "obsolete"); expect(store.ready()).toEqual([original]); expect(onModelCall).toHaveBeenCalledExactlyOnceWith({ stage: "dedup", From 226cf913ee26e19d7f92e15a5bc942676f72c597 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:03:40 +0200 Subject: [PATCH 16/21] fix: report malformed reviewer JSON with a clear error --- src/review-runner.ts | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/review-runner.ts b/src/review-runner.ts index 971f1bb..9a21427 100644 --- a/src/review-runner.ts +++ b/src/review-runner.ts @@ -181,7 +181,12 @@ async function complete( await invoke(args, cwd, prompt, signal, model, onModelCall) ).trim(); const fenced = /^```(?:json)?[ \t]*\r?\n([\s\S]*?)\r?\n```$/iu.exec(stdout); - return JSON.parse(fenced?.[1] ?? stdout) as unknown; + try { + return JSON.parse(fenced?.[1] ?? stdout) as unknown; + } catch (error) { + // The job records a failed review; name the cause instead of a bare SyntaxError. + throw new Error("Reviewer returned malformed JSON", { cause: error }); + } } export async function reviewFile( From 9e255f6ddb06aafe570aa36be34db9def361b278 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:04:31 +0200 Subject: [PATCH 17/21] refactor: share path truncation and frame drawing helpers --- src/finding-card.ts | 2 +- src/format.ts | 51 +++++++++++++++++++++++++++++++++++++ src/review-sidebar.ts | 34 +++---------------------- src/stats-view.ts | 45 ++++++++++---------------------- test/format.test.ts | 32 +++++++++++++++++++++++ test/review-sidebar.test.ts | 11 -------- 6 files changed, 101 insertions(+), 74 deletions(-) create mode 100644 src/format.ts create mode 100644 test/format.test.ts diff --git a/src/finding-card.ts b/src/finding-card.ts index cae9d2f..93d6152 100644 --- a/src/finding-card.ts +++ b/src/finding-card.ts @@ -10,7 +10,7 @@ import { type Component, } from "@earendil-works/pi-tui"; import { z } from "zod"; -import { truncatePath } from "./review-sidebar.js"; +import { truncatePath } from "./format.js"; export const ACCEPTED_MESSAGE = "pair-programmer-accepted"; /** Custom entry type: rendered in the transcript, never sent to the model. */ diff --git a/src/format.ts b/src/format.ts new file mode 100644 index 0000000..ae913ca --- /dev/null +++ b/src/format.ts @@ -0,0 +1,51 @@ +import type { Theme } from "@earendil-works/pi-coding-agent"; +import { truncateToWidth, visibleWidth } from "@earendil-works/pi-tui"; + +/** Keeps the end of a path: drop whole leading directories first, then characters. */ +export function truncatePath(file: string, width: number): string { + if (visibleWidth(file) <= width) return file; + const parts = file.split("/"); + for (let index = 1; index < parts.length; index += 1) { + const tail = `…/${parts.slice(index).join("/")}`; + if (visibleWidth(tail) <= width) return tail; + } + const graphemes = Array.from( + new Intl.Segmenter(undefined, { granularity: "grapheme" }).segment(file), + ({ segment }) => segment, + ); + const start = graphemes.findIndex( + (_, index) => visibleWidth(`…${graphemes.slice(index).join("")}`) <= width, + ); + return start === -1 ? "…" : `…${graphemes.slice(start).join("")}`; +} + +export interface Frame { + /** Columns available to content inside the border and padding. */ + inner: number; + /** Truncates and pads each line, then wraps the rows in a rounded border. */ + render(lines: readonly string[]): string[]; +} + +/** A rounded border `width` columns wide with `padding` spaces inside each side. */ +export function frame( + theme: Pick, + width: number, + padding: number, +): Frame { + const inner = Math.max(1, width - 2 - 2 * padding); + const space = " ".repeat(padding); + const side = theme.fg("borderMuted", "│"); + const rule = "─".repeat(Math.max(0, width - 2)); + return { + inner, + render: (lines) => [ + theme.fg("borderMuted", `╭${rule}╮`), + ...lines.map((text) => { + const clipped = truncateToWidth(text, inner, "…"); + const pad = " ".repeat(Math.max(0, inner - visibleWidth(clipped))); + return `${side}${space}${clipped}${pad}${space}${side}`; + }), + theme.fg("borderMuted", `╰${rule}╯`), + ], + }; +} diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index 8854335..d4ead76 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -1,10 +1,10 @@ import type { ExtensionContext, Theme } from "@earendil-works/pi-coding-agent"; import { sliceByColumn, - truncateToWidth, visibleWidth, wrapTextWithAnsi, } from "@earendil-works/pi-tui"; +import { frame, truncatePath } from "./format.js"; import type { FeedEntry } from "./review-feed.js"; import type { FindingView } from "./review-store.js"; @@ -17,24 +17,6 @@ export const MIN_COLUMNS = 90; // Rows kept clear under the card for the editor and footer. const EDITOR_RESERVE = 8; -/** Keeps the end of a path: drop whole leading directories first, then characters. */ -export function truncatePath(file: string, width: number): string { - if (visibleWidth(file) <= width) return file; - const parts = file.split("/"); - for (let index = 1; index < parts.length; index += 1) { - const tail = `…/${parts.slice(index).join("/")}`; - if (visibleWidth(tail) <= width) return tail; - } - const graphemes = Array.from( - new Intl.Segmenter(undefined, { granularity: "grapheme" }).segment(file), - ({ segment }) => segment, - ); - const start = graphemes.findIndex( - (_, index) => visibleWidth(`…${graphemes.slice(index).join("")}`) <= width, - ); - return start === -1 ? "…" : `…${graphemes.slice(start).join("")}`; -} - /** Cuts text to `width` columns with a plain ellipsis (no stray reset codes). */ function clip(text: string, width: number): string { if (visibleWidth(text) <= width) return text; @@ -162,18 +144,14 @@ export function layoutSidebar( now = Date.now(), expanded: ReadonlySet = new Set(), ): SidebarLayout { - const inner = Math.max(1, width - 4); const visible = entries.flatMap((entry) => { const findings = decided(entry, lookup); return entry.phase === "running" || findings.length > 0 ? [{ entry, findings }] : []; }); - const row = (text: string): string => { - const clipped = truncateToWidth(text, inner, "…"); - const pad = " ".repeat(Math.max(0, inner - visibleWidth(clipped))); - return `${theme.fg("borderMuted", "│")} ${clipped}${pad} ${theme.fg("borderMuted", "│")}`; - }; + const box = frame(theme, width, 1); + const inner = box.inner; const running = entries.filter((entry) => entry.phase === "running").length; const header = [ theme.bold(theme.fg("accent", "Pair Programmer")), @@ -225,11 +203,7 @@ export function layoutSidebar( } const content = [...header, ...body].slice(0, room + header.length); return { - lines: [ - theme.fg("borderMuted", `╭${"─".repeat(Math.max(0, width - 2))}╮`), - ...[...content, ...footer].map(row), - theme.fg("borderMuted", `╰${"─".repeat(Math.max(0, width - 2))}╯`), - ], + lines: box.render([...content, ...footer]), rows: [ ...Array.from({ length: 1 + header.length }), ...owners.slice(0, content.length - header.length), diff --git a/src/stats-view.ts b/src/stats-view.ts index 38ec272..7f34c6f 100644 --- a/src/stats-view.ts +++ b/src/stats-view.ts @@ -1,12 +1,8 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; -import { - truncateToWidth, - visibleWidth, - wrapTextWithAnsi, -} from "@earendil-works/pi-tui"; +import { truncateToWidth, wrapTextWithAnsi } from "@earendil-works/pi-tui"; import type { MeasuredTotal, StatsSnapshot } from "./pair-stats.js"; import type { ReviewStore } from "./review-store.js"; -import { truncatePath } from "./review-sidebar.js"; +import { frame, truncatePath } from "./format.js"; function measured( total: MeasuredTotal, @@ -205,8 +201,8 @@ export class StatsView { return { render(width: number): string[] { const columns = Math.max(1, width); - const framed = columns >= 12; - const inner = Math.max(1, columns - (framed ? 6 : 0)); + const box = columns >= 12 ? frame(theme, columns, 2) : undefined; + const inner = box?.inner ?? columns; const tab = spec[1]; let lines = report.summary; if (tab === "details") lines = report.details; @@ -228,17 +224,6 @@ export class StatsView { ); offset = Math.min(offset, Math.max(0, wrapped.length - pageSize)); if (compact) return wrapped.slice(offset, offset + pageSize); - const row = (text: string): string => { - const clipped = truncateToWidth(text, inner, "…"); - return framed - ? theme.fg("borderMuted", "│") + - " " + - clipped + - " ".repeat(Math.max(0, inner - visibleWidth(clipped))) + - " " + - theme.fg("borderMuted", "│") - : clipped; - }; const title = theme.bold(theme.fg("accent", "Pair Programmer")); const position = `${String(offset + 1)}–${String(Math.min(offset + pageSize, wrapped.length))}/${String(wrapped.length)}`; const others = TABS.filter(([, name]) => name !== tab); @@ -250,20 +235,16 @@ export class StatsView { ? `↑↓ scroll ${tabs} r refresh esc ${position}` : `↑↓ ${others.map(([key]) => key).join(" ")} r q close`; const body = [ - row(title), - row(theme.fg("dim", spec[2])), - row(""), - ...wrapped.slice(offset, offset + pageSize).map(row), - row(""), - row(theme.fg("dim", hint)), + title, + theme.fg("dim", spec[2]), + "", + ...wrapped.slice(offset, offset + pageSize), + "", + theme.fg("dim", hint), ]; - return framed - ? [ - theme.fg("borderMuted", `╭${"─".repeat(columns - 2)}╮`), - ...body, - theme.fg("borderMuted", `╰${"─".repeat(columns - 2)}╯`), - ] - : body; + return box === undefined + ? body.map((line) => truncateToWidth(line, inner, "…")) + : box.render(body); }, handleInput(data: string): void { if ( diff --git a/test/format.test.ts b/test/format.test.ts new file mode 100644 index 0000000..8615abc --- /dev/null +++ b/test/format.test.ts @@ -0,0 +1,32 @@ +import { visibleWidth } from "@earendil-works/pi-tui"; +import { describe, expect, it } from "vitest"; +import { frame, truncatePath } from "../src/format.js"; + +describe("truncatePath", () => { + it("keeps file names and drops leading directories first", () => { + const file = "apps/hush/lib/pages/chat_detail/widgets/bubble.dart"; + expect(truncatePath(file, 80)).toBe(file); + expect(truncatePath(file, 28)).toBe("…/widgets/bubble.dart"); + expect(truncatePath(file, 16)).toBe("…/bubble.dart"); + expect(truncatePath(file, 8)).toBe("…le.dart"); + expect(truncatePath("變更變更變更.ts", 6)).toBe("…更.ts"); + expect(truncatePath("x.ts", 0)).toBe("…"); + }); +}); + +describe("frame", () => { + const theme = { fg: (_color: string, text: string) => text }; + + it("pads and truncates rows inside a border of the exact width", () => { + const box = frame(theme, 12, 2); + expect(box.inner).toBe(6); + expect(box.render(["ab", "abcdefghij"])).toEqual([ + "╭──────────╮", + "│ ab │", + "│ abcde\u{1B}[0m…\u{1B}[0m │", + "╰──────────╯", + ]); + for (const line of frame(theme, 3, 1).render(["x"])) + expect(visibleWidth(line)).toBeLessThanOrEqual(5); + }); +}); diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index 47c9083..f2a938e 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -6,7 +6,6 @@ import { MIN_COLUMNS, renderSidebar, ReviewSidebar, - truncatePath, } from "../src/review-sidebar.js"; import type { FindingView } from "../src/review-store.js"; @@ -129,16 +128,6 @@ describe("renderSidebar", () => { for (const line of text(feed, 38, 5)) expect(visibleWidth(line)).toBe(38); }); - it("keeps file names and drops leading directories first", () => { - const file = "apps/hush/lib/pages/chat_detail/widgets/bubble.dart"; - expect(truncatePath(file, 80)).toBe(file); - expect(truncatePath(file, 28)).toBe("…/widgets/bubble.dart"); - expect(truncatePath(file, 16)).toBe("…/bubble.dart"); - expect(truncatePath(file, 8)).toBe("…le.dart"); - expect(truncatePath("變更變更變更.ts", 6)).toBe("…更.ts"); - expect(truncatePath("x.ts", 0)).toBe("…"); - }); - it("collapses each review to one row of file, reviewer and verdict counts", () => { const feed = new ReviewFeed(); feed.start("done", { ...job, file: "src/deep/done.ts" }, 0); From f1c036fb5fa1c8e3482d63aad6383e8ac7523c0a Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:05:24 +0200 Subject: [PATCH 18/21] fix: keep rejected findings out of the transcript --- README.md | 2 +- src/finding-card.ts | 111 +++++++++++++++----------------------- src/index.ts | 7 +-- test/extension.test.ts | 19 ++----- test/finding-card.test.ts | 66 +++++------------------ 5 files changed, 62 insertions(+), 143 deletions(-) diff --git a/README.md b/README.md index 0649ee8..3c73ff0 100644 --- a/README.md +++ b/README.md @@ -82,7 +82,7 @@ export TYPESAFE_API_KEY="your-api-key" ## 🧭 Use -Reviews are on by default. After each successful `write` or `edit`, reviewers run in the background. The coding agent must accept or reject each finding with a reason before continuing with other tools. Accepted findings appear in your transcript as compact cards; rejected ones appear as folded one-line cards that are never sent to the model. Click a card (fullscreen mode) or press `ctrl+o` to expand it. +Reviews are on by default. After each successful `write` or `edit`, reviewers run in the background. The coding agent must accept or reject each finding with a reason before continuing with other tools. Accepted findings appear in your transcript as compact cards; rejected ones stay out of the transcript and are listed in the review feed (`/pair-feed`). Click a card (fullscreen mode) or press `ctrl+o` to expand it. | Command | Action | | ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | diff --git a/src/finding-card.ts b/src/finding-card.ts index 93d6152..34ab579 100644 --- a/src/finding-card.ts +++ b/src/finding-card.ts @@ -1,8 +1,4 @@ -import type { - EntryRenderer, - MessageRenderer, - Theme, -} from "@earendil-works/pi-coding-agent"; +import type { MessageRenderer, Theme } from "@earendil-works/pi-coding-agent"; import { sliceByColumn, visibleWidth, @@ -13,8 +9,6 @@ import { z } from "zod"; import { truncatePath } from "./format.js"; export const ACCEPTED_MESSAGE = "pair-programmer-accepted"; -/** Custom entry type: rendered in the transcript, never sent to the model. */ -export const REJECTED_ENTRY = "pair-programmer-rejected"; const CardDetails = z.object({ title: z.string(), @@ -25,19 +19,17 @@ const CardDetails = z.object({ reviewer: z.string().optional(), }); export type CardDetails = z.infer; -export type Verdict = "accepted" | "rejected"; type Painter = Pick; -/** The transcript object a card renders; stable across Pi's rebuilds. */ -type CardKey = Parameters[0] | Parameters[0]; +/** The transcript message a card renders; stable across Pi's rebuilds. */ +type CardKey = Parameters[0]; /** - * Accepted: bold title plus location; rejected: one dimmed line. Expanding - * either reveals the location, evidence and the agent's reason. + * An accepted finding: bold title plus location. Expanding reveals the + * evidence and the agent's reason. */ export function cardLines( details: CardDetails, - verdict: Verdict, theme: Painter, width: number, expanded: boolean, @@ -45,28 +37,19 @@ export function cardLines( ): string[] { const indent = " ".repeat(Math.max(0, Math.min(pad, width - 4))); const inner = Math.max(1, width - visibleWidth(indent) - 2); - const accepted = verdict === "accepted"; - const suffix = accepted ? "" : " · rejected"; - const marker = theme.fg("dim", `${suffix}${expanded ? " ▾" : " ▸"}`); - const title = wrapTextWithAnsi( - details.title, - Math.max(1, inner - visibleWidth(suffix) - 2), - ); - const icon = accepted ? theme.fg("warning", "◆") : theme.fg("muted", "✗"); + const marker = theme.fg("dim", expanded ? " ▾" : " ▸"); + const title = wrapTextWithAnsi(details.title, Math.max(1, inner - 2)); const lines = title.map((part, index) => { - const text = accepted ? theme.bold(part) : theme.fg("muted", part); - const first = index === 0 ? icon : " "; - return `${first} ${text}${index === title.length - 1 ? marker : ""}`; + const first = index === 0 ? theme.fg("warning", "◆") : " "; + return `${first} ${theme.bold(part)}${index === title.length - 1 ? marker : ""}`; }); - if (accepted || expanded) { - const location = `:${String(details.line)}`; - const by = details.reviewer === undefined ? "" : ` · ${details.reviewer}`; - const path = truncatePath( - details.file, - Math.max(1, inner - visibleWidth(location + by)), - ); - lines.push(` ${theme.fg("dim", path + location + by)}`); - } + const location = `:${String(details.line)}`; + const by = details.reviewer === undefined ? "" : ` · ${details.reviewer}`; + const path = truncatePath( + details.file, + Math.max(1, inner - visibleWidth(location + by)), + ); + lines.push(` ${theme.fg("dim", path + location + by)}`); if (expanded) lines.push( ...wrapTextWithAnsi(details.evidence, inner).map( @@ -80,51 +63,45 @@ export function cardLines( } /** - * A card's own click toggle, remembered per message or entry because Pi - * rebuilds the component on every expand or theme change. `base` records the - * global expand state when clicked, so a later ctrl+o overrides the toggle. + * A card's own click toggle, remembered per message because Pi rebuilds the + * component on every expand or theme change. `base` records the global expand + * state when clicked, so a later ctrl+o overrides the toggle. */ const toggled = new WeakMap(); -function card( - key: CardKey, - payload: unknown, - verdict: Verdict, - theme: Painter, - expanded: boolean, - pad: number, -): Component | undefined { - const parsed = CardDetails.safeParse(payload); +function isOpen(key: CardKey, expanded: boolean): boolean { + const state = toggled.get(key); + return state?.base === expanded ? state.open : expanded; +} + +/** Accepted findings; unknown payloads fall back to Pi's default rendering. */ +export const renderAccepted: MessageRenderer = ( + message, + options, + theme, +): Component | undefined => { + const parsed = CardDetails.safeParse(message.details); if (!parsed.success) return; - const open = (): boolean => { - const state = toggled.get(key); - return state?.base === expanded ? state.open : expanded; - }; + const { expanded, outputPad } = options; return { render: (width: number): string[] => - cardLines(parsed.data, verdict, theme, width, open(), pad), + cardLines( + parsed.data, + theme, + width, + isOpen(message, expanded), + outputPad, + ), handleMouse(event) { if (event.type !== "click" || event.button !== "left") return; - toggled.set(key, { open: !open(), base: expanded }); + toggled.set(message, { + open: !isOpen(message, expanded), + base: expanded, + }); return { handled: true, render: true }; }, invalidate(): void { return; }, }; -} - -/** Accepted findings; unknown payloads fall back to Pi's default rendering. */ -export const renderAccepted: MessageRenderer = (message, options, theme) => - card( - message, - message.details, - "accepted", - theme, - options.expanded, - options.outputPad, - ); - -/** Rejected findings, stored as transcript-only entries. */ -export const renderRejected: EntryRenderer = (entry, options, theme) => - card(entry, entry.data, "rejected", theme, options.expanded, 1); +}; diff --git a/src/index.ts b/src/index.ts index 9b0b17d..3885d0f 100644 --- a/src/index.ts +++ b/src/index.ts @@ -14,9 +14,7 @@ import { } from "./change-evidence.js"; import { ACCEPTED_MESSAGE, - REJECTED_ENTRY, renderAccepted, - renderRejected, type CardDetails, } from "./finding-card.js"; import { FindingAdmission } from "./finding-admission.js"; @@ -957,6 +955,7 @@ export default function pairProgrammer(pi: ExtensionAPI): void { reason: params.reason, ...optionalReviewer(reviewerNames.get(finding.reviewer)), } satisfies CardDetails; + // Only accepted findings reach the transcript; rejections stay in the feed. if (params.decision === "accept") pi.sendMessage( { @@ -967,8 +966,6 @@ export default function pairProgrammer(pi: ExtensionAPI): void { }, { triggerTurn: false }, ); - // Rejections are transcript-only: rendered for the user, never sent to the model. - else pi.appendEntry(REJECTED_ENTRY, details); } return Promise.resolve({ content: [ @@ -1020,8 +1017,6 @@ export default function pairProgrammer(pi: ExtensionAPI): void { // OMP may not expose message renderers; its default rendering stays readable. if (typeof pi.registerMessageRenderer === "function") pi.registerMessageRenderer(ACCEPTED_MESSAGE, renderAccepted); - if (typeof pi.registerEntryRenderer === "function") - pi.registerEntryRenderer(REJECTED_ENTRY, renderRejected); pi.registerCommand("pair-feed", { description: "Toggle the live review sidebar", diff --git a/test/extension.test.ts b/test/extension.test.ts index 731ff3d..17cee63 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -2874,27 +2874,14 @@ it("narrates review progress above the editor without surfacing finding contents decision: "reject", reason: "Intentional", }); - expect( - environment.rejectedCards.map((card) => - JSON.stringify(card, ["title", "reason", "reviewer"]), - ), - ).toEqual( - findings(environment.entries).map((finding) => - JSON.stringify({ - title: finding.title, - reason: "Intentional", - reviewer: "gpt-5", - }), - ), - ); + // Rejections stay in the opt-in review feed, never the transcript. + expect(environment.rejectedCards).toEqual([]); expect( environment.sendMessage.mock.calls.some( ([message]) => message.customType === "pair-programmer-accepted", ), ).toBe(false); - expect(environment.renderers.get("pair-programmer-rejected")).toBeTypeOf( - "function", - ); + expect(environment.renderers.has("pair-programmer-rejected")).toBe(false); expect(environment.status()).toBe("◆ pair · watching"); await environment.emit("session_shutdown"); await environment.decide("late", { diff --git a/test/finding-card.test.ts b/test/finding-card.test.ts index cd126b1..04050a7 100644 --- a/test/finding-card.test.ts +++ b/test/finding-card.test.ts @@ -5,11 +5,7 @@ import { type TuiMouseEvent, } from "@earendil-works/pi-tui"; import { describe, expect, it } from "vitest"; -import { - cardLines, - renderAccepted, - renderRejected, -} from "../src/finding-card.js"; +import { cardLines, renderAccepted } from "../src/finding-card.js"; const theme = { fg: (color: string, text: string) => `<${color}>${text}`, @@ -38,39 +34,15 @@ const render = ( theme as unknown as Args[2], )?.render(width); -describe("rejected finding card", () => { - const render = ( - expanded: boolean, - data: unknown = details, - ): string[] | undefined => - renderRejected( - { data } as Parameters[0], - { expanded }, - plain as unknown as Args[2], - )?.render(120); - - it("folds to one dimmed line and expands to location, evidence and reason", () => { - expect(render(false)).toEqual([ - " ✗ Imperative loop used to construct collection · rejected ▸", - ]); - const open = render(true) ?? []; - expect(open[0]).toMatch(/· rejected ▾$/u); - expect(open[1]).toContain(`${details.file}:91`); - expect(open.at(-1)).toBe(" “Reworking it for the unsafe-index warning.”"); - }); - - it("toggles by click and ignores malformed entries", () => { - const entry = { data: details } as Parameters[0]; - const card = renderRejected( - entry, - { expanded: false }, +/** Rebuilds the same message's card, as Pi does on each expand change. */ +const cardFor = + (message: Args[0]) => + (expanded: boolean): Component | undefined => + renderAccepted( + message, + { expanded, outputPad: 0 }, plain as unknown as Args[2], ); - card?.handleMouse?.({ type: "click", button: "left" } as TuiMouseEvent); - expect(card?.render(120).length).toBeGreaterThan(1); - expect(render(false, { title: "only" })).toBeUndefined(); - }); -}); describe("accepted finding card", () => { it("collapses to the title and a file-name-first location", () => { @@ -78,7 +50,7 @@ describe("accepted finding card", () => { " ◆ *Imperative loop used to construct collection ▸", ` ${details.file}:91`, ]); - expect(cardLines(details, "accepted", plain, 70, false, 1)).toEqual([ + expect(cardLines(details, plain, 70, false, 1)).toEqual([ " ◆ Imperative loop used to construct collection ▸", " …/chat_detail/widgets/message_bubble/linkified_message_text.dart:91", ]); @@ -94,23 +66,17 @@ describe("accepted finding card", () => { " “Reworking it for the unsafe-index warning.”", ); expect( - cardLines( - { ...details, reviewer: "entropy" }, - "accepted", - plain, - 60, - false, - )[1], + cardLines({ ...details, reviewer: "entropy" }, plain, 60, false)[1], ).toBe(" …/message_bubble/linkified_message_text.dart:91 · entropy"); }); it("wraps within narrow widths and keeps the file name", () => { for (const width of [3, 12, 24, 40]) { - const lines = cardLines(details, "accepted", plain, width, true, 4); + const lines = cardLines(details, plain, width, true, 4); for (const line of lines) expect(visibleWidth(line)).toBeLessThanOrEqual(width); } - const narrow = cardLines(details, "accepted", plain, 24, false); + const narrow = cardLines(details, plain, 24, false); expect(narrow.at(-1)).toContain("…"); expect(narrow.at(-1)).toContain(":91"); expect(narrow.slice(1, -1).every((line) => line.startsWith(" "))).toBe( @@ -119,13 +85,7 @@ describe("accepted finding card", () => { }); it("toggles by left click, survives rebuilds, and yields to the global expand", () => { - const message = { details } as Args[0]; - const card = (expanded: boolean): Component | undefined => - renderAccepted( - message, - { expanded, outputPad: 0 }, - plain as unknown as Args[2], - ); + const card = cardFor({ details } as Args[0]); const click = { type: "click", button: "left" } as TuiMouseEvent; const collapsed = card(false); expect(collapsed?.render(120)).toHaveLength(2); From 371213a741d4c3c21991c7215903b14c900311b9 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:05:25 +0200 Subject: [PATCH 19/21] fix: let ctrl+o collapse a card that was clicked open --- src/finding-card.ts | 8 ++++++-- test/finding-card.test.ts | 11 +++++++++++ 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/src/finding-card.ts b/src/finding-card.ts index 34ab579..57b371e 100644 --- a/src/finding-card.ts +++ b/src/finding-card.ts @@ -65,13 +65,17 @@ export function cardLines( /** * A card's own click toggle, remembered per message because Pi rebuilds the * component on every expand or theme change. `base` records the global expand - * state when clicked, so a later ctrl+o overrides the toggle. + * state when clicked; once ctrl+o changes it, the toggle is dropped so the + * global state wins from then on. */ const toggled = new WeakMap(); function isOpen(key: CardKey, expanded: boolean): boolean { const state = toggled.get(key); - return state?.base === expanded ? state.open : expanded; + if (state === undefined) return expanded; + if (state.base === expanded) return state.open; + toggled.delete(key); + return expanded; } /** Accepted findings; unknown payloads fall back to Pi's default rendering. */ diff --git a/test/finding-card.test.ts b/test/finding-card.test.ts index 04050a7..67c24d2 100644 --- a/test/finding-card.test.ts +++ b/test/finding-card.test.ts @@ -119,6 +119,17 @@ describe("accepted finding card", () => { ).toHaveLength(4); }); + it("lets ctrl+o collapse a card that was clicked open", () => { + const card = cardFor({ details } as Args[0]); + card(false)?.handleMouse?.({ + type: "click", + button: "left", + } as TuiMouseEvent); + expect(card(false)?.render(120)).toHaveLength(4); + expect(card(true)?.render(120)).toHaveLength(4); + expect(card(false)?.render(120)).toHaveLength(2); + }); + it("falls back to Pi's default rendering for older or malformed messages", () => { expect(render({}, false)).toBeUndefined(); expect( From 5502bf8d829d3b65f8c9534307ecf82b21baef78 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:06:01 +0200 Subject: [PATCH 20/21] fix: keep Pi overlays owned and avoid OMP keyboard capture --- src/host.ts | 20 +++ src/index.ts | 6 +- src/overlay-slot.ts | 95 ++++++++++++++ src/review-runner.ts | 3 +- src/review-sidebar.ts | 144 ++++++++++----------- src/status-overlay.ts | 115 ++++++++--------- test/extension.test.ts | 3 +- test/fake-host.ts | 194 ++++++++++++++++++++++++++++ test/review-sidebar.test.ts | 228 ++++++++++++++------------------- test/status-overlay.test.ts | 247 ++++++++++++++++++------------------ 10 files changed, 655 insertions(+), 400 deletions(-) create mode 100644 src/host.ts create mode 100644 src/overlay-slot.ts create mode 100644 test/fake-host.ts diff --git a/src/host.ts b/src/host.ts new file mode 100644 index 0000000..e48f52a --- /dev/null +++ b/src/host.ts @@ -0,0 +1,20 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; + +export type Host = "pi" | "omp"; + +/** Pi exposes `streamSimple` on its model registry; OMP does not. */ +export function hostOf(ctx: Pick): Host { + return typeof ctx.modelRegistry.streamSimple === "function" ? "pi" : "omp"; +} + +/** + * Whether the host can float a non-capturing overlay. OMP reports + * `mode: "tui"` but ignores `nonCapturing`, so its overlays steal typing. + */ +export function canFloat(ctx: ExtensionContext): boolean { + return ( + ctx.hasUI && + (ctx as Partial).mode === "tui" && + hostOf(ctx) === "pi" + ); +} diff --git a/src/index.ts b/src/index.ts index 3885d0f..2201872 100644 --- a/src/index.ts +++ b/src/index.ts @@ -35,11 +35,11 @@ import { import { StatusOverlay, type StatusTone } from "./status-overlay.js"; import { ReviewFeed } from "./review-feed.js"; import { ReviewSidebar } from "./review-sidebar.js"; +import { hostOf, type Host } from "./host.js"; import { isInherited, reviewFile, ReviewTimeoutError, - type Host, } from "./review-runner.js"; import { ReviewStore, type Finding } from "./review-store.js"; import { @@ -120,10 +120,6 @@ function decisionInteraction( ); } -function hostOf(ctx: ExtensionContext): Host { - return typeof ctx.modelRegistry.streamSimple === "function" ? "pi" : "omp"; -} - function idle(ctx: ExtensionContext): boolean { try { return ctx.isIdle(); diff --git a/src/overlay-slot.ts b/src/overlay-slot.ts new file mode 100644 index 0000000..6a990fe --- /dev/null +++ b/src/overlay-slot.ts @@ -0,0 +1,95 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import type { OverlayHandle, OverlayOptions } from "@earendil-works/pi-tui"; + +type UI = ExtensionContext["ui"]; +type Custom = Parameters[0]; +type Factory = ( + ...args: Parameters extends [...infer Rest, unknown] ? Rest : never +) => Awaited>; + +interface Mount { + ui: UI; + closed: boolean; + handle?: OverlayHandle; + done?: () => void; + requestRender?: () => void; +} + +/** + * Owns at most one floating overlay. Pi's `done()` hides the topmost overlay, + * not the caller's, so closing uses the overlay's own handle; `done()` is only + * a fallback for hosts that never provide one. Each mount is its own token, so + * a stale mount's late completion never affects a newer one. + */ +export class OverlaySlot { + private mount: Mount | undefined; + + /** The host UI the current overlay belongs to. */ + get ui(): UI | undefined { + return this.mount?.ui; + } + + /** + * Replaces any current overlay with one on `ctx.ui`. Per-event contexts + * share one UI, so callers compare `ui` to avoid remounting. `onEnd` runs if + * the host ends the overlay, with whether it was ever created. + */ + open( + ctx: ExtensionContext, + factory: Factory, + options: OverlayOptions | (() => OverlayOptions), + onEnd: (created: boolean) => void, + ): void { + this.close(); + const mount: Mount = { ui: ctx.ui, closed: false }; + this.mount = mount; + const end = (): void => { + if (this.mount !== mount) return; + this.mount = undefined; + onEnd(mount.done !== undefined); + }; + void Promise.resolve( + ctx.ui.custom( + (tui, theme, keys, done) => { + mount.done = () => { + done(undefined); + }; + mount.requestRender = () => { + tui.requestRender(); + }; + if (mount.closed) setTimeout(settle(mount), 0); + return factory(tui, theme, keys); + }, + { + overlay: true, + overlayOptions: options, + onHandle: (handle) => { + mount.handle = handle; + if (mount.closed) handle.hide(); + }, + }, + ), + ).then(end, end); + } + + requestRender(): void { + this.mount?.requestRender?.(); + } + + close(): void { + const mount = this.mount; + if (mount === undefined) return; + this.mount = undefined; + mount.closed = true; + // Without a handle yet, wait for the host to supply one before `done()`. + if (mount.handle === undefined) setTimeout(settle(mount), 0); + else mount.handle.hide(); + } +} + +function settle(mount: Mount): () => void { + return () => { + if (mount.handle === undefined) mount.done?.(); + else mount.handle.hide(); + }; +} diff --git a/src/review-runner.ts b/src/review-runner.ts index 9a21427..74b120e 100644 --- a/src/review-runner.ts +++ b/src/review-runner.ts @@ -2,6 +2,7 @@ import { spawn } from "node:child_process"; import { choice, TypeSafeClient } from "@typesafe-ai/sdk"; import { z } from "zod"; import type { ChangeEvidence } from "./change-evidence.js"; +import type { Host } from "./host.js"; import { observeJudgment, ReviewEventStream, @@ -49,7 +50,7 @@ function jev(): TypeSafeClient { return jevClient; } -export type Host = "pi" | "omp"; +export type { Host } from "./host.js"; export class ReviewTimeoutError extends Error { override name = "ReviewTimeoutError"; diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index d4ead76..ffa3779 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -5,6 +5,8 @@ import { wrapTextWithAnsi, } from "@earendil-works/pi-tui"; import { frame, truncatePath } from "./format.js"; +import { canFloat } from "./host.js"; +import { OverlaySlot } from "./overlay-slot.js"; import type { FeedEntry } from "./review-feed.js"; import type { FindingView } from "./review-store.js"; @@ -223,9 +225,7 @@ export function renderSidebar( export class ReviewSidebar { private readonly entries: () => readonly FeedEntry[]; private readonly lookup: Lookup; - private ctx: ExtensionContext | undefined; - private close: (() => void) | undefined; - private requestRender: (() => void) | undefined; + private readonly slot = new OverlaySlot(); private ticker: NodeJS.Timeout | undefined; /** Reviews the user expanded by click; ids are unique per review. */ private readonly expanded = new Set(); @@ -236,7 +236,7 @@ export class ReviewSidebar { } get open(): boolean { - return this.ctx !== undefined; + return this.slot.ui !== undefined; } toggle(ctx: ExtensionContext): void { @@ -244,7 +244,7 @@ export class ReviewSidebar { this.dispose(); return; } - if (!ctx.hasUI || (ctx as Partial).mode !== "tui") { + if (!canFloat(ctx)) { ctx.ui.notify("The review sidebar requires Pi's terminal UI.", "warning"); return; } @@ -253,88 +253,74 @@ export class ReviewSidebar { /** Re-render after feed or verdict changes; follows session replacement. */ refresh(ctx?: ExtensionContext): void { - if (ctx !== undefined && this.open && ctx !== this.ctx) this.mount(ctx); - this.requestRender?.(); + if (ctx !== undefined && this.open && ctx.ui !== this.slot.ui) { + if (canFloat(ctx)) this.mount(ctx); + else this.dispose(); + } + this.slot.requestRender(); } dispose(): void { + this.stopTicker(); + this.slot.close(); + } + + private stopTicker(): void { clearInterval(this.ticker); this.ticker = undefined; - this.close?.(); - this.close = undefined; - this.requestRender = undefined; - this.ctx = undefined; } private mount(ctx: ExtensionContext): void { - this.dispose(); - this.ctx = ctx; - let active = true; - this.close = () => { - active = false; - }; - const release = (): void => { - if (this.ctx === ctx) this.dispose(); - }; - void Promise.resolve( - ctx.ui.custom( - (tui, theme, _keys, done) => { - const finish = (): void => { - active = false; - done(undefined); - }; - if (active) this.close = finish; - else queueMicrotask(finish); - this.requestRender = () => { - tui.requestRender(); - }; - this.ticker = setInterval(() => { - if (this.entries().some((entry) => entry.phase === "running")) - tui.requestRender(); - }, 250); - this.ticker.unref(); - let rows: SidebarLayout["rows"] = []; - return { - render: (width: number): string[] => { - const layout = layoutSidebar( - this.entries(), - this.lookup, - theme, - width, - Math.max(8, tui.terminal.rows - EDITOR_RESERVE), - Date.now(), - this.expanded, - ); - rows = layout.rows; - return layout.lines; - }, - handleMouse: (event) => { - if (event.type !== "click" || event.button !== "left") return; - const id = rows[event.y]; - const entry = this.entries().find( - (candidate) => candidate.id === id, - ); - if (entry === undefined || entry.phase === "running") return; - if (!this.expanded.delete(entry.id)) this.expanded.add(entry.id); - return { handled: true, render: true }; - }, - invalidate(): void { - return; - }, - }; - }, - { - overlay: true, - overlayOptions: { - anchor: "top-right", - width: "34%", - minWidth: 38, - margin: { top: 1, right: 1 }, - nonCapturing: true, - visible: (columns: number) => columns >= MIN_COLUMNS, + this.stopTicker(); + this.slot.open( + ctx, + (tui, theme) => { + let rows: SidebarLayout["rows"] = []; + return { + render: (width: number): string[] => { + const layout = layoutSidebar( + this.entries(), + this.lookup, + theme, + width, + Math.max(8, tui.terminal.rows - EDITOR_RESERVE), + Date.now(), + this.expanded, + ); + rows = layout.rows; + return layout.lines; }, - }, - ), - ).then(release, release); + handleMouse: (event) => { + if (event.type !== "click" || event.button !== "left") return; + const id = rows[event.y]; + const entry = this.entries().find( + (candidate) => candidate.id === id, + ); + if (entry === undefined || entry.phase === "running") return; + if (!this.expanded.delete(entry.id)) this.expanded.add(entry.id); + return { handled: true, render: true }; + }, + invalidate(): void { + return; + }, + }; + }, + { + anchor: "top-right", + width: "34%", + minWidth: 38, + margin: { top: 1, right: 1 }, + nonCapturing: true, + visible: (columns: number) => columns >= MIN_COLUMNS, + }, + () => { + this.stopTicker(); + }, + ); + this.ticker = setInterval(() => { + if (this.entries().some((entry) => entry.phase === "running")) + this.slot.requestRender(); + }, 250); + this.ticker.unref(); } } diff --git a/src/status-overlay.ts b/src/status-overlay.ts index 15b34c5..7c8baf5 100644 --- a/src/status-overlay.ts +++ b/src/status-overlay.ts @@ -1,5 +1,7 @@ import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; import { truncateToWidth, visibleWidth } from "@earendil-works/pi-tui"; +import { canFloat } from "./host.js"; +import { OverlaySlot } from "./overlay-slot.js"; export type StatusTone = "dim" | "accent" | "warning" | "success"; export interface StatusState { @@ -9,6 +11,8 @@ export interface StatusState { const KEY = "pair-programmer"; +type UI = ExtensionContext["ui"]; + interface Painter { fg(color: StatusTone | "muted", text: string): string; } @@ -18,88 +22,79 @@ function styled(theme: Painter, state: StatusState): string { return `${theme.fg(state.tone, "◆")} ${theme.fg("muted", label)}`; } -/** Permanent, non-focusable top-right badge; widget fallback outside Pi's TUI. */ +/** + * Permanent, non-focusable top-right badge in Pi's TUI; a widget line above + * the editor elsewhere (RPC, OMP) or when the host cannot keep the overlay. + */ export class StatusOverlay { private state: StatusState = { tone: "success", text: "watching" }; - private ctx: ExtensionContext | undefined; - private close: (() => void) | undefined; - private requestRender: (() => void) | undefined; + private readonly slot = new OverlaySlot(); + /** UI showing the widget line instead of the badge. */ + private widgetUi: UI | undefined; show(ctx: ExtensionContext, state: StatusState): void { this.state = state; - if (ctx !== this.ctx) this.mount(ctx); - if (this.close === undefined) this.widget(); - else this.requestRender?.(); + const ui = ctx.ui; + if (this.widgetUi !== ui && canFloat(ctx)) { + if (this.slot.ui === ui) this.slot.requestRender(); + else this.float(ctx); + return; + } + if (this.slot.ui !== undefined) this.slot.close(); + if (this.widgetUi !== undefined && this.widgetUi !== ui) this.clearWidget(); + this.widgetUi = ui; + this.widget(); } dispose(): void { - this.close?.(); - this.close = undefined; - this.requestRender = undefined; - this.ctx?.ui.setWidget(KEY, undefined); - this.ctx = undefined; + this.slot.close(); + this.clearWidget(); } private label(): string { return `◆ pair · ${this.state.text}`; } + private clearWidget(): void { + this.widgetUi?.setWidget(KEY, undefined); + this.widgetUi = undefined; + } + private widget(): void { - const ui = this.ctx?.ui; + const ui = this.widgetUi; // SAFETY: OMP and test hosts may omit Pi's theme; plain text remains valid. - const theme = (ui as Partial | undefined)?.theme; + const theme = (ui as Partial | undefined)?.theme; ui?.setWidget(KEY, [ theme === undefined ? this.label() : styled(theme, this.state), ]); } - private mount(ctx: ExtensionContext): void { - this.dispose(); - this.ctx = ctx; - if (!ctx.hasUI || (ctx as Partial).mode !== "tui") return; - let open = true; - let mounted = false; - this.close = () => { - open = false; - }; - const fallback = (): void => { - if (mounted || this.ctx !== ctx) return; - this.close = undefined; - this.widget(); - }; - void Promise.resolve( - ctx.ui.custom( - (tui, theme, _keys, done) => { - mounted = true; - this.requestRender = () => { - tui.requestRender(); - }; - const finish = (): void => { - done(undefined); - }; - if (open) this.close = finish; - else queueMicrotask(finish); - return { - render: (width: number): string[] => { - const body = " " + styled(theme, this.state) + " "; - return [truncateToWidth(body, Math.max(1, width), "…")]; - }, - invalidate(): void { - return; - }, - }; + private float(ctx: ExtensionContext): void { + this.clearWidget(); + const ui = ctx.ui; + this.slot.open( + ctx, + (_tui, theme) => ({ + render: (width: number): string[] => { + const body = " " + styled(theme, this.state) + " "; + return [truncateToWidth(body, Math.max(1, width), "…")]; }, - { - overlay: true, - overlayOptions: () => ({ - anchor: "top-right", - width: visibleWidth(this.label()) + 2, - margin: { top: 0, right: 1 }, - nonCapturing: true, - visible: (columns: number) => columns >= 40, - }), + invalidate(): void { + return; }, - ), - ).then(fallback, fallback); + }), + () => ({ + anchor: "top-right", + width: visibleWidth(this.label()) + 2, + margin: { top: 0, right: 1 }, + nonCapturing: true, + visible: (columns: number) => columns >= 40, + }), + () => { + // The host could not keep the badge: fall back to the widget line. + this.widgetUi = ui; + this.widget(); + }, + ); } } diff --git a/test/extension.test.ts b/test/extension.test.ts index 17cee63..79a8e84 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -343,7 +343,8 @@ async function setup( getEntries: vi.fn(() => transcript), }, hasUI: true, - mode: host === "pi" ? "tui" : undefined, + // OMP 18.4.2 reports "tui" too, but ignores non-capturing overlays. + mode: "tui", ui: { notify, custom, setWidget }, } as unknown as ExtensionContext; const emit = async (name: string, event: unknown = {}): Promise => { diff --git a/test/fake-host.ts b/test/fake-host.ts new file mode 100644 index 0000000..a32bec6 --- /dev/null +++ b/test/fake-host.ts @@ -0,0 +1,194 @@ +import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; +import type { + Component, + OverlayHandle, + OverlayOptions, + TuiMouseEvent, +} from "@earendil-works/pi-tui"; +import { vi, type Mock } from "vitest"; + +type Custom = ExtensionContext["ui"]["custom"]; +type Factory = Parameters[0]; +type CustomOptions = NonNullable[1]>; + +export interface Overlay { + component: Component; + options: OverlayOptions | undefined; + /** Resolves `ui.custom`, as Pi's `done()` does, whether or not it is shown. */ + settled: boolean; +} + +export interface FakeHost { + ctx: ExtensionContext; + custom: Mock; + setWidget: Mock<(key: string, lines: string[] | undefined) => void>; + notify: Mock; + requestRender: Mock; + /** Visible overlays, bottom to top. */ + stack: Overlay[]; + /** Every overlay ever created, oldest first. */ + created: Overlay[]; + /** Holds the factory until `release()` when deferred. */ + defer(): void; + release(): void; + /** A fresh per-event context sharing this host's UI, like Pi's runner. */ + event(): ExtensionContext; + top(): Overlay | undefined; + render(overlay: Overlay | undefined, width: number): string[]; + click(overlay: Overlay | undefined, y: number): unknown; +} + +interface HostOptions { + host?: "pi" | "omp"; + mode?: string | undefined; + rows?: number; + hasUI?: boolean; + theme?: unknown; + /** Omit `onHandle`, like hosts older than Pi's overlay handles. */ + handles?: boolean; +} + +const plain = { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, +}; + +function handleFor(stack: Overlay[], entry: Overlay): OverlayHandle { + return { + hide: () => { + const index = stack.indexOf(entry); + if (index !== -1) stack.splice(index, 1); + }, + } as OverlayHandle; +} + +/** + * Mirrors Pi 0.87.1's `showExtensionCustom`: `done()` pops the *topmost* + * overlay, and `onHandle` exposes a `hide()` that removes only its own. + */ +export function fakeHost(options: HostOptions = {}): FakeHost { + const { + host = "pi", + mode = "tui", + rows = 20, + hasUI = true, + theme = plain, + handles = true, + } = options; + const stack: Overlay[] = []; + const created: Overlay[] = []; + const requestRender = vi.fn(); + const tui = { requestRender, terminal: { rows } }; + let deferred: (() => void)[] | undefined; + const custom = vi.fn( + (factory: Factory, opts?: CustomOptions): Promise => + new Promise((resolve) => { + let closed = false; + const run = (): void => { + let overlay: Overlay | undefined; + const done = (result: unknown): void => { + if (closed) return; + closed = true; + stack.pop(); + if (overlay !== undefined) overlay.settled = true; + resolve(result); + }; + void Promise.resolve( + factory( + tui as unknown as Parameters[0], + theme as Parameters[1], + {} as Parameters[2], + done, + ), + ).then((component) => { + if (closed) return; + const resolved = + typeof opts?.overlayOptions === "function" + ? opts.overlayOptions() + : opts?.overlayOptions; + overlay = { component, options: resolved, settled: false }; + created.push(overlay); + stack.push(overlay); + if (handles) opts?.onHandle?.(handleFor(stack, overlay)); + return overlay; + }); + }; + if (deferred === undefined) run(); + else deferred.push(run); + }), + ); + const setWidget = vi.fn<(key: string, lines: string[] | undefined) => void>(); + const notify = vi.fn(); + const ui = { + custom, + setWidget, + notify, + ...(theme === plain ? {} : { theme }), + }; + const event = (): ExtensionContext => + ({ + hasUI, + mode, + ui, + modelRegistry: host === "pi" ? { streamSimple: vi.fn() } : {}, + }) as unknown as ExtensionContext; + return { + ctx: event(), + custom, + setWidget, + notify, + requestRender, + stack, + created, + defer: () => { + deferred = []; + }, + release: () => { + const pending = deferred ?? []; + deferred = undefined; + for (const run of pending) run(); + }, + event, + top: () => stack.at(-1), + render: (overlay, width) => overlay?.component.render(width) ?? [], + click: (overlay, y) => + overlay?.component.handleMouse?.({ + type: "click", + button: "left", + y, + } as TuiMouseEvent), + }; +} + +/** Opens a capturing overlay such as `/pair-stats` on the host. */ +export function openCapturing(host: FakeHost): { + closed: () => boolean; + close: () => void; +} { + let done: ((value: undefined) => void) | undefined; + let closed = false; + void host.ctx.ui + .custom( + (_tui, _theme, _keys, finish) => { + done = finish; + return { + render: () => ["stats"], + invalidate: () => { + return; + }, + }; + }, + { overlay: true }, + ) + .then(() => { + closed = true; + return closed; + }); + return { + closed: () => closed, + close: () => done?.(undefined), + }; +} + +export const flush = (): Promise => + new Promise((resolve) => setTimeout(resolve, 0)); diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index f2a938e..8fdbd00 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -1,7 +1,7 @@ -import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; import { visibleWidth, type TuiMouseEvent } from "@earendil-works/pi-tui"; -import { afterEach, describe, expect, it, vi, type Mock } from "vitest"; +import { afterEach, describe, expect, it, vi } from "vitest"; import { ReviewFeed } from "../src/review-feed.js"; +import { fakeHost, flush, openCapturing } from "./fake-host.js"; import { MIN_COLUMNS, renderSidebar, @@ -176,74 +176,6 @@ describe("renderSidebar", () => { }); }); -type Factory = Parameters[0]; -interface Host { - ctx: ExtensionContext; - custom: Mock; - notify: Mock; - requestRender: Mock; - closed: Mock; - options: () => { - anchor: string; - nonCapturing: boolean; - visible: (columns: number) => boolean; - }; - render: (width: number) => string[] | undefined; - click: (y: number, button?: string, type?: string) => unknown; -} - -function host(mode = "tui", rows = 20): Host { - const requestRender = vi.fn(); - const notify = vi.fn(); - const closed = vi.fn(); - let options: ReturnType | undefined; - let render: ((width: number) => string[]) | undefined; - let mouse: ((event: TuiMouseEvent) => unknown) | undefined; - const custom = vi.fn( - async ( - factory: Factory, - opts: { overlayOptions: ReturnType }, - ): Promise => { - options = opts.overlayOptions; - const done = Promise.withResolvers(); - const component = await factory( - { - requestRender, - terminal: { rows }, - } as unknown as Parameters[0], - plain as unknown as Parameters[1], - {} as Parameters[2], - () => { - closed(); - done.resolve(undefined); - }, - ); - component.invalidate(); - render = (width) => component.render(width); - mouse = (event) => component.handleMouse?.(event); - return done.promise; - }, - ); - return { - ctx: { - hasUI: true, - mode, - ui: { custom, notify }, - } as unknown as ExtensionContext, - custom, - notify, - requestRender, - closed, - options: () => { - if (options === undefined) throw new Error("not mounted"); - return options; - }, - render: (width) => render?.(width), - click: (y, button = "left", type = "click") => - mouse?.({ type, button, y } as TuiMouseEvent), - }; -} - afterEach(() => { vi.useRealTimers(); }); @@ -252,31 +184,33 @@ describe("ReviewSidebar", () => { it("toggles a non-capturing right-hand overlay that renders the live feed", async () => { vi.useFakeTimers(); const feed = new ReviewFeed(); - const tui = host(); + const tui = fakeHost(); const sidebar = new ReviewSidebar(() => feed.list(), lookup); sidebar.refresh(tui.ctx); expect(tui.custom).not.toHaveBeenCalled(); sidebar.toggle(tui.ctx); - await Promise.resolve(); + await vi.advanceTimersByTimeAsync(0); expect(sidebar.open).toBe(true); - expect(tui.options()).toMatchObject({ + const panel = tui.top(); + expect(panel?.options).toMatchObject({ anchor: "top-right", nonCapturing: true, }); - expect(tui.options().visible(MIN_COLUMNS - 1)).toBe(false); - expect(tui.options().visible(MIN_COLUMNS)).toBe(true); - expect(tui.render(40)).toHaveLength(10); + expect(panel?.options?.visible?.(MIN_COLUMNS - 1, 20)).toBe(false); + expect(panel?.options?.visible?.(MIN_COLUMNS, 20)).toBe(true); + expect(tui.render(panel, 40)).toHaveLength(10); + panel?.component.invalidate(); vi.advanceTimersByTime(250); expect(tui.requestRender).not.toHaveBeenCalled(); feed.start("a", job, Date.now()); vi.advanceTimersByTime(250); expect(tui.requestRender).toHaveBeenCalledTimes(1); - sidebar.refresh(); + sidebar.refresh(tui.event()); expect(tui.requestRender).toHaveBeenCalledTimes(2); + expect(tui.custom).toHaveBeenCalledTimes(1); sidebar.toggle(tui.ctx); expect(sidebar.open).toBe(false); - await Promise.resolve(); - expect(tui.closed).toHaveBeenCalledTimes(1); + expect(tui.stack).toEqual([]); vi.advanceTimersByTime(1000); expect(tui.requestRender).toHaveBeenCalledTimes(2); }); @@ -287,48 +221,100 @@ describe("ReviewSidebar", () => { feed.finish("done", "success", 1); feed.attach("done", ["accepted"]); feed.start("live", { ...job, file: "live.ts" }, Date.now()); - const tui = host("tui", 40); + const tui = fakeHost({ rows: 40 }); const sidebar = new ReviewSidebar(() => feed.list(), lookup); sidebar.toggle(tui.ctx); - await Promise.resolve(); - const rows = (): string[] => tui.render(40) ?? []; + await flush(); + const panel = tui.top(); + const rows = (): string[] => tui.render(panel, 40); const at = (name: string): number => rows().findIndex((line) => line.includes(name)); expect(rows().join("\n")).not.toContain("Duplicate helper"); + const mouse = (event: Partial): unknown => + panel?.component.handleMouse?.({ + type: "click", + button: "left", + ...event, + } as TuiMouseEvent); for (const ignored of [ - tui.click(0), - tui.click(at("live.ts")), - tui.click(at("done.ts"), "right"), - tui.click(at("done.ts"), "left", "press"), - tui.click(99), + mouse({ y: 0 }), + mouse({ y: at("live.ts") }), + mouse({ y: at("done.ts"), button: "right" }), + mouse({ y: at("done.ts"), type: "press" }), + mouse({ y: 99 }), ]) expect(ignored).toBeUndefined(); - expect(tui.click(at("done.ts"))).toEqual({ handled: true, render: true }); + expect(tui.click(panel, at("done.ts"))).toEqual({ + handled: true, + render: true, + }); expect(rows().join("\n")).toContain("Duplicate helper :42"); - expect(tui.click(at("Duplicate helper"))).toMatchObject({ handled: true }); + expect(tui.click(panel, at("Duplicate helper"))).toMatchObject({ + handled: true, + }); expect(rows().join("\n")).not.toContain("Duplicate helper"); sidebar.dispose(); }); - it("follows session replacement while open", async () => { - const first = host(); - const second = host(); + it("follows session replacement while open, and closes if it cannot float", async () => { + const first = fakeHost(); + const second = fakeHost(); const sidebar = new ReviewSidebar(() => [], lookup); sidebar.toggle(first.ctx); - sidebar.refresh(first.ctx); + sidebar.refresh(first.event()); expect(first.custom).toHaveBeenCalledTimes(1); sidebar.refresh(second.ctx); - await Promise.resolve(); - expect(first.closed).toHaveBeenCalledTimes(1); - expect(second.custom).toHaveBeenCalledTimes(1); + await flush(); + expect(first.stack).toEqual([]); + expect(second.stack).toHaveLength(1); + expect(sidebar.open).toBe(true); + sidebar.refresh(fakeHost({ host: "omp" }).ctx); + expect(sidebar.open).toBe(false); + expect(second.stack).toEqual([]); + }); + + it("keeps the newest mount when an older one's cleanup lands late (A → B → A)", async () => { + const a = fakeHost(); + const b = fakeHost(); + const sidebar = new ReviewSidebar(() => [], lookup); + a.defer(); + sidebar.toggle(a.ctx); + sidebar.refresh(b.ctx); + sidebar.refresh(a.event()); + a.release(); + await flush(); + await flush(); expect(sidebar.open).toBe(true); + expect(a.stack).toHaveLength(1); + expect(b.stack).toEqual([]); sidebar.dispose(); + await flush(); + expect(a.stack).toEqual([]); + }); + + it("never dismisses an overlay stacked above it", async () => { + const tui = fakeHost(); + const sidebar = new ReviewSidebar(() => [], lookup); + sidebar.toggle(tui.ctx); + await flush(); + const stats = openCapturing(tui); + await flush(); + sidebar.toggle(tui.ctx); + await flush(); + expect(tui.stack.map((entry) => tui.render(entry, 40))).toEqual([ + ["stats"], + ]); + expect(stats.closed()).toBe(false); }); - it("explains that the sidebar needs Pi's terminal UI", () => { - const headless = host(); - headless.ctx.hasUI = false; - for (const unsupported of [host("rpc"), headless]) { + it.each([ + { host: "pi", mode: "rpc", hasUI: true }, + { host: "pi", mode: "tui", hasUI: false }, + { host: "omp", mode: "tui", hasUI: true }, + ] as const)( + "refuses to float on $host in $mode mode (UI: $hasUI)", + (options) => { + const unsupported = fakeHost(options); const sidebar = new ReviewSidebar(() => [], lookup); sidebar.toggle(unsupported.ctx); expect(sidebar.open).toBe(false); @@ -337,50 +323,32 @@ describe("ReviewSidebar", () => { expect.stringContaining("terminal UI"), "warning", ); - } - }); + }, + ); it("closes when the host ends the overlay or disposes it before mounting", async () => { - const ended = host(); + const ended = fakeHost(); ended.custom.mockReturnValueOnce(Promise.resolve(undefined)); const sidebar = new ReviewSidebar(() => [], lookup); sidebar.toggle(ended.ctx); - await new Promise((resolve) => setTimeout(resolve, 0)); + await flush(); expect(sidebar.open).toBe(false); - const failed = host(); + const failed = fakeHost(); failed.custom.mockReturnValueOnce(Promise.reject(new Error("no"))); sidebar.toggle(failed.ctx); - await new Promise((resolve) => setTimeout(resolve, 0)); + await flush(); expect(sidebar.open).toBe(false); - const late = host(); - let mount: (() => void) | undefined; - late.custom.mockImplementationOnce( - (factory: Factory): Promise => { - return new Promise((resolve) => { - mount = () => { - const component = factory( - { - requestRender: vi.fn(), - terminal: { rows: 20 }, - } as unknown as Parameters[0], - plain as unknown as Parameters[1], - {} as Parameters[2], - () => { - resolve(undefined); - }, - ); - expect(component).toBeDefined(); - }; - }); - }, - ); + const late = fakeHost(); + late.defer(); sidebar.toggle(late.ctx); sidebar.toggle(late.ctx); - mount?.(); - await new Promise((resolve) => setTimeout(resolve, 0)); + late.release(); + await flush(); + await flush(); expect(sidebar.open).toBe(false); + expect(late.stack).toEqual([]); sidebar.toggle(late.ctx); expect(sidebar.open).toBe(true); sidebar.dispose(); diff --git a/test/status-overlay.test.ts b/test/status-overlay.test.ts index 36ad7ab..33aa0c6 100644 --- a/test/status-overlay.test.ts +++ b/test/status-overlay.test.ts @@ -1,185 +1,185 @@ -import type { ExtensionContext } from "@earendil-works/pi-coding-agent"; -import { describe, expect, it, vi, type Mock } from "vitest"; +import { describe, expect, it } from "vitest"; import { StatusOverlay } from "../src/status-overlay.js"; +import { fakeHost, flush, openCapturing } from "./fake-host.js"; -type Factory = Parameters[0]; -interface Options { - overlayOptions: () => { - anchor: string; - width: number; - nonCapturing: boolean; - visible: (columns: number) => boolean; - }; -} - -interface Host { - ctx: ExtensionContext; - custom: Mock<(factory: Factory, opts: Options) => Promise>; - setWidget: ReturnType; - requestRender: ReturnType; - closed: ReturnType; - render: (width: number) => string[] | undefined; - options: () => ReturnType | undefined; -} - -function host(mode: string | undefined = "tui"): Host { - const requestRender = vi.fn(); - const setWidget = vi.fn(); - const closed = vi.fn(); - let render: ((width: number) => string[]) | undefined; - let options: Options | undefined; - const custom = vi.fn( - async (factory: Factory, opts: Options): Promise => { - options = opts; - const done = Promise.withResolvers(); - const component = await factory( - { requestRender } as unknown as Parameters[0], - { - fg: (color: string, text: string) => `<${color}>${text}`, - } as unknown as Parameters[1], - {} as Parameters[2], - () => { - closed(); - done.resolve(undefined); - }, - ); - render = (width) => component.render(width); - component.invalidate(); - return done.promise; - }, - ); - const ctx = { - hasUI: true, - mode, - ui: { custom, setWidget }, - } as unknown as ExtensionContext; - return { - ctx, - custom, - setWidget, - requestRender, - closed, - render: (width: number) => render?.(width), - options: () => options?.overlayOptions(), - }; -} +const colored = { + fg: (color: string, text: string) => `<${color}>${text}`, + bold: (text: string) => text, +}; describe("StatusOverlay", () => { it("pins a non-capturing badge to the top-right and updates in place", async () => { - const tui = host(); + const tui = fakeHost({ theme: colored }); const overlay = new StatusOverlay(); overlay.show(tui.ctx, { tone: "success", text: "watching" }); - await Promise.resolve(); - expect(tui.options()).toMatchObject({ + await flush(); + const badge = tui.top(); + expect(badge?.options).toMatchObject({ anchor: "top-right", width: 19, nonCapturing: true, }); - expect(tui.options()?.visible(39)).toBe(false); - expect(tui.options()?.visible(40)).toBe(true); - expect(tui.render(80)).toEqual([" ◆ pair · watching "]); - overlay.show(tui.ctx, { tone: "warning", text: "2 awaiting decision" }); + expect(badge?.options?.visible?.(39, 20)).toBe(false); + expect(badge?.options?.visible?.(40, 20)).toBe(true); + expect(tui.render(badge, 80)).toEqual([ + " ◆ pair · watching ", + ]); + overlay.show(tui.event(), { + tone: "warning", + text: "2 awaiting decision", + }); expect(tui.custom).toHaveBeenCalledTimes(1); - expect(tui.requestRender).toHaveBeenCalledTimes(2); - expect(tui.render(80)?.[0]).toContain("◆"); - expect(tui.options()?.width).toBe(30); - expect(tui.render(5)?.[0]).toContain("…"); + expect(tui.requestRender).toHaveBeenCalledTimes(1); + expect(tui.render(badge, 80)[0]).toContain("◆"); + expect(tui.render(badge, 5)[0]).toContain("…"); expect(tui.setWidget).not.toHaveBeenCalledWith( "pair-programmer", expect.anything(), ); overlay.dispose(); - expect(tui.closed).toHaveBeenCalledTimes(1); - expect(tui.setWidget).toHaveBeenLastCalledWith( - "pair-programmer", - undefined, - ); + expect(tui.stack).toEqual([]); + badge?.component.invalidate(); + }); + + it("never dismisses another overlay or remounts for fresh event contexts", async () => { + const tui = fakeHost(); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "success", text: "watching" }); + await flush(); + const badge = tui.top(); + const stats = openCapturing(tui); + await flush(); + overlay.show(tui.event(), { tone: "accent", text: "reviewing · 1" }); + await flush(); + expect(tui.custom).toHaveBeenCalledTimes(2); + expect(tui.stack).toHaveLength(2); + overlay.dispose(); + await flush(); + expect(tui.stack.map((entry) => tui.render(entry, 40))).toEqual([ + ["stats"], + ]); + expect(stats.closed()).toBe(false); + stats.close(); + await flush(); + expect(stats.closed()).toBe(true); + expect(badge?.settled).toBe(false); }); it("closes a badge disposed before the host mounts it", async () => { - const tui = host(); - let mount: (() => void) | undefined; - tui.custom.mockImplementationOnce( - (factory: Factory): Promise => { - return new Promise((resolve) => { - mount = () => { - const component = factory( - { requestRender: vi.fn() } as unknown as Parameters[0], - {} as Parameters[1], - {} as Parameters[2], - () => { - resolve(undefined); - }, - ); - expect(component).toBeDefined(); - }; - }); - }, - ); + const tui = fakeHost(); + tui.defer(); + const overlay = new StatusOverlay(); + overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + overlay.dispose(); + const stats = openCapturing(tui); + tui.release(); + await flush(); + await flush(); + expect(tui.stack.map((entry) => tui.render(entry, 40))).toEqual([ + ["stats"], + ]); + expect(stats.closed()).toBe(false); + }); + + it("closes through done() on hosts that provide no overlay handle", async () => { + const tui = fakeHost({ handles: false }); const overlay = new StatusOverlay(); overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + await flush(); + expect(tui.stack).toHaveLength(1); overlay.dispose(); - mount?.(); - await new Promise((resolve) => setTimeout(resolve, 0)); - expect(tui.setWidget).toHaveBeenLastCalledWith( + await flush(); + expect(tui.stack).toEqual([]); + expect(tui.setWidget).not.toHaveBeenCalledWith( "pair-programmer", - undefined, + expect.anything(), ); }); - it.each(["rpc", "print"])( - "falls back to a widget in %s mode without mounting an overlay", - (mode) => { - const tui = host(mode); + it.each([ + { host: "pi", mode: "rpc" }, + { host: "pi", mode: "print" }, + // OMP reports "tui" but ignores nonCapturing, so a badge would eat typing. + { host: "omp", mode: "tui" }, + ] as const)( + "uses a widget line on $host in $mode mode without mounting an overlay", + ({ host, mode }) => { + const tui = fakeHost({ host, mode }); const overlay = new StatusOverlay(); overlay.show(tui.ctx, { tone: "dim", text: "paused" }); expect(tui.custom).not.toHaveBeenCalled(); expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ "◆ pair · paused", ]); - Object.assign(tui.ctx.ui, { - theme: { fg: (color: string, text: string) => `<${color}>${text}` }, - }); - overlay.show(tui.ctx, { tone: "accent", text: "1 queued" }); + Object.assign(tui.ctx.ui, { theme: colored }); + overlay.show(tui.event(), { tone: "accent", text: "1 queued" }); expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ "◆ pair · 1 queued", ]); + overlay.dispose(); + expect(tui.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); }, ); it("falls back to a widget when the host cannot keep the overlay", async () => { for (const fail of [false, true]) { - const tui = host(); - tui.custom.mockImplementationOnce((): Promise => + const tui = fakeHost(); + tui.custom.mockImplementationOnce(() => fail ? Promise.reject(new Error("no")) : Promise.resolve(undefined), ); const overlay = new StatusOverlay(); overlay.show(tui.ctx, { tone: "success", text: "watching" }); - await new Promise((resolve) => setTimeout(resolve, 0)); + await flush(); expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ "◆ pair · watching", ]); - overlay.show(tui.ctx, { tone: "dim", text: "paused" }); + overlay.show(tui.event(), { tone: "dim", text: "paused" }); + expect(tui.custom).toHaveBeenCalledTimes(1); expect(tui.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ "◆ pair · paused", ]); } }); - it("ignores stale host completions after moving to a new session", async () => { - const first = host(); - const pending = Promise.withResolvers(); - first.custom.mockReturnValueOnce(pending.promise); + it("moves between host UIs and ignores the old one's completions", async () => { + const first = fakeHost(); const overlay = new StatusOverlay(); overlay.show(first.ctx, { tone: "success", text: "watching" }); - const second = host("rpc"); - overlay.show(second.ctx, { tone: "success", text: "watching" }); - pending.resolve(undefined); - await new Promise((resolve) => setTimeout(resolve, 0)); + await flush(); + const rpc = fakeHost({ mode: "rpc" }); + overlay.show(rpc.ctx, { tone: "success", text: "watching" }); + await flush(); + expect(first.stack).toEqual([]); + expect(rpc.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · watching", + ]); + const second = fakeHost(); + overlay.show(second.ctx, { tone: "dim", text: "paused" }); + await flush(); + expect(rpc.setWidget).toHaveBeenLastCalledWith( + "pair-programmer", + undefined, + ); + expect(second.stack).toHaveLength(1); + overlay.dispose(); + }); + + it("clears the previous widget line when moving between widget hosts", () => { + const first = fakeHost({ mode: "rpc" }); + const second = fakeHost({ host: "omp" }); + const overlay = new StatusOverlay(); + overlay.show(first.ctx, { tone: "dim", text: "paused" }); + overlay.show(second.ctx, { tone: "dim", text: "paused" }); expect(first.setWidget).toHaveBeenLastCalledWith( "pair-programmer", undefined, ); + expect(second.setWidget).toHaveBeenLastCalledWith("pair-programmer", [ + "◆ pair · paused", + ]); }); it("does nothing on dispose before any session", () => { @@ -189,8 +189,7 @@ describe("StatusOverlay", () => { }); it("skips overlays for headless contexts", () => { - const tui = host(); - tui.ctx.hasUI = false; + const tui = fakeHost({ hasUI: false }); new StatusOverlay().show(tui.ctx, { tone: "dim", text: "paused" }); expect(tui.custom).not.toHaveBeenCalled(); }); From a3c4b799236a9eb043bfa1f127a039575c745c92 Mon Sep 17 00:00:00 2001 From: Theo Grillat Date: Tue, 29 Sep 2026 11:07:00 +0200 Subject: [PATCH 21/21] feat: offer an on-demand review feed on OMP --- README.md | 12 ++++---- src/index.ts | 50 +++++++++++++++++++-------------- src/review-sidebar.ts | 41 +++++++++++++++++++++++---- src/stats-view.ts | 27 ++++++++++++++---- test/extension.test.ts | 55 +++++++++++++++++++++++++++++++++++++ test/review-sidebar.test.ts | 26 ++++++++++++++++++ test/stats-view.test.ts | 34 +++++++++++++++++++++-- 7 files changed, 205 insertions(+), 40 deletions(-) diff --git a/README.md b/README.md index 3c73ff0..d51273c 100644 --- a/README.md +++ b/README.md @@ -84,12 +84,12 @@ export TYPESAFE_API_KEY="your-api-key" Reviews are on by default. After each successful `write` or `edit`, reviewers run in the background. The coding agent must accept or reject each finding with a reason before continuing with other tools. Accepted findings appear in your transcript as compact cards; rejected ones stay out of the transcript and are listed in the review feed (`/pair-feed`). Click a card (fullscreen mode) or press `ctrl+o` to expand it. -| Command | Action | -| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `/pair-programmer` | Toggle reviews. | -| `/pair-clear` | Cancel reviews and clear all findings. | -| `/pair-feed` or `alt+r` | Toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first. Each review is one row with its file, reviewer and verdict counts; click it (fullscreen mode) to show its duration, decided findings and the agent's reasons. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. | -| `/pair-stats` | Open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `h` for hotspots (files ranked by accepted findings on the selected branch), `o` to return to the overview, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. | +| Command | Action | +| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `/pair-programmer` | Toggle reviews. | +| `/pair-clear` | Cancel reviews and clear all findings. | +| `/pair-feed` or `alt+r` | Toggle a live review sidebar on the right. It lists running reviews and reviews with accepted or rejected findings, newest first. Each review is one row with its file, reviewer and verdict counts; click it (fullscreen mode) to show its duration, decided findings and the agent's reasons. Reviews with no findings, undecided findings, failures and superseded reviews are left out. It never takes keyboard focus, hides in terminals narrower than 90 columns, and holds up to 100 reviews for the current session; `/pair-clear` empties it. On OMP, `/pair-feed` opens the same feed on demand (the **f** tab of `/pair-stats`), since OMP overlays would capture the keyboard. | +| `/pair-stats` | Open a themed session overview with review activity, findings, and estimated cost. Press `d` for detailed token and model accounting, `h` for hotspots (files ranked by accepted findings on the selected branch), `f` for the review feed, `o` to return to the overview, `r` to refresh the snapshot, arrows or Page Up/Down to scroll, and Esc, Enter, or `q` to close. Missing usage stays unknown; review usage covers the session across branches, while findings reflect the selected branch. | The default reviewer uses your current model and asks: _“Does it add entropy?”_ To change the prompt, model, or files reviewed, create `pair-programmer.reviewers.json` in your working directory: diff --git a/src/index.ts b/src/index.ts index 2201872..ed2c04d 100644 --- a/src/index.ts +++ b/src/index.ts @@ -29,13 +29,14 @@ import { import { StatsView, statsHotspots, + type Tab, statsLines, statsSummary, } from "./stats-view.js"; import { StatusOverlay, type StatusTone } from "./status-overlay.js"; import { ReviewFeed } from "./review-feed.js"; -import { ReviewSidebar } from "./review-sidebar.js"; -import { hostOf, type Host } from "./host.js"; +import { feedLines, ReviewSidebar } from "./review-sidebar.js"; +import { canFloat, hostOf, type Host } from "./host.js"; import { isInherited, reviewFile, @@ -1006,41 +1007,50 @@ export default function pairProgrammer(pi: ExtensionAPI): void { }, }); - const toggleSidebar = (ctx: ExtensionContext): void => { + const openStats = (ctx: ExtensionContext, tab?: Tab): Promise => { checkReset?.(); + return statsView.open( + ctx, + () => { + const snapshot = accounting.stats.snapshot(); + return { + summary: statsSummary(snapshot, store), + details: statsLines(snapshot, store), + hotspots: (width: number) => statsHotspots(store, width), + feed: (width: number) => + feedLines(feed.list(), (id) => store.lookup(id), width), + }; + }, + tab, + ); + }; + // Hosts without non-capturing overlays (OMP) get the feed on demand instead. + const toggleFeed = (ctx: ExtensionContext): Promise => { + checkReset?.(); + if (!sidebar.open && !canFloat(ctx)) return openStats(ctx, "feed"); sidebar.toggle(ctx); + return Promise.resolve(); }; // OMP may not expose message renderers; its default rendering stays readable. if (typeof pi.registerMessageRenderer === "function") pi.registerMessageRenderer(ACCEPTED_MESSAGE, renderAccepted); pi.registerCommand("pair-feed", { - description: "Toggle the live review sidebar", - handler: (_args, ctx) => { - toggleSidebar(ctx); - return Promise.resolve(); - }, + description: "Toggle the live review sidebar (a feed view on OMP)", + handler: (_args, ctx) => toggleFeed(ctx), }); // OMP may not expose shortcuts; the command remains the portable toggle. if (typeof pi.registerShortcut === "function") pi.registerShortcut("alt+r", { description: "Toggle the Pair Programmer review sidebar", - handler: toggleSidebar, + handler: (ctx) => { + void toggleFeed(ctx); + }, }); pi.registerCommand("pair-stats", { description: "Show on-demand Pair Programmer activity, findings and extension usage", - handler: (_args, ctx) => { - checkReset?.(); - return statsView.open(ctx, () => { - const snapshot = accounting.stats.snapshot(); - return { - summary: statsSummary(snapshot, store), - details: statsLines(snapshot, store), - hotspots: (width: number) => statsHotspots(store, width), - }; - }); - }, + handler: (_args, ctx) => openStats(ctx), }); } diff --git a/src/review-sidebar.ts b/src/review-sidebar.ts index ffa3779..ab9b172 100644 --- a/src/review-sidebar.ts +++ b/src/review-sidebar.ts @@ -55,6 +55,19 @@ function decided(entry: FeedEntry, lookup: Lookup): Decided[] { }); } +/** Running reviews and reviews with decided findings; the rest are noise. */ +function surfaced( + entries: readonly FeedEntry[], + lookup: Lookup, +): { entry: FeedEntry; findings: Decided[] }[] { + return entries.flatMap((entry) => { + const findings = decided(entry, lookup); + return entry.phase === "running" || findings.length > 0 + ? [{ entry, findings }] + : []; + }); +} + function summary(findings: readonly Decided[]): { icon: string; color: Color; @@ -146,12 +159,7 @@ export function layoutSidebar( now = Date.now(), expanded: ReadonlySet = new Set(), ): SidebarLayout { - const visible = entries.flatMap((entry) => { - const findings = decided(entry, lookup); - return entry.phase === "running" || findings.length > 0 - ? [{ entry, findings }] - : []; - }); + const visible = surfaced(entries, lookup); const box = frame(theme, width, 1); const inner = box.inner; const running = entries.filter((entry) => entry.phase === "running").length; @@ -214,6 +222,27 @@ export function layoutSidebar( }; } +const PLAIN: Painter = { fg: (_color, text) => text, bold: (text) => text }; + +/** Unframed, fully expanded feed rows for on-demand views such as OMP's. */ +export function feedLines( + entries: readonly FeedEntry[], + lookup: Lookup, + width: number, + now = Date.now(), +): string[] { + const lines = surfaced(entries, lookup).flatMap(({ entry, findings }) => [ + "", + ...entryLines(entry, findings, true, PLAIN, width, now), + ]); + return lines.length === 0 + ? [ + "Nothing to show yet.", + "Running reviews and decided findings appear here.", + ] + : lines.slice(1); +} + /** Renders the framed sidebar at an exact width, at most `height` rows. */ export function renderSidebar( ...args: Parameters diff --git a/src/stats-view.ts b/src/stats-view.ts index 7f34c6f..8f45a2c 100644 --- a/src/stats-view.ts +++ b/src/stats-view.ts @@ -150,9 +150,10 @@ export interface StatsReport { summary: readonly string[]; details: readonly string[]; hotspots: (width: number) => readonly string[]; + feed: (width: number) => readonly string[]; } -type Tab = "overview" | "details" | "hotspots"; +export type Tab = "overview" | "details" | "hotspots" | "feed"; type TabSpec = readonly [key: string, tab: Tab, subtitle: string]; const OVERVIEW: TabSpec = ["o", "overview", "Session overview"]; /** Tabs in hint order. */ @@ -160,6 +161,7 @@ const TABS: readonly TabSpec[] = [ OVERVIEW, ["d", "details", "Detailed accounting"], ["h", "hotspots", "Hotspots · accepted findings by file"], + ["f", "feed", "Review feed · recent reviews, newest first"], ]; const TAB_BY_KEY = new Map(TABS.map((spec) => [spec[0], spec])); @@ -171,7 +173,11 @@ export class StatsView { this.closeCurrent = undefined; } - async open(ctx: ExtensionContext, read: () => StatsReport): Promise { + async open( + ctx: ExtensionContext, + read: () => StatsReport, + initial: Tab = "overview", + ): Promise { this.close(); const unavailable = "/pair-stats requires an interactive terminal (TUI)."; if (!ctx.hasUI) throw new Error(unavailable); @@ -188,7 +194,7 @@ export class StatsView { (tui, theme, keys, done) => { state.mounted = true; let report = read(); - let spec = OVERVIEW; + let spec = TABS.find(([, name]) => name === initial) ?? OVERVIEW; let offset = 0; let pageSize = 1; let disposed = false; @@ -205,8 +211,19 @@ export class StatsView { const inner = box?.inner ?? columns; const tab = spec[1]; let lines = report.summary; - if (tab === "details") lines = report.details; - else if (tab === "hotspots") lines = report.hotspots(inner); + switch (tab) { + case "details": + lines = report.details; + break; + case "hotspots": + lines = report.hotspots(inner); + break; + case "feed": + lines = report.feed(inner); + break; + case "overview": + break; + } const wrapped = lines.flatMap((line) => { let tone: "warning" | "accent" | "text" = "text"; if (line.startsWith("!")) tone = "warning"; diff --git a/test/extension.test.ts b/test/extension.test.ts index 79a8e84..2d75a9c 100644 --- a/test/extension.test.ts +++ b/test/extension.test.ts @@ -2811,6 +2811,61 @@ it("feeds each review's outcome and attributed findings into the toggled sidebar await environment.emit("session_shutdown"); }); +it("gives OMP a widget badge and an on-demand review feed instead of floating overlays", async () => { + const environment = await setup("omp"); + const file = path.join(environment.cwd, "change.ts"); + await fsPromises.writeFile(file, "export const broken = true;\n"); + expect(environment.custom).not.toHaveBeenCalled(); + expect(environment.status()).toBe("◆ pair · watching"); + vi.mocked(reviewFile).mockResolvedValue([ + { line: 1, title: "Feed issue", quote: "broken", evidence: "Why" }, + ]); + await environment.emit("tool_result", { + toolName: "write", + input: { path: file }, + isError: false, + }); + await advanceReviews(() => { + expect(findings(environment.entries)).toHaveLength(2); + }); + await environment.emit("turn_end"); + for (const [index, finding] of findings(environment.entries).entries()) + await environment.decide(finding.id, { + findingId: finding.id, + decision: index === 0 ? "accept" : "reject", + reason: index === 0 ? "Real issue" : "Intentional", + }); + expect(environment.custom).not.toHaveBeenCalled(); + let view = ""; + environment.custom.mockImplementation(async (factory) => { + const component = await factory( + { + requestRender: vi.fn(), + terminal: { rows: 60 }, + } as unknown as Parameters[0], + { + fg: (_color: string, text: string) => text, + bold: (text: string) => text, + } as unknown as Parameters[1], + { matches: () => false } as unknown as Parameters[2], + vi.fn(), + ); + view = component.render(78).join("\n"); + }); + await environment.command("pair-feed"); + expect(environment.custom).toHaveBeenCalledTimes(1); + expect(view).toContain("Review feed"); + expect(view).toContain("change.ts"); + expect(view).toContain("Feed issue :1"); + expect(view).toContain("1 accepted"); + expect(view).toContain("“Intentional”"); + expect(environment.notify).not.toHaveBeenCalledWith( + expect.stringContaining("terminal UI"), + "warning", + ); + await environment.emit("session_shutdown"); +}); + it("omits the reviewer name from cards for findings restored from an earlier process", async () => { const environment = await setup(); const restored: Finding = { diff --git a/test/review-sidebar.test.ts b/test/review-sidebar.test.ts index 8fdbd00..d828941 100644 --- a/test/review-sidebar.test.ts +++ b/test/review-sidebar.test.ts @@ -3,6 +3,7 @@ import { afterEach, describe, expect, it, vi } from "vitest"; import { ReviewFeed } from "../src/review-feed.js"; import { fakeHost, flush, openCapturing } from "./fake-host.js"; import { + feedLines, MIN_COLUMNS, renderSidebar, ReviewSidebar, @@ -354,3 +355,28 @@ describe("ReviewSidebar", () => { sidebar.dispose(); }); }); + +describe("feedLines", () => { + it("lists surfaced reviews fully expanded, without a frame", () => { + expect(feedLines([], lookup, 40)).toEqual([ + "Nothing to show yet.", + "Running reviews and decided findings appear here.", + ]); + const feed = new ReviewFeed(); + feed.start("a", { ...job, file: "a.ts" }, 0); + feed.finish("a", "success", 1); + feed.attach("a", ["accepted", "rejected"]); + feed.start("b", { ...job, file: "b.ts" }, 0); + feed.finish("b", "success", 1); + feed.start("c", { ...job, file: "c.ts" }, 0); + const lines = feedLines(feed.list(), lookup, 50, 1000); + const joined = lines.join("\n"); + expect(lines[0]).toContain("c.ts"); + expect(lines[1]).toBe(""); + expect(joined).toContain("Duplicate helper :42"); + expect(joined).toContain("Rejected :2"); + expect(joined).not.toContain("b.ts"); + expect(joined).not.toContain("│"); + expect(feedLines(feed.list(), lookup, 50)[0]).toContain("c.ts"); + }); +}); diff --git a/test/stats-view.test.ts b/test/stats-view.test.ts index b62d5fd..a7d27df 100644 --- a/test/stats-view.test.ts +++ b/test/stats-view.test.ts @@ -9,6 +9,7 @@ import { statsHotspots, statsSummary, type StatsReport, + type Tab, } from "../src/stats-view.js"; const report = @@ -17,6 +18,7 @@ const report = summary: lines, details: [`detail ${lines.join(" ")}`], hotspots: (width: number) => [`hot ${String(width)}`], + feed: (width: number) => [`feed ${String(width)}`], }); type Factory = Parameters[0]; @@ -259,12 +261,12 @@ describe("StatsView", () => { "USAGE /", "now", "", - "↑↓ d h r\u{1B}[0m…\u{1B}[0m", + "↑↓ d h f \u{1B}[0m…\u{1B}[0m", ]); const framed = component.render(24); expect(framed[0]).toBe(`╭${"─".repeat(22)}╮`); expect(framed[4]).toBe(`│ Review counts${" ".repeat(5)} │`); - expect(framed.at(-2)).toContain("↑↓ d h r q clo"); + expect(framed.at(-2)).toContain("↑↓ d h f r q c"); expect(framed.at(-1)).toBe(`╰${"─".repeat(22)}╯`); component.invalidate(); component.handleInput?.(key); @@ -274,6 +276,31 @@ describe("StatsView", () => { component.dispose?.(); }); + it("opens on a requested tab and toggles back to the overview", async () => { + const host = ui(40); + const view = new StatsView(); + const opened = view.open(host.ctx, report(["Summary"]), "feed"); + await host.mounted; + const component = host.component(); + expect(component.render(80).join("\n")).toContain("feed 74"); + expect(component.render(80).join("\n")).toContain("Review feed"); + component.handleInput?.("f"); + expect(component.render(80).join("\n")).toContain("Summary"); + component.handleInput?.("q"); + await opened; + // Runtime (untyped) callers asking for an unknown tab get the overview. + const other = ui(40); + const fallback = view.open( + other.ctx, + report(["Summary"]), + "unknown" as Tab, + ); + await other.mounted; + expect(other.component().render(80).join("\n")).toContain("Summary"); + view.close(); + await fallback; + }); + it("scrolls within snapshot bounds and closes on session disposal", async () => { const host = ui(); const view = new StatsView(); @@ -287,7 +314,7 @@ describe("StatsView", () => { const first = (): string | undefined => component.render(80)[4]; expect(first()).toContain("Row 0"); expect(component.render(80).at(-2)).toContain( - "d details h hotspots r refresh esc 1–9/12", + "d details h hotspots f feed r refresh esc 1–9/12", ); component.handleInput?.("\u{1B}[B"); expect(first()).toContain("Row 1"); @@ -326,6 +353,7 @@ describe("StatsView", () => { summary: [value], details: [value], hotspots: () => [value], + feed: () => [value], })); const view = new StatsView(); const opened = view.open(host.ctx, read);