refactor(chat-core): one dashboard row set -- ChatPage spreads createTranscriptRenderers

[CrysisDeu]

Problem / Motivation

After P5-a (#8713) ChatPage dispatches rows through the app-sdk registry — but the dashboard's rich rows (tool line + launch cards, thinking block, nudge, recovery inject, the two completion cards, the error card with Continue) still lived in two hand-kept host lists: ChatPage's own, and pages/chat/transcriptRenderers.tsx's createTranscriptRenderers, which ChatPane consumes. #8713's review named this precisely: the mechanism was unified, the row set was not, and the mcp_oauth defect class survived between the two lists. The factory was also the weaker variant — tool disclosure keyed by row key rather than tool_call_id (#8204), "running" animation off the transcript flag rather than the page's trailing-group rule, no transcriptHot, no session hand-offs on the completion cards — so a pane drew a reduced transcript by construction.

Why it matters

RFC chat-core extraction P5-b. One dashboard row set: a row added or fixed once reaches the page and every pane, and a pane stops being the surface that silently gets less. Stacked on #8713 because it rewrites the host list that PR introduced.

What changed (motivation → approach → change)

The factory learns the page's behaviours (TranscriptRendererOptions). Two are unconditional because they are pure and no surface wanted the old default: the tool row's disclosure identity is now the #8204 tool_call_id fold for every surface (toolDisclosureKey moves out of ChatPage.tsx into the factory and is exported from there — a pane's row keys already embed the index, so its output does not change), and an unparseable file row draws whatever renderUnparsedFile(m, ctx) returns (default nothing, the pane's behaviour; ChatPage passes its bubble, the if-chain's fall-through). The rest are options with the pane's defaults: toolRunning(m, ctx) (default: ctx.running; ChatPage passes the trailing-group rule slotState === 'tool_running' && index > lastTextIdx), transcriptHot, and onSessionOpen / sessions / activeSession for the two completion cards. slot becomes optional — omitted, the tool line and launch cards read the active slot, which is what the page did. ChatPane's call is unchanged and every default reproduces what it drew before.

ChatPage's host list is the factory spread plus page-only rows, in this order: ...createTranscriptRenderers({...}) — which now includes tool_completion (the ✅/🚫 sibling draws nothing on every surface; a pane's fallback already drew nothing, the page's is the bubble, so claiming it in the factory closes "✅ done as a bubble" for both) — stop_event, notice, permission, the narrow undrawn, mcp_oauth, hidden_invisible_assistant, and the bubble. Nine page entries are deleted (file included — its one difference is the renderUnparsedFile option) (thinking_block, tool, nudge, recovery_inject, error, workflow_completion, subagent_completion), along with nine now-unused imports and the page's lastErrorIdx memo (the factory derives the newest-error index from the transcript it is handed).

ctx.row / ctx.wrapper are keyed Fragments, not elements: the factory returns ctx.row(<ToolCallLine …/>), and the row has to land in the DOM exactly as the page's own entry rendered it (the virtualizer measures the row's component root). ctx.wrapper is reached by no row on this page — the SDK defaults that use it (user, assistant, inject) claim roles the page's bubble entry claims first — and the parity test now pins that (it enumerates the wrapper-using defaults from the registry and checks the bubble's role list covers them), so the "no pixel to show" claim is asserted, not assumed.

No row's output changes on the page: every option ChatPage passes reproduces the prop set its deleted entry passed (checked entry by entry — tool line, launch cards, thinking block, nudge Loop gating, recovery card, completion cards' hand-offs, error Continue gating on continuable && interrupted && index === lastErrorIndex). ChatPane's output is unchanged because every new option defaults to the old behaviour.

A recorded decision this supersedes. transcriptRenderers.test.tsx's old drift guard carried a comment that converging the two row sets by moving ChatPage onto the registry "was considered and rejected (#3332, closed not-planned): the single-chat surface has no problem to fix". The RFC's §2.2 names the problem (mcp_oauth shipped registered-but-raw in the main chat because there were two lists), P1 introduced the registry and its parity test for exactly that, and P5 is the phase that removes the second list; the guard's comment is rewritten to say so. Flagging it here so the reversal is explicit, not silent.

Tests

• transcriptRenderers.test.tsx: the role-by-role "drift guard against the single-chat row chain" (which parsed ChatPage's .role === literals) becomes a structural guard — ChatPage imports and spreads createTranscriptRenderers ahead of its page-only rows, and keeps no private copy of a row the factory draws (zero exceptions). toolDisclosureKey.collision.test.ts imports the helper from its new home; toolDisclosureKey.renderSite.test.tsx (the Same-tick tool rows share one disclosure key (messageRowKey) #8204 render-site pin) passes unchanged.
• chatRolesParity.contract.test.ts: host-entry ids now include the factory's when the page spreads it; the documented non-default ids gain the factory's (workflow_run_tool, subagent_run_tool, tool_completion); a new assertion pins that every SDK default rendering through ctx.wrapper is shadowed by the bubble.
• transcriptRenderers.test.tsx: the deny-sibling guard now asserts the sibling resolves to tool_completion and renders null (it used to assert "unclaimed").
• RecoveryCard.test.tsx: the card wiring is asserted in the factory and the spread asserted in the page.
• Compatibility evidence — unmodified: all 63 ChatPage* suites and all ChatPane* suites, transcriptRenderersRenderCov80, TranscriptRowGeometry, TurnBlock.rowMemo, ChatPageCoverage (the ✅ done tool-completion row still draws nothing) — 890 tests across 89 files.

Local gates: tsc -b, eslint on changed files, check-i18n-strings (0 added). The full vitest suite runs in CI.

Manual verification

N/A — a dispatch/ownership refactor with prop-set-equivalent entries and deterministic coverage; no rendered output changes on either surface.

Why no screenshot: every option ChatPage passes reproduces the prop set of the entry it replaces, and every new factory option defaults to ChatPane's previous behaviour; there is no pixel to show on either surface.

Related Issues

• RFC chat-core extraction (docs(rfc): record the chat-core extraction RFC and its progress #8690), P5-b; stacked on refactor(chat-core): dispatch ChatPage's transcript rows through the renderer registry #8713 (P5-a). Supersedes the not-planned outcome of chat transcript: ChatPage should consume the shared row registry so there is one row set #3332 (see above).
• Later P5 cuts: turn grouping, composer band; chat-core P3: store-free seam for ChatInput so ChatEmbed no longer requires the host store #8651 (store-free ChatInput seam).

Checklist

• Single commit, conventional title
• Drift guard replaced by a structural guard, not deleted; parity contract extended
• No user-visible strings added
• Render dispatch / row ownership only

Contribution License Agreement

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

[github-actions]

UX Review (Fable 5) — ✅ PASS

UX-level review of b5be62e6141741c797f2dd2a2184464e3a96c9fe — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

UX-Verdict: PASS

Pure renderer consolidation: every row a user sees renders identically, and parity is pinned by contract tests rather than left to intent.

I traced each apparent narrowing against its predicate: workflow_completion roles ['*']→['assistant'] is a no-op (isWorkflowCompletionMessage already requires role assistant), the thinking entry's role gate matches hasReasoningContent's own role check, stop events travel as role system so no shared entry outranks their card, the unparseable-file-to-bubble fallback is preserved via renderUnparsedFile, and the ✅/🚫 tool-completion sibling still draws nothing (now claimed explicitly on every surface). The div→Fragment wrapper swap is guarded by a new contract test proving no row reaches the wrapper-using defaults. No new strings, states, or surfaces; nothing a user could perceive changed.

[UX-REVIEWED] b5be62e

[github-actions]

Design Review (Fable 5) — ✅ PASS

Design-level review of b5be62e6141741c797f2dd2a2184464e3a96c9fe — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: PASS

The two hand-kept row lists were a proven defect source; converging them into one factory with pane-default options is the right shape, and the reversal of #3332 is recorded, not silent.

The equivalences the "no output change" claim rests on check out against the code, not just the description: isWorkflowCompletionMessage self-gates on role so the ['*']→['assistant'] narrowing is a no-op, hasReasoningContent is exactly REASONING_ROLES plus a content guard, and tool_completion's unconditional null claim reproduces the page's old unconditional tool-role claim. The riskiest move — swapping the page's styled wrapper/row divs for keyed Fragments — is backed by a registry-enumerating test that pins every wrapper-using SDK default as bubble-shadowed, so the assumption fails loudly if a future default breaks it. The old drift guard was rewritten into structural guards (spread present, no shadowing ids) rather than deleted.

[DESIGN-REVIEWED] b5be62e

[github-actions]

First Principles Review (Fable 5) — ✅ PASS

Premise-level review of b5be62e6141741c797f2dd2a2184464e3a96c9fe — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Verification complete. Both factory consumers counted (ChatPane at website/src/components/ChatPane.tsx:589, ChatPage at website/src/pages/ChatPage.tsx:7773), toolDisclosureKey's consumers checked (its one render site is now the factory itself; the two test files pin #8204), and the factory's pre-existing lastErrorIndex helper confirmed as the mechanism the page's deleted memo deduplicates onto. The review follows.

First-Principles-Verdict: PASS

Deletes the second hand-kept row list — the named cause of the mcp_oauth drift class — and every rider is declared with its prop-parity accounted for.

What this change ships

Intent: make the dashboard's rich chat rows exist once so a row fixed once reaches the page and every pane — a FIX for a recorded defect class (RFC P5-b, #8713 review).

1. Page's rich rows (tool line, launch cards, thinking, nudge, recovery, completions, error) now come from the shared set; nine private entries deleted — justified, cause-level
2. "✅ done"/"🚫 denied" tool siblings can no longer surface as a bubble on the page (tool_completion) — justified, declared
3. Pane tool-row expand/collapse now keyed per tool_call_id (Same-tick tool rows share one disclosure key (messageRowKey) #8204 reaches panes) — justified, derived from reported defect
4. Completion cards gain session hand-off wiring a host can pass — one consumer (ChatPage), but it reproduces the deleted page entry's props — justified
5. Factory gains six options (toolRunning, transcriptHot, renderUnparsedFile, session trio), each single-consumer — justified: each replaces a prop a deleted entry passed; pane defaults preserve its old output
6. slot becomes optional (omitted → active slot) — declared, justified
7. Page's ctx.row/ctx.wrapper divs become keyed Fragments; wrapper unreachability now pinned by a test, not assumed — declared
8. Role-by-role drift guard replaced by a structural spread guard; chat transcript: ChatPage should consume the shared row registry so there is one row set #3332's not-planned decision reversed — declared explicitly, derived
9. toolDisclosureKey moves to the factory — justified move: the factory is now its only render site (grepped: 1 render site, 2 test files)
10. Page's newest-error memo deleted in favor of the factory's existing lastErrorIndex — deduplication onto an existing mechanism

The direction is the one this lane exists to reward: net deletion (~120 page lines out, ~60 factory lines in), a second spelling removed rather than guarded, and the one recorded decision it overturns (#3332) is named in the description with the defect that overturns it. Remaining split from the store-free app-sdk/ChatMessageList default registry is documented as deliberate in the factory header, not an unfixed sibling.

[FIRST-PRINCIPLES-REVIEWED] b5be62e

[github-actions]

GPT 5.6 Review — ✅ no blocking findings

GPT 5.6 completed its review of b5be62e6141741c797f2dd2a2184464e3a96c9fe and found no blocking issues.

This comment is updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] b5be62e

False positive or not applicable? A repository writer can comment:
/ai-review override gpt b5be62e6141741c797f2dd2a2184464e3a96c9fe: <one-sentence reason>

[github-actions]

Opus 4.8 Review — ✅ no blocking findings

Reviewed b5be62e6141741c797f2dd2a2184464e3a96c9fe — this comment is updated in place on each push.

Review details

No findings.

[OPUS-REVIEWED] b5be62e

Verdict parsed from the review's SHA-scoped output markers for commit b5be62e6141741c797f2dd2a2184464e3a96c9fe.

False positive or not applicable? A repository writer can comment:
/ai-review override fable b5be62e6141741c797f2dd2a2184464e3a96c9fe: <one-sentence reason>

[CrysisDeu]

Design — Suggestion: fold tool_completion's () => null into the factory so file is the only page exception (062d7a013 → fabb37a47)

Applied. tool_completion is a factory entry now — behaviour-identical for a pane (its unclaimed-role fallback already drew nothing) and it closes "✅ done as a bubble" on the page without a page exception. The pane's registry test that pinned the deny sibling as unclaimed now pins it as resolving to tool_completion and rendering null, which is the property the old wording was after ("drawing it would double the row"). file is the only remaining page-side asymmetry, called out in the structural guard.

First Principles — Watch: the no-screenshot waiver rests on the unpinned claim that ctx.wrapper is unreachable on the page (062d7a013 → fabb37a47)

Pinned. The parity contract now enumerates the SDK defaults whose render calls ctx.wrapper( (asserting the set is exactly user / assistant / inject) and checks the page's bubble entry claims every role they claim — host entries resolve first, so no row can reach a wrapper-using default. The description's claim is now backed by that assertion rather than by a comment.

[CrysisDeu]

First Principles — Subtraction: drop the toolDisclosureKey option; make the #8204 fold the factory's unconditional disclosure identity (fabb37a47 → 85ef5fdb9)

Applied. toolDisclosureKey moves out of ChatPage.tsx into transcriptRenderers.tsx (exported from there; the collision suite imports the new home, the render-site pin passes unchanged) and the factory uses it unconditionally — the option, its branch, and the description's "weaker variant" carve-out are gone. As you note, a pane's row keys already embed the index, so its output does not change.

Design — Suggestion: fold the page-only file shadow into a factory option so the shadow guard can assert zero duplicates (fabb37a47 → 85ef5fdb9)

Applied. renderUnparsedFile?(m, ctx) on TranscriptRendererOptions, default nothing (the pane's behaviour); ChatPage passes its bubble (the if-chain's fall-through). The page's file entry is deleted, FileCard is no longer imported by the page, and the structural guard now asserts zero shadowed ids — no carve-out left to mask a future duplicate.

[CrysisDeu]

First Principles — Subtraction (deferred, as you frame it): renderUnparsedFile exists only to preserve the if-chain's fall-through for a non-JSON file row (85ef5fdb9)

Agreed and recorded as a follow-up: once this lands, a small PR makes an unparseable file row draw nothing on both surfaces and deletes the option — a deliberate behaviour change (raw JSON as an assistant bubble was never designed) that this PR's "no row's output changes" invariant keeps out of scope. Noted on the P5 follow-up list in the RFC.

[chenmingwei23]

Tier 1 auto-approve: refactor (6 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: refactor -- code-move only, ChatPage spreads createTranscriptRenderers ahead of page-only rows, prop-set-equivalent entries, no rendered output change on either surface.

[iamwhatever]

Tier 1 auto-approve: refactor (6 files). Criteria: no conflict, no requested changes, security path denylist clean, design-doc gate clean, SAST annotations clean, security checklist all-NO, AI reviewers green. Category: code-move only -- ChatPage stops carrying its own duplicate renderer if-chain and spreads the shared createTranscriptRenderers row set extracted into website/src/pages/chat/transcriptRenderers.tsx, with the four accompanying test files re-pointed at the new module; no runtime behaviour change, no backend surface, no auth/session path touched.