From 3837e9b28baa356c6a42a95e777461399231c208 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Elberte=20Pl=C3=ADnio?= Date: Mon, 27 Jul 2026 10:11:34 -0300 Subject: [PATCH] feat(chat): render pi-kit lane cards inside a pickforge-lanes MCP row MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #362 PR 3, the last slice of that issue. When an agent dispatched lanes and waited, the chat showed nothing about which lanes existed or how they were doing, while the Settings panel had exactly that information a screen away. **One card, not two.** `RunCard` is extracted out of `PiKitLanesPanel` into `PiKitRunCard.tsx` and rendered by both surfaces. The issue was explicit about this: a second lane card would drift from the first the moment either changed. The extraction is behaviour-neutral — the panel's own tests pass untouched. **Correlation is a pure function.** `pikitRunRef` reads the run id out of the row's argument summary, which the parser already attaches: `lanes_wait` and `lanes_status` take a `run` argument, and `lanes_spawn` names its run only in the result. So an in-flight spawn correctly resolves to nothing rather than guessing. Matching `run-` rather than any token keeps a stray word in a result from being read as a run id. **Live in flight, frozen after** (decided 2026-07-27). Polling while the call runs is what makes `lanes_wait` worth watching; freezing afterwards keeps a replayed row from quietly rewriting itself from a run that has since moved on or been pruned. A finished run whose status is gone says so plainly instead of rendering an empty box. Polling is scoped to a mounted, open row — `Disclosure` keeps closed bodies out of the DOM, so a transcript full of old lanes rows costs nothing — and `startPiKitLanesPolling` is idempotent, so it will not double the interval or stop the Settings panel's own poll. **Abandon is inert in a transcript.** Acting on a run belongs in the panel that owns it; a destructive action two clicks from a replayed message is a trap. Refs #362 --- docs/releases/UNRELEASED.md | 6 + src/components/chat/McpCard.tsx | 27 ++++- src/components/pikit/PiKitLanesPanel.tsx | 137 +-------------------- src/components/pikit/PiKitRunCard.tsx | 147 +++++++++++++++++++++++ src/components/pikit/PiKitRunLanes.tsx | 55 +++++++++ src/components/pikit/pikitLanes.css | 9 ++ src/lib/pikitRunRef.ts | 46 +++++++ tests/unit/PiKitRunLanes.test.tsx | 127 ++++++++++++++++++++ tests/unit/pikitRunRef.test.ts | 61 ++++++++++ 9 files changed, 476 insertions(+), 139 deletions(-) create mode 100644 src/components/pikit/PiKitRunCard.tsx create mode 100644 src/components/pikit/PiKitRunLanes.tsx create mode 100644 src/lib/pikitRunRef.ts create mode 100644 tests/unit/PiKitRunLanes.test.tsx create mode 100644 tests/unit/pikitRunRef.test.ts diff --git a/docs/releases/UNRELEASED.md b/docs/releases/UNRELEASED.md index c85c594c..f94a2ad4 100644 --- a/docs/releases/UNRELEASED.md +++ b/docs/releases/UNRELEASED.md @@ -113,6 +113,12 @@ reset this file. permissions are still bypassed only when the user explicitly selects bypass mode. - Removed the `piAgents` flag after Pi native chat shipped default-on in v0.2.0. +- A `pickforge-lanes` MCP call in chat now expands to the run's lane cards — + the same cards the Settings panel shows, not a second copy (#362 PR 3, behind + the default-off `mcpToolDetail` flag). Lanes are live while the call is in + flight, so watching a `lanes_wait` is worth something, and frozen once it + finishes so a replayed message does not rewrite itself from a run that has + moved on. Abandon stays in Settings. - The working row now names what is running and how long it has been running, behind the new default-off `turnActivity` flag (#365). During a long MCP call the chat used to show a bare "Working" dot with no way to tell thinking from diff --git a/src/components/chat/McpCard.tsx b/src/components/chat/McpCard.tsx index 57a40730..616e90ea 100644 --- a/src/components/chat/McpCard.tsx +++ b/src/components/chat/McpCard.tsx @@ -3,6 +3,8 @@ import { compactInline, hasHiddenDetail } from "../../lib/chatDisplay"; import { IconChevronRight } from "../icons"; import type { ToolCallStatus } from "../../stores/agentChat"; import { Disclosure, StatusPill, type StatusIntent } from "../ui"; +import { pikitRowIsLive, pikitRunRef } from "../../lib/pikitRunRef"; +import { PiKitRunLanes } from "../pikit/PiKitRunLanes"; import "./chat.css"; const STATUS_INTENT: Record = { @@ -23,7 +25,15 @@ export function McpCard(props: { const open = () => props.open ?? localOpen(); const toggle = () => (props.onToggle ? props.onToggle() : setLocalOpen((v) => !v)); const detail = () => props.detail ?? ""; - const canExpand = () => hasHiddenDetail(detail(), 120); + const row = () => ({ + server: props.server, + tool: props.tool, + detail: props.detail, + status: props.status, + }); + const runRef = () => pikitRunRef(row()); + const live = () => pikitRowIsLive(row()); + const canExpand = () => hasHiddenDetail(detail(), 120) || runRef() !== null; return (
@@ -64,11 +74,18 @@ export function McpCard(props: { - -
+
+ {/* A pickforge-lanes call renders the run's lanes the way Settings + does — same component, so the two cannot drift (#362). Live while + the call is in flight, frozen afterwards, so a replayed row does + not quietly rewrite itself from a run that has moved on. */} + + {(run) => } + + 
{detail()}
-
- + +
); diff --git a/src/components/pikit/PiKitLanesPanel.tsx b/src/components/pikit/PiKitLanesPanel.tsx index b2cffe94..64f212a3 100644 --- a/src/components/pikit/PiKitLanesPanel.tsx +++ b/src/components/pikit/PiKitLanesPanel.tsx @@ -6,8 +6,9 @@ // token usage. import { For, Show, type JSX, createSignal, onCleanup, onMount } from "solid-js"; import { ConfirmDialog } from "../ConfirmDialog"; -import { Disclosure, ForgeEmptyState, MonoEyebrow } from "../ui"; -import { IconChevronRight, IconGrid, IconRefresh } from "../icons"; +import { RunCard, type AbandonTarget } from "./PiKitRunCard"; +import { ForgeEmptyState, MonoEyebrow } from "../ui"; +import { IconGrid, IconRefresh } from "../icons"; import type { PiKitRunEntry } from "../../lib/process"; import { loadAllPiKitRuns, @@ -20,145 +21,13 @@ import { startPiKitLanesPolling, stopPiKitLanesPolling, } from "../../stores/pikitLanes"; -import { - abandonDisabledReason, - abandonHint, - formatCost, - formatDuration, - formatTokens, - laneDetail, - laneStatusTone, - orphanNote, - runLabel, - runStatusTone, -} from "./pikitLaneDisplay"; import "./pikitLanes.css"; -interface AbandonTarget { - run: string; - lane: string | null; - label: string; -} - interface Notice { text: string; error: boolean; } -// eslint-disable-next-line max-lines-per-function -- single cohesive panel; see codebase-design note in PR. -function RunCard(props: { - entry: PiKitRunEntry; - onAbandon: (target: AbandonTarget) => void; -}): JSX.Element { - const [open, setOpen] = createSignal(false); - const entry = () => props.entry; - const status = () => entry().status; - const runAbandonReason = () => abandonDisabledReason(entry()); - const runAbandonHint = () => abandonHint(entry()); - - return ( -
- - -
- No status details available.
}> - {(s) => ( -
- - {(lane) => { - const reason = () => abandonDisabledReason(entry(), lane); - const hint = () => abandonHint(entry()); - return ( -
- - - {lane.state} - - - {lane.lane} - - {lane.model} · {lane.effort} · {formatTokens(lane.tokensIn)}/ - {formatTokens(lane.tokensOut)} tok · {formatCost(lane.cost)} ·{" "} - {formatDuration(lane.durationMs)} - - {laneDetail(lane)} - - -
- ); - }} - -
- )} - - - {(note) =>
{note()}
} - - 0}> - - - - -
- ); -} - /** The complete run list, behind the panel's "view all" affordance. Reuses * `ConfirmDialog`'s portal/backdrop shape rather than introducing a second * modal idiom (#363). */ diff --git a/src/components/pikit/PiKitRunCard.tsx b/src/components/pikit/PiKitRunCard.tsx new file mode 100644 index 00000000..808a27e2 --- /dev/null +++ b/src/components/pikit/PiKitRunCard.tsx @@ -0,0 +1,147 @@ +// One pi-kit run, rendered identically in Settings and in an agent chat's MCP +// row (#362). Extracted out of PiKitLanesPanel rather than written twice — +// a second lane card would drift from this one the first time either changed. +import { For, Show, type JSX, createSignal } from "solid-js"; +import { Disclosure } from "../ui"; +import { IconChevronRight, IconGrid } from "../icons"; +import type { PiKitRunEntry } from "../../lib/process"; +import { + abandonDisabledReason, + abandonHint, + formatCost, + formatDuration, + formatTokens, + laneDetail, + laneStatusTone, + orphanNote, + runLabel, + runStatusTone, +} from "./pikitLaneDisplay"; +import "./pikitLanes.css"; + +export interface AbandonTarget { + run: string; + lane: string | null; + label: string; +} + +// eslint-disable-next-line max-lines-per-function -- one cohesive card; splitting it would spread the lane row across files. +export function RunCard(props: { + entry: PiKitRunEntry; + onAbandon: (target: AbandonTarget) => void; + /** Hides the Abandon controls. Acting on a run belongs in the Settings panel + * that owns it — offering a destructive action two clicks from a replayed + * chat message would be a trap (#362). */ + readOnly?: boolean; +}): JSX.Element { + const [open, setOpen] = createSignal(false); + const entry = () => props.entry; + const status = () => entry().status; + const runAbandonReason = () => abandonDisabledReason(entry()); + const runAbandonHint = () => abandonHint(entry()); + + return ( +
+ + +
+ No status details available.
}> + {(s) => ( +
+ + {(lane) => { + const reason = () => abandonDisabledReason(entry(), lane); + const hint = () => abandonHint(entry()); + return ( +
+ + + {lane.state} + + + {lane.lane} + + {lane.model} · {lane.effort} · {formatTokens(lane.tokensIn)}/ + {formatTokens(lane.tokensOut)} tok · {formatCost(lane.cost)} ·{" "} + {formatDuration(lane.durationMs)} + + {laneDetail(lane)} + + + + +
+ ); + }} + +
+ )} + + + {(note) =>
{note()}
} + + 0}> + + + + +
+ ); +} + diff --git a/src/components/pikit/PiKitRunLanes.tsx b/src/components/pikit/PiKitRunLanes.tsx new file mode 100644 index 00000000..592a2f26 --- /dev/null +++ b/src/components/pikit/PiKitRunLanes.tsx @@ -0,0 +1,55 @@ +// One pi-kit run's lanes, rendered inside an agent chat's MCP row (#362). +// +// Renders the SAME `RunCard` Settings uses, so the two surfaces cannot drift. +// What differs is the lifecycle, not the rendering: +// +// live — the MCP call is in flight, so this polls while the row is open. +// That is what makes watching a `lanes_wait` worth anything. +// frozen — the call has finished, so it shows whatever the store already +// knows and never polls. A replayed row must not rewrite itself from +// a run that has since moved on, or blank out because the run was +// pruned. +// +// Polling is scoped to a MOUNTED, OPEN row: the card only mounts when its +// disclosure opens (`Disclosure` keeps closed bodies out of the DOM), so a +// transcript full of old lanes rows costs nothing. +import { Show, type JSX, onCleanup, onMount } from "solid-js"; +import { RunCard } from "./PiKitRunCard"; +import { + pikitRuns, + startPiKitLanesPolling, + stopPiKitLanesPolling, +} from "../../stores/pikitLanes"; + +export function PiKitRunLanes(props: { run: string; live: boolean }): JSX.Element { + onMount(() => { + if (!props.live) return; + // Idempotent: the Settings panel may already be polling, and this must not + // double the interval or stop it out from under that panel on cleanup — + // `startPiKitLanesPolling` no-ops when a poll is already running. + startPiKitLanesPolling(); + onCleanup(() => stopPiKitLanesPolling()); + }); + + const entry = () => pikitRuns().find((candidate) => candidate.run === props.run) ?? null; + + return ( + + {props.live ? "Waiting for lane status…" : `No lane status retained for ${props.run}.`} + + } + > + {(found) => ( +
+ {/* Abandon is deliberately inert here: acting on a run belongs in the + Settings panel that owns it, and offering it mid-transcript would + be a destructive action two clicks from a replayed message. */} + undefined} readOnly /> +
+ )} + + ); +} diff --git a/src/components/pikit/pikitLanes.css b/src/components/pikit/pikitLanes.css index 7761a036..fedbf9ad 100644 --- a/src/components/pikit/pikitLanes.css +++ b/src/components/pikit/pikitLanes.css @@ -236,3 +236,12 @@ overflow-y: auto; min-height: 0; } + +/* A run's lanes rendered inside a chat's MCP row (#362). The card keeps its own + frame; this just stops it inheriting the chat body's monospace tail styling + and gives it room from the argument summary below it. */ +.pf-pikit-inline { + display: flex; + flex-direction: column; + margin-bottom: var(--pf-space-sm); +} diff --git a/src/lib/pikitRunRef.ts b/src/lib/pikitRunRef.ts new file mode 100644 index 00000000..b39c098d --- /dev/null +++ b/src/lib/pikitRunRef.ts @@ -0,0 +1,46 @@ +// Correlating a `pickforge-lanes` MCP row to the pi-kit run it is about (#362). +// +// The row already carries everything needed — the parser attaches a compact +// summary of the tool input, and `lanes_wait`/`lanes_status` take a `run` +// argument. `lanes_spawn` has no run to name going in; its run id only exists +// in the result, which the same summary carries once the call completes. +// +// Kept as a pure function so the correlation is testable without a store, a +// timer, or a chat. + +/** The MCP server whose calls render lane cards. */ +export const PIKIT_LANES_SERVER = "pickforge-lanes"; + +/** pi-kit run ids are `run--`; matching that shape + * rather than "any token" keeps a stray word in a result from being read as a + * run id. */ +const RUN_ID = /\brun-[0-9A-Za-z]+(?:-[0-9A-Za-z]+)*\b/; + +export interface PiKitRowLike { + server: string; + tool: string; + detail?: string | null; + status?: "inProgress" | "completed" | "failed"; +} + +/** The run a `pickforge-lanes` row is about, or null when the row is not one + * of ours or has not named a run yet. + * + * A `lanes_spawn` row names no run until its result lands, so an in-flight + * spawn correctly resolves to null rather than guessing. */ +export function pikitRunRef(row: PiKitRowLike): string | null { + if (row.server !== PIKIT_LANES_SERVER) return null; + const detail = row.detail?.trim(); + if (!detail) return null; + return RUN_ID.exec(detail)?.[0] ?? null; +} + +/** Whether this row should poll for live lane state, or render the snapshot it + * captured when the call finished. + * + * Live while the call is in flight — that is what makes `lanes_wait` worth + * watching. Frozen once it completes, because a replayed row must not quietly + * rewrite itself from a run that has since moved on or been pruned. */ +export function pikitRowIsLive(row: PiKitRowLike): boolean { + return pikitRunRef(row) !== null && row.status === "inProgress"; +} diff --git a/tests/unit/PiKitRunLanes.test.tsx b/tests/unit/PiKitRunLanes.test.tsx new file mode 100644 index 00000000..5e2427a1 --- /dev/null +++ b/tests/unit/PiKitRunLanes.test.tsx @@ -0,0 +1,127 @@ +// @vitest-environment jsdom +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { render } from "solid-js/web"; +import type { PiKitRunEntry } from "../../src/lib/process"; + +const testEnv = vi.hoisted(() => ({ invoke: vi.fn() })); +vi.mock("@tauri-apps/api/core", () => ({ invoke: testEnv.invoke })); + +const RUN = "run-20260726T101112-4821"; + +const ENTRY: PiKitRunEntry = { + run: RUN, + supported: true, + orphaned: false, + status: { + schemaVersion: 1, + revision: 3, + updatedAtMs: 1_000, + run: RUN, + state: "active", + durationMs: 5_000, + totals: { cost: 0.1, tokensIn: 100, tokensOut: 50 }, + lanes: [ + { + lane: "lane-1", + model: "openai-codex/gpt-5.6-sol", + effort: "medium", + mode: "read-only", + state: "running", + tokensIn: 100, + tokensOut: 50, + cost: 0.1, + context: 1200, + }, + ], + }, +}; + +let root: HTMLDivElement; +let dispose: (() => void) | undefined; + +beforeEach(() => { + testEnv.invoke.mockReset(); + vi.resetModules(); + vi.useFakeTimers(); + root = document.createElement("div"); + document.body.appendChild(root); +}); + +afterEach(() => { + dispose?.(); + dispose = undefined; + root.remove(); + vi.useRealTimers(); +}); + +async function mount(live: boolean) { + const { PiKitRunLanes } = await import("../../src/components/pikit/PiKitRunLanes"); + dispose = render(() => , root); + return root; +} + +function pollCount() { + return testEnv.invoke.mock.calls.filter((call) => String(call[0]).startsWith("list_pi_kit_run")).length; +} + +describe("PiKitRunLanes — live in flight, frozen after (#362)", () => { + it("polls while the call is in flight", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + expect(pollCount()).toBeGreaterThan(0); + expect(root.querySelector(".pf-pikit-card")).not.toBeNull(); + + const before = pollCount(); + await vi.advanceTimersByTimeAsync(4_000); + expect(pollCount()).toBeGreaterThan(before); + }); + + it("never polls once the call has finished", async () => { + // A replayed row must not rewrite itself from a run that has moved on. + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(false); + await vi.advanceTimersByTimeAsync(10_000); + + expect(pollCount()).toBe(0); + }); + + it("renders the same card component Settings uses", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + // The Settings panel's own selectors — proof it is one component, not a + // second lane card that will drift. + expect(root.querySelector(".pf-pikit-card")).not.toBeNull(); + expect(root.querySelector(".pf-pikit-summary")).not.toBeNull(); + }); + + it("offers no Abandon control in a transcript", async () => { + testEnv.invoke.mockResolvedValue({ runs: [ENTRY], total: 1 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + root.querySelector(".pf-pikit-summary")!.click(); + + expect(root.querySelector(".pf-pikit-abandon")).toBeNull(); + expect(root.textContent).not.toContain("Abandon all lanes"); + }); + + it("says so plainly when a finished run's status is gone", async () => { + testEnv.invoke.mockResolvedValue({ runs: [], total: 0 }); + await mount(false); + await vi.advanceTimersByTimeAsync(0); + + expect(root.textContent).toContain("No lane status retained"); + expect(root.textContent).toContain(RUN); + }); + + it("shows a waiting note rather than an empty box while in flight", async () => { + testEnv.invoke.mockResolvedValue({ runs: [], total: 0 }); + await mount(true); + await vi.advanceTimersByTimeAsync(0); + + expect(root.textContent).toContain("Waiting for lane status"); + }); +}); diff --git a/tests/unit/pikitRunRef.test.ts b/tests/unit/pikitRunRef.test.ts new file mode 100644 index 00000000..dc4d21ee --- /dev/null +++ b/tests/unit/pikitRunRef.test.ts @@ -0,0 +1,61 @@ +import { describe, expect, it } from "vitest"; +import { pikitRowIsLive, pikitRunRef } from "../../src/lib/pikitRunRef"; + +const row = (over: Partial[0]> = {}) => ({ + server: "pickforge-lanes", + tool: "lanes_wait", + detail: "run: run-20260726T101112-4821", + status: "inProgress" as const, + ...over, +}); + +describe("pikitRunRef — correlating an MCP row to its pi-kit run (#362)", () => { + it("reads the run from a lanes_wait argument summary", () => { + expect(pikitRunRef(row())).toBe("run-20260726T101112-4821"); + }); + + it("reads the run out of a lanes_spawn result once it lands", () => { + // A spawn names no run going in; the id only exists in the result. + expect( + pikitRunRef(row({ tool: "lanes_spawn", detail: "started run-20260726T101112-4821 with 3 lanes" })), + ).toBe("run-20260726T101112-4821"); + }); + + it("resolves to nothing for an in-flight spawn that has not named a run", () => { + expect(pikitRunRef(row({ tool: "lanes_spawn", detail: "lanes: 3, model: sol" }))).toBeNull(); + }); + + it("ignores rows from any other MCP server", () => { + expect(pikitRunRef(row({ server: "github" }))).toBeNull(); + }); + + it("ignores a row with no detail at all", () => { + expect(pikitRunRef(row({ detail: null }))).toBeNull(); + expect(pikitRunRef(row({ detail: " " }))).toBeNull(); + }); + + it("does not mistake an ordinary word for a run id", () => { + expect(pikitRunRef(row({ detail: "waiting for the run to finish" }))).toBeNull(); + }); +}); + +describe("pikitRowIsLive — live in flight, frozen after (#362)", () => { + it("polls while the call is in flight", () => { + expect(pikitRowIsLive(row({ status: "inProgress" }))).toBe(true); + }); + + it("freezes once the call has completed or failed", () => { + // A replayed row must not rewrite itself from a run that has moved on. + expect(pikitRowIsLive(row({ status: "completed" }))).toBe(false); + expect(pikitRowIsLive(row({ status: "failed" }))).toBe(false); + }); + + it("never polls a row with no run to poll for", () => { + expect(pikitRowIsLive(row({ detail: null }))).toBe(false); + expect(pikitRowIsLive(row({ server: "github" }))).toBe(false); + }); + + it("does not poll a row whose status is unknown (pre-flag history)", () => { + expect(pikitRowIsLive(row({ status: undefined }))).toBe(false); + }); +});