jerry/buzz
1
0
Fork 0
mirror of https://github.com/block/buzz.git synced 2026-10-29 21:41:33 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(desktop): keep a reader's work block open when a later answer regroups it

Browse Source 
A reader who opened a finished work block to read its steps was folded shut by
the agent posting another message — an event they did not cause and could not
predict.

Work blocks are derived, not stored. `groupConversationWorkBlocks` rebuilds them
every render and ids each one after its first step, and `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.

    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` stops existing, React unmounts it, and the `useState` holding
the reader's expansion goes with it. The block they were reading is still on
screen — it is just inside a different block now — so this is not a case where
the intent has nowhere to live.

The choice therefore moves above the block, keyed by the STEP ids it was taken
on rather than by block id, since the steps are what the reader's intent was
actually about and they survive regrouping. Two rules follow from the merge:

- **An open choice wins over a folded one.** A merged block can carry both, and
  the two are not symmetric: showing steps a reader did not ask for costs them a
  scroll, hiding steps they did ask for loses what they were reading.
- **A choice is recorded against every step in the block, not just the first.**
  Recording only the first leaves the absorbed block's stale `open` entry behind,
  and since open wins the read, the merged block could never be folded again —
  the reader's click would do nothing. Caught by mutation, not by inspection; the
  first version of the test passed against it, so it now asserts the fold-back.

`useWorkBlockDisclosureStore` returns `null` rather than a no-op store when no
transcript provides one, and the block falls back to local state. A no-op default
would make a block mounted on its own silently swallow every click — how most of
this component's tests mount it — and local state is the correct behaviour in
isolation, where nothing is regrouping anything. A provider emits no DOM, so
`default`/`compactPreview` markup stays byte-identical (the byte-for-byte
baseline test covers this).

Verified by mutation, each applied and reverted in isolation:

- bypass the shared store (i.e. the pre-fix behaviour) — the new test fails on
  the merge frame, `0 !== 1` open blocks, which is exactly the reader-visible
  symptom.
- record the choice against only the first step — fails the fold-back
  assertion, `1 !== 0`.

The test drives the real `AgentSessionTranscriptList` across the three frames so
the actual regrouping runs, and asserts the steps are on screen rather than
trusting a block that reports itself open.

Full desktop suite 5492 passing / 0 failing, 81 suites; `tsc --noEmit` clean.

Co-authored-by: Bradley Axen <baxen@squareup.com>
Signed-off-by: Bradley Axen <baxen@squareup.com>
This commit is contained in:
ss-dev-02
2026-08-24 22:36:47 -07:00
parent d02437981a
commit 91a5754ad5
4 changed files with 335 additions and 39 deletions
Download Patch File Download Diff File Expand all files Collapse all files

140
desktop/src/features/agents/ui/AgentSessionTranscriptList.conversation.test.mjs
View File

@@ -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",
);
});

67
desktop/src/features/agents/ui/AgentSessionTranscriptList.tsx
View File

@@ -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({
>
<AgentSessionTranscriptVariantProvider value={variant}>
<AgentSessionTranscriptTurnMetaProvider value={turnMeta}>
{displayBlocks.map((block) => {
const blockKey = getDisplayBlockKey(block);
return (
<motion.div
animate={ROW_ENTER_TO}
data-message-id={blockKey}
initial={
animationsDisabled ||
!hasCompletedInitialRenderRef.current
? false
: ROW_ENTER_FROM
}
key={blockKey}
layout={layoutAnimationsEnabled ? "position" : false}
transition={ROW_ENTER_SPRING}
>
{/* content-visibility stays on a non-animated child: motion
<AgentSessionWorkBlockDisclosureProvider>
{displayBlocks.map((block) => {
const blockKey = getDisplayBlockKey(block);
return (
<motion.div
animate={ROW_ENTER_TO}
data-message-id={blockKey}
initial={
animationsDisabled ||
!hasCompletedInitialRenderRef.current
? false
: ROW_ENTER_FROM
}
key={blockKey}
layout={layoutAnimationsEnabled ? "position" : false}
transition={ROW_ENTER_SPRING}
>
{/* 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. */}
<div className="content-visibility-auto">
<TranscriptDisplayBlockView
agentAvatarUrl={agentAvatarUrl}
agentName={agentName}
agentPubkey={agentPubkey}
block={block}
profiles={profiles}
/>
</div>
</motion.div>
);
})}
{isTurnLive && !isCompactPreview ? (
<TurnLivenessIndicator />
) : null}
<div className="content-visibility-auto">
<TranscriptDisplayBlockView
agentAvatarUrl={agentAvatarUrl}
agentName={agentName}
agentPubkey={agentPubkey}
block={block}
profiles={profiles}
/>
</div>
</motion.div>
);
})}
{isTurnLive && !isCompactPreview ? (
<TurnLivenessIndicator />
) : null}
</AgentSessionWorkBlockDisclosureProvider>
</AgentSessionTranscriptTurnMetaProvider>
</AgentSessionTranscriptVariantProvider>
</CodeBlockVariantContext.Provider>

46
desktop/src/features/agents/ui/AgentSessionWorkBlock.tsx
View File

@@ -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
* `<details>` echo trap: `<details>` 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<boolean | null>(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

121
desktop/src/features/agents/ui/agentSessionWorkBlockDisclosure.tsx Normal file
View File

@@ -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<string, boolean>;
/**
* 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<WorkBlockDisclosureStore | null>(null);
export function AgentSessionWorkBlockDisclosureProvider({
children,
}: {
children: React.ReactNode;
}) {
const store = useWorkBlockDisclosureState();
return (
<WorkBlockDisclosureContext.Provider value={store}>
{children}
</WorkBlockDisclosureContext.Provider>
);
}
/** The store's state and updater, so the fallback path can reuse it verbatim. */
export function useWorkBlockDisclosureState(): WorkBlockDisclosureStore {
const [choices, setChoices] =
React.useState<WorkBlockDisclosureChoices>(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);
}