feat(desktop): collapse consecutive tool steps into one tool-chain card

[baxen]

Slice C — tool-chain cards

Runs of consecutive tool items within a turn now collapse into a single tool-chain card that mutates in place as steps stream in, instead of leaving one row per tool call. A turn that read eight files and ran four commands was twelve rows of transcript; it is now one.

Behaviour

• Collapsed header — verb/object/outcome headline derived from the classifier's own per-step semantics (Read 4 files, Edited 3 files, Ran 6 commands, Sent 2 messages), an aggregate status glyph (spinner while any step executes, check when all clean, error mark if any failed), and elapsed/finished timing.
• Live run — reads as active with its position (Reviewing files · step 3) and is expanded by default so the reader watches work happen.
• On completion — the card collapses itself and hands the space back to the conversation, unless it contains an error, in which case it stays open with the failing step highlighted. A reader's own toggle overrides both rules from then on.
• Expanded body — one row per step reusing the existing AgentSessionToolItem/* rendering, so shell blocks, diffs, sent-message previews, and image previews all keep working. A lone tool item not in a run renders exactly as today.
• compactPreview keeps today's rendering: the plain, self-managed, collapsed Ran N tool calls row — no derived headline, no aggregate glyph, no timing, no disclosure policy. The variant decision lives in one boundary component (AgentSessionToolRunSegment: variant in, presentation out); the card itself is unconditionally controlled, so no piece of chrome has to remember to opt out of the preview.

One mechanism, not two

Per the brief, this replaces the existing "same-kind summary" grouping rather than layering a second mechanism on it. The old two-pass scheme collapsed same-kind runs, then wrapped leftovers in a "mixed burst" summary that could nest the same-kind ones — producing stacked redundant headlines (Ran 16 tool calls → Ran 12 commands). Grouping is now a single pass over maximal runs of eligible steps: one card, one level of steps, headline adapting to whether the run is homogeneous.

Removed: TranscriptToolRunSummary, TranscriptToolRunChildSegment, groupSameKindSegments, groupMixedToolRuns, isBurstParticipant, isGroupingEligible, sameKindKey, sameKindLabel, minimumSummaryRunLength, and the SameKindSummaryItem component (which recursed into nested summaries). The parallel bucket taxonomy added in the first revision (ToolRunBucket, dominantToolRunBucket, BUCKET_PHRASES, BUCKET_SALIENCE) is also gone — see below. Added: TranscriptToolRun + single-pass groupToolRunSegments, and a pure agentSessionToolRunSummary.ts driven by render classes rather than tool-name matching.

Headlines read the classifier instead of restating it

The first revision of this card carried its own bucket taxonomy (ToolRunBucket + BUCKET_PHRASES/BUCKET_SALIENCE) that re-derived, in parallel, vocabulary the activity classifier already produces for every step — two places to keep in sync and two places to be wrong. That taxonomy is gone. A run's kind is now the classifier's own {renderClass, tone}, verbs come from the step descriptors' descriptor.action.verb, and the summary module supplies only what a per-step descriptor structurally cannot: the collective noun (4 files), a per-render-class verb floor, and a tone fallback. Both maps are satisfies Record<...> so a new render class or tone cannot be added without deciding its phrasing. Live runs re-tense the classifier's closed past-tense vocabulary (Read → Reviewing, Ran → Running).

A failed step no longer erases what it was doing. classifyTool flattens failures to render class error, so one failed read used to drag an entire run's headline down to generic tool work. The summary re-classifies a failed step as if it had succeeded to recover its class, tone, and verb for headline purposes — and honestly keeps error if that is genuinely all the step was. The failure still surfaces through the aggregate glyph and the highlighted step row, which is where it belongs.

Two deliberate behaviour changes

1. Failed steps stay inside their run. The old grouping broke runs on isError, turning one stretch of work into three rows and stripping the failure of its context. A failure belongs to the run it happened in; the card surfaces it by staying open and highlighting it. This intentionally inverts a few existing test expectations, which are updated with the reasoning recorded at each site.
2. Runs are keyed on their FIRST step id (tool-run:<firstStepId>), so appending a streaming step never changes the id. A changed key remounts the card and drops the reader's disclosure state — this is the append-stability contract scroll anchoring depends on, and it has its own test.

Ineligible-and-run-breaking: raw-rail, suppressed, status, permission, thought. Those rows stay visible and split the run, so the ambient safety net and every intervention point keep their own row.

Eligibility is decided on a step's recovered class, not on the class its descriptor reports. classifyTool flattens every failed step to render class error while keeping its original groupKey, so reading the reported class let a failure launder an excluded class into an eligible one — a failed stop hook (error + suppressed:stop-hook) was admitted and the safety row vanished into a card. Both bug reviewers found this independently; fixed by routing eligibility through the same toolRunKind recovery the headline already used, so there is one recovery path rather than two that can disagree.

Grouping threshold changed 3 → 2. The old scheme had a per-class minimum run length (minimumSummaryRunLength: 2 for file-edit, 3 for everything else); TOOL_RUN_MINIMUM_STEPS is now a flat 2. Two consecutive reads that previously stayed as two separate rows now collapse into one card. That is intentional — the per-class split existed because the old summary row was a weak affordance that was not worth spending on a pair, and the card is not — but it is a visible change to how short runs read, so it should be reviewed as a decision rather than a detail.

The <details> echo trap — now a shared hook

<details> fires toggle for programmatic open changes as well as clicks, and the event carries no way to tell the two apart. So the card's own auto-expand echoes back an event that looks exactly like a reader opening it, and recording that as a reader choice pins the row to its first policy state forever — auto-collapse silently stops working. The discriminator is agreement: an echo always reports the state we just rendered, so only a toggle that disagrees can have come from the reader.

Quality found the same trap in Slice B's thought disclosure, so the guard is now a generic hook at src/shared/hooks/useControlledDisclosure.ts (matching where the repo already keeps shared UI hooks) rather than something private to this card. useToolRunDisclosure is a one-liner over it. Slice B should consume this hook instead of reinventing the guard — which is why C merges first.

jsdom does not emit the echo, so a test for this passes vacuously unless it injects the agreeing toggle itself. The hook's test drives a real controlled <details> and does exactly that; I confirmed it is non-vacuous by removing the guard (the echo test fails, the other four still pass) and restoring it.

ActivityRow's controlled disclosure is typed all-or-nothing — a discriminated union of {open, onOpenChange} or neither, not two independent optionals. A half-controlled <details> is always a bug: an open with no handler freezes the row, and a handler with no open never applies what it recorded. Verified the compiler rejects both half-controlled shapes.

Scope

Touches the transcript list, its grouping helper, ActivityRow, and one new shared hook (src/shared/hooks/useControlledDisclosure.ts, which Slice B will consume). No changes to the drawer/shell (Slice A) or message/thought/plan styling (Slice B).

Streaming cost

ToolRunStepRow is memoized on the step's identity. A run re-renders on every append while it streams and on every live-clock tick, and each step's presenter rebuilds compact tool summaries, parses diffs, and renders markdown/images — so an unmemoized row made a long run cost O(n) of that work per appended step (the transcript's own TranscriptItemView avoids it the same way). Transcript items are replaced rather than mutated, so reference equality on the item is a sound change test. Covered by a render-count test that counts at the presenter boundary rather than reaching into React internals, paired with a test that a step which really did change still re-renders, so the memo cannot be made vacuously strict.

Validation

At 74b90152 (full validation rerun after the latest revision):

• Desktop suite 5414/5414 pass — full pnpm test, not a scoped run
• tsc --noEmit clean; biome check clean (the four remaining repo findings are pre-existing on main, confirmed by stashing); file-size and px-text gates clean; all pre-push hooks green
• Tests: 32 unit tests for headline derivation / eligibility / aggregate status / timing, now built through the real classifyTool so tone and action values are production values rather than hand-written fixtures — including mixed relay-op runs (read vs write vs admin), an evenly-split run resolved by tone salience, and a failed step not dragging the headline to generic tool work
• 17 render tests for disclosure, header, and body behaviour, including 5 that render through the variant boundary under compactPreview to prove which presentation is picked
• 5 tests for the shared useControlledDisclosure hook, verified non-vacuous by removing the guard

🤖 Generated with Buzz

Bug-pass fixes (post-review)

Two bugs found by the independent bug pass, both fixed in 84eaf287:

1. [P1] Failed suppressed/status steps leaked into chains — described under "One mechanism, not two" above. Regression tests build the failed stop/postcompact items through the real classifier (new mkClassifiedTool helper) and assert the flattened class and preserved groupKey before asserting the row stays a standalone breaker, so the test still describes the trap if the classifier's behaviour changes. Confirmed non-vacuous: reverting the fix fails all three new tests.

2. [P2] Appending a step re-rendered every prior step — described under "Streaming cost" above. Confirmed non-vacuous: reverting the memo fails both new tests.

3. [P2] A live heterogeneous admin run read in the past tense — fixed in 74b90152. A run whose defining steps disagree on a verb falls back to its tone's verb, and the admin fallback "Changed" had no progressive form, so buzz channels create followed by an executing buzz channels delete headlined Changed Buzz relay ops · step 2 while still running.

   The root cause was the table's type, not the missing row: PROGRESSIVE_VERBS was Record<string, string>, so an absent verb was not a compile error but a silent fall-through to the past-tense form. The set of verbs this module can choose for a headline is now named RunVerb — the classifier's closed vocabulary plus the tone and render-class fallbacks — and the table is closed with satisfies Record<RunVerb, string>, so adding a verb or a new tone fallback fails to compile until its progressive form is decided. The remaining lookup goes through a named progressiveVerb helper documenting why the cast is safe the other way: a verb taken straight from a descriptor is only typed string, so an unrecognised one degrades to itself rather than to something wrong.

   Two tests: the reported mixed-admin live case, and one covering every tone's fallback. Note for reviewers — this fallback is only reached when the steps sharing the dominant kind disagree on a verb, which makes the fixtures easy to get wrong: a pair differing in render class instead narrows the dominant kind to a single step whose verb then trivially agrees, quietly testing nothing (my first write-tone pair had exactly that flaw and passed for the wrong reason). Each case now asserts its precondition — same kind, differing verbs — before asserting the headline. Confirmed non-vacuous both ways: removing the Changed entry fails tsc and both new tests.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 643b310690

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Gate running state on live session ownership

When an agent crashes or disconnects after emitting a tool start but before its terminal update, the archived item permanently retains executing or pending. This status-only check therefore marks the card as running whenever that history is reopened, even though AgentSessionTranscriptList already knows there is no matching active turn/live session; the card displays a spinner and ToolRunLiveElapsed keeps increasing from the original timestamp indefinitely. Gate this phase on current turn/session liveness, or terminalize orphaned steps so dead work is shown as stopped or timed out rather than actively running.

AGENTS.md reference: AGENTS.md:L13-L18

Useful? React with 👍 / 👎.

[baxen]

Latest revision and validation:

• Pushed 74b901527a8b8d8b1740693f55f64758237e89bb (origin matches).
• Full desktop pnpm test: 5414/5414 pass.
• pnpm typecheck: clean.
• pnpm check: clean for this change; the four reported Biome findings are pre-existing on main.
• pnpm check:file-sizes: clean; pre-push hooks green.
• PR CI for 74b901527: all applicable checks pass, including Desktop Core, Desktop Smoke E2E (1–4), Desktop Build, and Desktop E2E Integration. Inapplicable lanes are skipped.

The PR description now explicitly records the intentional flat threshold change from 3 to 2 and that compactPreview uses the plain legacy Ran N tool calls rendering without chain-card chrome or disclosure policy. One independent P2 review thread remains open about orphaned executing/pending history; it needs a liveness ownership decision rather than a headline-table change.

@ss-core-02

[baxen]

Validation complete at 74b901527a8b8d8b1740693f55f64758237e89bb:

• Full desktop pnpm test: 5414/5414 pass.
• pnpm typecheck: clean.
• pnpm check: clean for the changed files; the four repository findings are pre-existing on main.
• pnpm check:file-sizes: clean; pre-push hooks green.
• PR CI: all applicable checks pass, including Desktop Core, Desktop Smoke E2E (1–4), Desktop Build, Desktop E2E Relay, and Desktop E2E Integration. Inapplicable lanes are skipped.

The PR description now explicitly documents the intentional flat grouping threshold change from 3 to 2 and compactPreview's plain legacy Ran N tool calls presentation without chain-card chrome or disclosure policy.

One independent P2 review thread remains open about orphaned executing/pending history after a crashed or disconnected session. That is a separate liveness-ownership policy question; I have left it visible for review rather than silently changing the scope of the card implementation.

@ss-core-02