diff --git a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversation.test.mjs b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversation.test.mjs index 1af5e95143..18f40a3322 100644 --- a/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversation.test.mjs +++ b/desktop/src/features/agents/ui/AgentSessionTranscriptList.conversation.test.mjs @@ -24,6 +24,7 @@ import assert from "node:assert/strict"; import { test } from "node:test"; import { + act, AUTHOR, AUTHOR_PROFILES, AUTHOR_TRUNCATED, @@ -364,3 +365,142 @@ test("the byte-for-byte baseline actually exercises every renderable item kind", assert.match(lifecycleText, /Context compacted/); assert.match(lifecycleText, /Turn failed/); }); + +/** + * A reader's expansion survives the work blocks being regrouped underneath it. + * + * Work blocks are derived, not stored: `groupConversationWorkBlocks` rebuilds + * them every render and ids each one after its first step + * (`work-block:${items[0].id}`). `findFinalAnswerId` exempts only the LAST + * assistant message from the block, so a second assistant message demotes the + * first — the earlier answer becomes work, and the runs on either side of it + * merge into one block: + * + * frame 2 work-block:thought:1[...] msg:1 work-block:thought:2[...] + * frame 3 work-block:thought:1[thought:1,tool:1,msg:1,thought:2,tool:2] msg:2 + * + * `work-block:thought:2` stops existing, so React unmounts it. With the fold + * state held in that component, a reader who had opened the second block to read + * its steps was folded shut by the agent posting a second message — an event + * they did not cause and cannot predict. This drives the real list so the actual + * regrouping runs, rather than asserting against hand-built segments. + */ +test("conversation keeps a reader's expanded work block open when a later answer regroups it", async () => { + const shared = { channelId: "chan-1", sessionId: "sess-1", turnId: "turn-1" }; + const at = (seconds) => + `2026-06-14T19:00:${String(seconds).padStart(2, "0")}.000Z`; + const thought = (id, seconds) => ({ + ...shared, + id, + type: "thought", + renderClass: "thought", + title: "Thinking", + text: `reasoning ${id}`, + timestamp: at(seconds), + }); + const tool = (id, seconds) => ({ + ...shared, + id, + type: "tool", + renderClass: "shell", + title: id, + toolName: "shell", + buzzToolName: null, + status: "completed", + args: {}, + result: "ok", + isError: false, + timestamp: at(seconds), + startedAt: at(seconds), + completedAt: at(seconds), + descriptor: { + renderClass: "shell", + label: "Ran command", + preview: id, + source: "shell", + groupKey: "shell:command", + }, + }); + const answer = (id, seconds) => ({ + ...shared, + id, + type: "message", + renderClass: "message", + role: "assistant", + title: "Test Agent", + text: `answer ${id}`, + timestamp: at(seconds), + }); + + const firstRun = [thought("thought:1", 1), tool("tool:1", 2)]; + const frame1 = [...firstRun, answer("msg:1", 3)]; + const frame2 = [...frame1, thought("thought:2", 4), tool("tool:2", 5)]; + const frame3 = [...frame2, answer("msg:2", 6)]; + + const { container, setOverrides } = await renderRerenderableTranscript( + "conversation", + { items: frame1 }, + ); + const blocks = () => [ + ...container.querySelectorAll('[data-testid="transcript-work-block"]'), + ]; + const summaries = () => [ + ...container.querySelectorAll( + '[data-testid="transcript-work-block-summary"]', + ), + ]; + const openCount = () => + summaries().filter((node) => node.getAttribute("aria-expanded") === "true") + .length; + + await setOverrides({ items: frame2 }); + assert.equal( + blocks().length, + 2, + "the demoted-answer frame should show two separate work blocks", + ); + assert.equal(openCount(), 0, "both blocks start folded once work finished"); + + // The reader opens the SECOND block — the one the merge destroys. + await act(async () => { + summaries()[1].click(); + }); + assert.equal(openCount(), 1, "the reader's click should open that block"); + + await setOverrides({ items: frame3 }); + assert.equal( + blocks().length, + 1, + "the second answer should merge the runs into one block", + ); + assert.equal( + openCount(), + 1, + "the merged block must stay open — the reader asked to see those steps, and an agent posting again is not a reason to fold them away", + ); + // The steps they were reading are actually on screen, not merely a block + // reporting itself open. + const stepText = [ + ...container.querySelectorAll('[data-testid="transcript-work-block-step"]'), + ] + .map((node) => node.textContent) + .join("\n"); + assert.match( + stepText, + /tool:2/, + "the reader's steps should still be visible", + ); + + // ...and the merged block is still theirs to fold. Recording the choice + // against only the block's first step would leave the absorbed block's stale + // `open` entry behind, and since an open choice wins the read, the merged + // block could never be folded again — the reader's click would do nothing. + await act(async () => { + summaries()[0].click(); + }); + assert.equal( + openCount(), + 0, + "a reader who folds the merged block must actually fold it", + ); +}); diff --git a/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx b/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx index 720d762d86..11c4246479 100644 --- a/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx +++ b/desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx @@ -35,6 +35,7 @@ import { } from "./AgentSessionTranscriptChrome"; import { buildConversationTurnMeta } from "./agentSessionConversationMeta"; import { AgentSessionWorkBlockSegment } from "./AgentSessionWorkBlock"; +import { AgentSessionWorkBlockDisclosureProvider } from "./agentSessionWorkBlockDisclosure"; import { conversationSegmentsForBlock, type TranscriptConversationSegment, @@ -246,40 +247,42 @@ export function AgentSessionTranscriptList({ > - {displayBlocks.map((block) => { - const blockKey = getDisplayBlockKey(block); - return ( - - {/* content-visibility stays on a non-animated child: motion + + {displayBlocks.map((block) => { + const blockKey = getDisplayBlockKey(block); + return ( + + {/* content-visibility stays on a non-animated child: motion measures the outer wrapper for layout animations, which would otherwise force skipped offscreen rows to render. */} -
- -
-
- ); - })} - {isTurnLive && !isCompactPreview ? ( - - ) : null} +
+ +
+
+ ); + })} + {isTurnLive && !isCompactPreview ? ( + + ) : null} +
diff --git a/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx b/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx index 99621ae7be..7f3982deb6 100644 --- a/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx +++ b/desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx @@ -11,6 +11,11 @@ import { useAgentSessionTranscriptTurnMeta, } from "./agentSessionTranscriptContext"; import { useTranscriptAnimationEnabled } from "./transcriptAnimationPreference"; +import { + readWorkBlockChoice, + useWorkBlockDisclosureState, + useWorkBlockDisclosureStore, +} from "./agentSessionWorkBlockDisclosure"; import { formatTranscriptTimestampTitle } from "./agentSessionUtils"; import { TranscriptActivityItem } from "./activityRenderClasses/TranscriptActivityItem"; import type { AgentTranscriptIdentityProps } from "./activityRenderClasses/types"; @@ -166,10 +171,16 @@ function useSettleOnFinish(isActive: boolean, motionEnabled: boolean) { * Whether the block renders open, and how the reader takes that decision over. * * The policy is "open while work is in flight, folded once it finishes". The - * reader's own click wins from then on, for as long as the block stays mounted. + * reader's own click wins from then on. * - * This is deliberately local and button-native. An earlier version used a - * shared `useControlledDisclosure` hook whose whole reason to exist was the + * The reader's choice is held ABOVE this component, keyed by the step ids it was + * taken on, because a work block does not outlive its own membership: blocks are + * regrouped from scratch each render and merge when a second assistant message + * demotes the first from final answer, so a `useState` here is destroyed by the + * agent posting again. See `agentSessionWorkBlockDisclosure`. + * + * The trigger is deliberately button-native. An earlier version used a shared + * `useControlledDisclosure` hook whose whole reason to exist was the * `
` echo trap: `
` fires `toggle` for programmatic `open` * changes as well as for clicks, indistinguishably, so a policy-driven open * echoes back looking like reader intent and pins the row to its first policy @@ -177,14 +188,25 @@ function useSettleOnFinish(isActive: boolean, motionEnabled: boolean) { * can call `onToggle` is a real click — there is no echo to discriminate, and a * guard against it would be dead code that reads as though it were load-bearing. */ -function useWorkBlockDisclosure(policyOpen: boolean) { - const [readerChoice, setReaderChoice] = React.useState(null); +function useWorkBlockDisclosure( + policyOpen: boolean, + itemIds: readonly string[], +) { + // Local state is the fallback for a block mounted outside a transcript, where + // nothing regroups it and per-component state is correct. Both hooks always + // run; only one of them is read. + const localStore = useWorkBlockDisclosureState(); + const { choices, choose } = useWorkBlockDisclosureStore() ?? localStore; + const readerChoice = readWorkBlockChoice(choices, itemIds); const open = readerChoice ?? policyOpen; return { chosenByReader: readerChoice !== null, open, - toggle: React.useCallback(() => setReaderChoice(!open), [open]), + toggle: React.useCallback( + () => choose(itemIds, !open), + [choose, itemIds, open], + ), }; } @@ -218,7 +240,17 @@ export function AgentSessionWorkBlockSegment({ const motionEnabled = useWorkBlockMotionEnabled(); const settling = useSettleOnFinish(status.isActive, motionEnabled); - const disclosure = useWorkBlockDisclosure(status.isActive || settling); + // The steps the reader's fold choice is recorded against. Memoized on the + // items so `toggle`'s identity is stable between renders that did not change + // membership. + const itemIds = React.useMemo( + () => block.items.map((item) => item.id), + [block.items], + ); + const disclosure = useWorkBlockDisclosure( + status.isActive || settling, + itemIds, + ); const isLive = status.isActive; // A reader who expanded the block asked to see all of it, so windowing stops // applying. The test is the reader's *choice*, not `open`: policy already diff --git a/desktop/src/features/agents/ui/agentSessionWorkBlockDisclosure.tsx b/desktop/src/features/agents/ui/agentSessionWorkBlockDisclosure.tsx new file mode 100644 index 0000000000..1b5483bd3a --- /dev/null +++ b/desktop/src/features/agents/ui/agentSessionWorkBlockDisclosure.tsx @@ -0,0 +1,121 @@ +import * as React from "react"; + +/** + * The reader's fold/unfold choices for focus-mode work blocks, keyed by the + * STEP ids a choice was taken on rather than by block id. + * + * ## Why this cannot be `useState` inside the block + * + * A work block's id is derived from its first step (`work-block:${items[0].id}`, + * see `groupConversationWorkBlocks`), and block membership is recomputed from + * scratch on every render. `findFinalAnswerId` exempts only the LAST assistant + * message from the block, so when a second assistant message arrives the first + * one stops being the answer, becomes work, and the run of steps around it + * merges: two blocks become one. + * + * frame 2 work-block:th:1[th:1,tool:1] msg:1 work-block:th:2[th:2,tool:2] + * frame 3 work-block:th:1[th:1,tool:1,msg:1,th:2,tool:2] msg:2 + * + * `work-block:th:2` ceases to exist, so React unmounts it and any state it + * owned goes with it. A reader who had opened it to read the steps was folded + * shut by an event they did not cause — the agent posting another message. + * + * Keying on step ids survives that, because the steps are what the reader's + * intent was actually about: they are still on screen after the merge, just + * inside a different block. + */ +export type WorkBlockDisclosureChoices = ReadonlyMap; + +/** + * The reader's choice for a block, or `null` when they have not taken one and + * policy still owns the fold. + * + * An open choice wins over a folded one. After a merge the block can carry both + * — one constituent opened, another folded — and the two are not symmetric: + * showing steps a reader asked to see costs them a scroll, while hiding steps a + * reader asked to see loses the thing they were reading. Same reason the + * absorbed block's own choice cannot simply be dropped. + */ +export function readWorkBlockChoice( + choices: WorkBlockDisclosureChoices, + itemIds: readonly string[], +): boolean | null { + let folded: boolean | null = null; + for (const itemId of itemIds) { + const choice = choices.get(itemId); + if (choice === true) return true; + if (choice === false) folded = false; + } + return folded; +} + +/** + * Record one choice against every step the block currently holds. + * + * Written across all of them, not just the first, because the block this choice + * was taken on may later be absorbed into a block that begins with a different + * step — and the reader's intent has to be findable from whichever step the + * merged block happens to start with. + */ +export function recordWorkBlockChoice( + choices: WorkBlockDisclosureChoices, + itemIds: readonly string[], + choice: boolean, +): WorkBlockDisclosureChoices { + const next = new Map(choices); + for (const itemId of itemIds) next.set(itemId, choice); + return next; +} + +const EMPTY_CHOICES: WorkBlockDisclosureChoices = new Map(); + +export type WorkBlockDisclosureStore = { + choices: WorkBlockDisclosureChoices; + choose: (itemIds: readonly string[], choice: boolean) => void; +}; + +/** + * `null` when no transcript is providing a store. + * + * Deliberately not a no-op store: a work block rendered on its own — which is + * how most of its tests mount it — would then swallow every click silently and + * look like a component whose disclosure is broken. `useWorkBlockDisclosure` + * falls back to component-local state instead, which is correct in isolation + * (nothing is regrouping the block) and is the behaviour a reader of that + * component would expect. A provider emits no DOM, so `default` and + * `compactPreview` markup is unaffected either way. + */ +const WorkBlockDisclosureContext = + React.createContext(null); + +export function AgentSessionWorkBlockDisclosureProvider({ + children, +}: { + children: React.ReactNode; +}) { + const store = useWorkBlockDisclosureState(); + + return ( + + {children} + + ); +} + +/** The store's state and updater, so the fallback path can reuse it verbatim. */ +export function useWorkBlockDisclosureState(): WorkBlockDisclosureStore { + const [choices, setChoices] = + React.useState(EMPTY_CHOICES); + const choose = React.useCallback( + (itemIds: readonly string[], choice: boolean) => { + setChoices((current) => recordWorkBlockChoice(current, itemIds, choice)); + }, + [], + ); + + return React.useMemo(() => ({ choices, choose }), [choices, choose]); +} + +export function useWorkBlockDisclosureStore() { + return React.useContext(WorkBlockDisclosureContext); +}