feat(desktop): add conversation transcript variant for focus-mode agent sessions - #6538
feat(desktop): add conversation transcript variant for focus-mode agent sessions#6538baxen wants to merge 4 commits into
Conversation
…nt sessions **Category:** feature **User Impact:** In the wide/standalone agent session panel the observer transcript now reads like a conversation — the human's prompt as a right-aligned bubble, the agent's reply as unboxed prose at a comfortable measure, reasoning behind a "Thinking…/Thought for Ns" disclosure, the plan as a live checklist, and session/status noise receded to quiet dividers. **Problem:** The transcript had two presentations — the dense `default` activity feed and `compactPreview` — both tuned for a narrow panel. Read at full width they are a scan surface, not something you read a turn in. **Solution:** A third, purely additive `conversation` variant, threaded through `ManagedAgentSessionPanel` and `AgentSessionThreadPanel` as an optional prop with a width-based default. Tool items deliberately route through the same `ToolActivity` presenter as `default` so the parallel tool-chain-card work merges cleanly, and the drawer/shell is untouched. The byte-for-byte guarantee for the existing variants is enforced by a captured-markup fixture, not by inspection: the baseline was produced by mounting the same transcript on unmodified code, and the test fails on any drift in `default` or `compactPreview` output. Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7927fa37a3
ℹ️ 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".
| onToggle={(event) => { | ||
| setUserOpen(event.currentTarget.open); | ||
| }} | ||
| open={isOpen} |
There was a problem hiding this comment.
Ignore programmatic thought disclosure toggles
When a live thought mounts or isStreaming changes, React updates the <details> element's open property, which itself emits toggle; this handler then records that automatic state in userOpen. Consequently, the normal streaming transition pins userOpen to true, and the disclosure remains expanded when the next transcript item arrives instead of automatically collapsing as intended. Track actual summary interaction separately, or suppress toggle events caused by synchronizing open.
Useful? React with 👍 / 👎.
| const resolvedTranscriptVariant = | ||
| transcriptVariant ?? | ||
| resolveAgentSessionTranscriptVariant({ isSinglePanelView, widthPx }); |
There was a problem hiding this comment.
Select the variant using the rendered panel width
When a previously widened panel is restored and the channel viewport later narrows, widthPx retains the stored width while RightAuxiliaryPane clamps the rendered pane to calc(100% - 300px). For example, a stored 700px pane in an 800px channel area is actually rendered around 500px, but this check still selects conversation; because the single-panel breakpoint is only below 600px, the cramped pane remains in the layout this threshold was meant to avoid. Measure or clamp the effective panel width before resolving the variant.
Useful? React with 👍 / 👎.
Review follow-ups on the `conversation` transcript variant. Variant selection is now caller-pinned only. The 640px width heuristic and its module are gone: an automatic wide-pane mode is a separate product decision, and swapping the whole transcript presentation as a reader drags a resize handle across a threshold is not a decision to make implicitly. Slice A's cover host pins `conversation` explicitly. The thought disclosure no longer records the browser's programmatic toggle echo as a reader choice. `<details>` fires `toggle` when React changes `open`, so auto-opening a streaming thought was pinning it open forever and reasoning never receded once the agent acted. Only a toggle that disagrees with the last rendered state can have come from the reader. The regression test injects the echo and then rerenders with a following item, and it is the only test that fails when the guard is removed; a second test proves a genuine reader toggle still wins. Thought completion at the next leaf item of any kind — tool call or plan update, not just the next assistant message — is now documented as intended rather than incidental, with a thought -> tool -> message test pinning it. The byte-for-byte fixture now covers the contract it claims. The baseline transcript contains every renderable item kind (prompt with context and setup, assistant message, thought, plan, tool, lifecycle status, error, permission) across two sessions so a session boundary is forced, and both legacy variants were recaptured from pre-change main. A companion test asserts the baseline really renders each kind, so a fixture that silently stopped covering one cannot keep passing while protecting nothing. Also adds the plan-in-place rerender assertion: same plan id with advanced entries retains the same card node while the progress text changes. Known follow-up: Slice C is not merged yet, so its shared controlled-details hook does not exist on origin and the echo guard here is a local copy with the same semantics, flagged in a comment for replacement on rebase. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
The captured-markup fixture embeds formatted timestamps, because `formatTranscriptTimestampTitle` renders in the ambient zone. It was captured at UTC-7, so the byte-for-byte comparison failed in CI (UTC) for a reason that had nothing to do with markup: "at 12:00:01 PM" vs "at 7:00:01 PM". This is why Desktop Core failed on the previous two heads. Pin `process.env.TZ = "UTC"` before the transcript modules are imported — their `Intl.DateTimeFormat` instances are module-level constants that resolve the zone once at construction — and recapture the baseline from pre-change main under that zone. A guard assertion in the coverage test now fails with a specific message if the ambient zone is not UTC, rather than dumping a 12KB markup diff. Verified passing under TZ=UTC, TZ=America/New_York, and TZ=Asia/Tokyo. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Pinning TZ fixed one ambient input to the captured-markup fixture and missed a second one. The session-boundary divider formats its date with a bare `toLocaleString()` (`AgentSessionTranscriptChrome.tsx`), which is locale-sensitive as well as zone-sensitive: the fixture holds "6/14/2026, 7:05:00 PM", and under `LANG=de-DE` the same render produces "14.6.2026, 19:05:00". Node takes its default locale from LANG/LC_ALL, so the byte-for-byte comparison failed on any machine with a non-US locale — the same class of bug as the timezone one, one variable over, and the TZ guard passed happily while it happened. Assigning `process.env.LANG` at runtime does not fix this: node resolves its default locale once at startup, so unlike `TZ` (which `Date` reads lazily) the assignment is silently ineffective. Pinning the locale means overriding the two formatting surfaces the render path reaches — `Intl.DateTimeFormat` constructed without an explicit locale, and `Date.prototype.toLocale*`, which does not route through `Intl.DateTimeFormat` and needs its own patch. `Intl.DateTimeFormat` must be replaced with a plain function, not an arrow: the render path calls it with `new`, and an arrow function is not a constructor. The guard assertion now covers locale and `toLocaleString` output alongside the zone, so a future drift fails with a specific message instead of a 12KB markup diff. Verified: the de-DE failure reproduces before this change, and the suite passes after it under TZ/LANG combinations UTC/en-US, UTC/de-DE, America/New_York/ja-JP, Asia/Tokyo/de-DE, and UTC with LANG unset. Signed-off-by: ss-dev-01 <11939edb7df583f855dbef923f2358f1184538f88ca452e19e7e35e42ad6d796@buzz.block.builderlab.xyz> Co-authored-by: Bradley Axen <baxen@squareup.com> Signed-off-by: Bradley Axen <baxen@squareup.com>
Category: feature
User Impact: In the full-cover focus view (any host that pins the
conversationvariant) the observer transcript now reads like a conversation instead of a dense activity feed — the human's prompt as a right-aligned bubble, the agent's reply as unboxed prose at a comfortable measure, reasoning behind a "Thinking…/Thought for Ns" disclosure, the plan as a live checklist, and session/status noise receded to quiet dividers.Problem: The transcript had two presentations — the dense
defaultactivity feed andcompactPreview— both tuned for a narrow panel. Read at full width they are a scan surface, not something you read a turn in.Solution: A third, purely additive
conversationvariant, threaded throughManagedAgentSessionPanelandAgentSessionThreadPanelas an optional prop with a width-based default.Rhythm
conversationtreatmentbg-muted/60, capped atmax-w-[85%], author label + avatar, timestamp/context footer revealed on hover or keyboard focus. Unclamped — focus mode shows the whole prompt. Channel context stays behind the existing prompt-context affordance, not inline.max-w-3xlreading column,leading-relaxed, markdown/code through the existing sharedMarkdownrenderer.<details>disclosure labelled "Thinking…" while streaming and "Thought for Ns" once the turn moves on. Auto-opens while streaming, collapses when the turn produces its next item. A reader who toggles it takes over — stream transitions can't yank it back.ConversationDividerhairlines.ToolActivitypresenter asdefault.Spacing is
gap-8between turns andgap-3within one. The timestamp preference is still respected.Constraints honoured
AgentSessionTranscriptList.conversation.baseline.jsonwas produced by mounting the same transcript on unmodified code, and I re-verified it againstmain's tip in a clean throwaway worktree before committing. The test fails on any drift indefaultorcompactPreviewoutput.Notes for review
AgentSessionThreadPaneldefaults todefaultand only rendersconversationwhen a host explicitly asks for it (Slice A's cover drawer does). The earlier 640px width heuristic and its module were removed in review: an automatic wide-pane mode is a separate product decision, and swapping the whole transcript presentation as a reader drags a resize handle across a threshold is not a decision to make implicitly.agentSessionConversationMeta.tsand pinned by a thought → tool → message test.AgentSessionTranscriptChrome.tsxis a straight extraction of prompt/divider/setup chrome out ofAgentSessionTranscriptList.tsx, which was approaching the 1000-line ratchet. Its one behavioural change isimport.meta.env?.(optional chaining) so the module is importable from node render tests — the same pattern already used infeatures/onboarding/devFreshOnboarding.ts.Review round (changes requested → addressed)
Programmatic-toggle echo in the thought disclosure — fixed.
<details>firestogglewhen React changesopen, so auto-opening a streaming thought was recorded as a reader choice and pinned the disclosure open forever; reasoning never receded once the agent acted. Only a toggle that disagrees with the last rendered state can have come from the reader. The regression test injects the echo while streaming, then rerenders with a following item and asserts the disclosure collapsed — it is the only test that fails when the guard is reverted (verified). A second test proves a genuine reader toggle still survives later items.Byte-for-byte fixture now covers the contract it claims. The baseline transcript contains every renderable item kind — prompt with prompt-context and setup lifecycle, assistant message, thought, plan, tool, lifecycle status, error, permission — across two sessions so a session-boundary divider is forced. Both legacy variants were recaptured from pre-change main (
074561233) in a clean throwaway worktree.compactPreview's intentional omissions are captured in the fixture and therefore also protected. A companion test asserts the baseline really renders each kind, so a fixture that silently stopped covering one cannot keep passing while protecting nothing.Thought completion documented as intended (ruling: next leaf item of any kind) with a thought → tool (4s) → message (88s) test asserting the 4s duration.
Width heuristic removed;
agentSessionTranscriptVariantChoice.tsand its tests are deleted.Plan-in-place rerender assertion added: same plan id with advanced entries retains the same card DOM node while progress text changes 1/3 → 2/3.
Fixture pinned to UTC — this was a real bug in the previous head's CI. The captured markup embeds formatted timestamps (
formatTranscriptTimestampTitlerenders in the ambient zone), and the fixture had been captured at UTC-7, soDesktop Corefailed in CI (UTC) on a 12KB diff whose only real content was12:00:01 PMvs7:00:01 PM— on7927fa37atoo, not just this round. The test now pinsprocess.env.TZ = "UTC"before the transcript modules are imported (theirIntl.DateTimeFormatinstances are module-level constants that resolve the zone once at construction), the baseline was recaptured from pre-change main under that zone, and a guard assertion fails with a specific message instead of a markup dump if the ambient zone is not UTC. Verified passing underTZ=UTC,TZ=America/New_York, andTZ=Asia/Tokyo.Fixture locale pinned too — same bug class as the timezone one, one variable over.
AgentSessionTranscriptChrome.tsxformats the session-boundary date with a barenew Date(...).toLocaleString(), which is locale-sensitive as well as zone-sensitive: the fixture holds6/14/2026, 7:05:00 PM, and underLANG=de-DEthe same render produces14.6.2026, 19:05:00. The TZ guard passed silently while this failed. Three things make the fix non-obvious: assigningprocess.env.LANGat runtime is a no-op (node resolves its default locale once at startup —TZworks only becauseDatereads it lazily); patchingIntl.DateTimeFormatalone is insufficient becauseDate.prototype.toLocale*does not route through it; and theIntl.DateTimeFormatreplacement must be a plain function, not an arrow, since the render path calls it withnew. The guard now asserts resolved locale and a knowntoLocaleString()output alongside the zone. Non-vacuous: stripping the pin fails underde-DE. Verified green underUTC/en-US,UTC/de-DE,America/New_York/ja-JP,Asia/Tokyo/de-DE, andUTCwithLANGunset.Known follow-up (Slice C sequencing)
Slice C (#6536) is not merged yet, so the shared controlled-details hook it promises does not exist on origin. The echo guard here is a local implementation with the same semantics as C's
useToolRunDisclosure(rendered-state ref +useLayoutEffect+ ignore-agreeing-toggle), flagged in a code comment so the swap is mechanical on rebase. Still outstanding for the rebase:agentSessionConversationMeta.tshardcodessegment.kind === "summary", which C replaces with"tool-run"— it must flattensegment.run.items, plus a thought → tool-run meta test.Validation
At
e6c994a69(based on main tip074561233):pnpm test— 5379 passing, 0 failing (full desktop suite, not scoped), and re-run green across five TZ/locale combinationspnpm typecheck— cleanpnpm check(biome + px-text + pubkey-truncation) — findings identical to main's baseline (pre-existingterminal.css!importantwarnings and infos)pnpm check:file-sizes— cleandesktop-check,desktop-typecheck,desktop-test,file-size-checke6c994a69— every check SUCCESS or SKIPPED, includingDesktop Core, which was the job the ambient-formatting-dependent fixture had been failing (polled to completion, not inferred from local green)New coverage: 13 JSDOM rendering tests for the variant (including the recaptured baseline byte-identity assertion, its coverage guard, the echo-regression pair, and the plan-in-place rerender) plus the meta/checklist unit tests.