Skip to content

Commit cf0da24

Browse files
committed
refactor(agent): ReasoningBlock replaces ActivityBlock on native details (CL-8482)
1 parent abe844f commit cf0da24

7 files changed

Lines changed: 73 additions & 120 deletions

File tree

‎ARCHITECTURE.md‎

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -219,6 +219,7 @@ in `useState` with no way for a parent to read or drive it.
219219
| `NotificationsBell` | `src/ui/notifications-bell.tsx` | 1 importer | **Fixed** — added optional `open`/`onOpenChange` |
220220
| `TenantSelector` | `src/ui/tenant-selector.tsx` | 1 importer | **Fixed** — added optional `open`/`onOpenChange` |
221221
| `ThreadSwitcher` | `src/ui/thread-switcher.tsx` | 1 importer | **Fixed** — added optional `open`/`onOpenChange` |
222+
| `ReasoningBlock` (was `ActivityBlock`) | `src/ui/reasoning-block.tsx` | 1 importer | **Fixed** — added optional `open`/`onOpenChange` over the native `<details>` |
222223
| `SubagentDock` (per-row disclosure) | `src/ui/subagent-dock.tsx` | 1 importer | Deferred — lifting it means a parent tracking one open flag per subagent row (a map, not a boolean); a bigger surface change than this pass covers |
223224

224225
All five fixes are additive: `open`/`onOpenChange` are optional, and the
@@ -227,10 +228,7 @@ calls these components today changes behavior.
227228

228229
A few more components hold local `open`/`expanded`-shaped state and were
229230
checked against the same bar, but don't belong on the fix list:
230-
`ActivityBlock` is backed by a native `<details>` on purpose (see its own
231-
doc comment) — the browser supplies the disclosure semantics, keyboard
232-
behavior and find-in-page expansion, which making it controlled would mean
233-
re-deriving by hand. `ApprovalCard`'s per-detail "show more" is a text
231+
`ApprovalCard`'s per-detail "show more" is a text
234232
truncation toggle, not a layout region — nobody outside the row has a reason
235233
to know or set it. `ConfirmButton`'s `armed` state is already documented as
236234
deliberately uncontrolled (`defaultArmed` only seeds the initial value).
@@ -347,8 +345,8 @@ pair: `open` + `onOpenChange`, or `collapsed` + `onToggle`. `Sidebar` sets
347345
this precedent; `Dialog` and `CommandPalette` apply it to popups;
348346
`NotificationsBell`, `ToolBlock` and `ToolNarrative` do too, all
349347
uncontrolled-by-default so adding the pair is never a breaking change. The
350-
exception is state a parent has no legitimate reason to want: `ActivityBlock`
351-
stays a native `<details>`, and a text-truncation toggle stays local. If
348+
exception is state a parent has no legitimate reason to want: a
349+
text-truncation toggle stays local. If
352350
you're unsure which side of that line something is on, ask "would a real
353351
caller ever need to read or set this from outside" — not "could they."
354352

‎CHANGELOG.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,10 @@ contract — modules that are not exported are internal and may change in any re
3737
`activity-block` stayed — `chat-thread` uses them. The package is now 113
3838
public modules. **`@tanstack/react-query` is no longer a peer dependency** —
3939
nothing in the package imports it.
40+
- **`ActivityBlock` is renamed `ReasoningBlock`** (module `reasoning-block`),
41+
aligned with `reasoningText()` naming. The prop `working` is now `streaming`,
42+
and it gains `durationLabel` plus the optional `open`/`onOpenChange`
43+
controlled pair while staying a native `<details>`.
4044
- **`AuthLayout` no longer defaults its decorative panel to `DitherCanvas`.** The panel is
4145
now an explicit `panel` slot with no fallback — pass `<DitherCanvas />` or any other
4246
content. A caller that omitted `panel` to get the dither canvas for free now gets a

‎package.json‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -191,10 +191,6 @@
191191
"types": "./dist/lib/workflow-run-progress.d.ts",
192192
"default": "./dist/lib/workflow-run-progress.js"
193193
},
194-
"./ui/activity-block": {
195-
"types": "./dist/ui/activity-block.d.ts",
196-
"default": "./dist/ui/activity-block.js"
197-
},
198194
"./ui/agent-turn": {
199195
"types": "./dist/ui/agent-turn.d.ts",
200196
"default": "./dist/ui/agent-turn.js"
@@ -415,6 +411,10 @@
415411
"types": "./dist/ui/quote-card.d.ts",
416412
"default": "./dist/ui/quote-card.js"
417413
},
414+
"./ui/reasoning-block": {
415+
"types": "./dist/ui/reasoning-block.d.ts",
416+
"default": "./dist/ui/reasoning-block.js"
417+
},
418418
"./ui/research-body": {
419419
"types": "./dist/ui/research-body.d.ts",
420420
"default": "./dist/ui/research-body.js"

‎src/index.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,6 @@ export * from "./lib/workflow-registry.js";
3333
export * from "./lib/workflow-run.js";
3434
export * from "./lib/workflow-run-error.js";
3535
export * from "./lib/workflow-run-progress.js";
36-
export * from "./ui/activity-block.js";
3736
export * from "./ui/agent-turn.js";
3837
export * from "./ui/animated-number.js";
3938
export * from "./ui/approval-card.js";
@@ -89,6 +88,7 @@ export * from "./ui/progress-checklist.js";
8988
export * from "./ui/provider-mark.js";
9089
export * from "./ui/quick-reply-chips.js";
9190
export * from "./ui/quote-card.js";
91+
export * from "./ui/reasoning-block.js";
9292
export * from "./ui/research-body.js";
9393
export * from "./ui/rich-empty-state.js";
9494
export * from "./ui/run-now-button.js";

‎src/ui/activity-block.tsx‎

Lines changed: 0 additions & 54 deletions
This file was deleted.

‎src/ui/reasoning-block.test.tsx‎

Lines changed: 36 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -13,64 +13,72 @@ function mount(props: ComponentProps<typeof ReasoningBlock>) {
1313
root.render(createElement(ReasoningBlock, props));
1414
});
1515
return {
16-
button: () => container.querySelector("button") as HTMLButtonElement,
17-
region: () => container.querySelector('[role="region"]'),
16+
details: () => container.querySelector("details"),
17+
summary: () => container.querySelector("summary"),
18+
body: () => container.querySelector("details > p"),
1819
unmount: () => act(() => root.unmount()),
1920
};
2021
}
2122

2223
describe("ReasoningBlock", () => {
2324
test("renders nothing for empty reasoning text", () => {
24-
const { button, unmount } = mount({ text: " " });
25-
expect(button()).toBeNull();
25+
const { details, unmount } = mount({ text: " " });
26+
expect(details()).toBeNull();
2627
unmount();
2728
});
2829

29-
test("is collapsed by default and toggles open on click", () => {
30-
const { button, region, unmount } = mount({ text: "Weighing two options." });
31-
expect(button().getAttribute("aria-expanded")).toBe("false");
32-
expect(region()).toBeNull();
30+
test("is collapsed by default and opens via the details toggle", () => {
31+
const { details, summary, body, unmount } = mount({ text: "Weighing two options." });
32+
expect(details()?.hasAttribute("open")).toBe(false);
33+
expect(body()).toBeTruthy();
3334

35+
const el = details() as HTMLDetailsElement;
3436
act(() => {
35-
button().dispatchEvent(new MouseEvent("click", { bubbles: true }));
37+
el.open = true;
38+
el.dispatchEvent(new Event("toggle"));
3639
});
3740

38-
expect(button().getAttribute("aria-expanded")).toBe("true");
39-
expect(region()?.textContent).toBe("Weighing two options.");
41+
expect(details()?.hasAttribute("open")).toBe(true);
42+
expect(body()?.textContent).toBe("Weighing two options.");
43+
expect(summary()).toBeTruthy();
4044
unmount();
4145
});
4246

4347
test("shows the duration label once idle, and \"Thinking…\" while streaming", () => {
4448
const idle = mount({ text: "Done thinking.", durationLabel: "Thought for 4s" });
45-
expect(idle.button().textContent).toBe("Thought for 4s");
49+
expect(idle.summary()?.textContent).toBe("Thought for 4s");
4650
idle.unmount();
4751

4852
const streaming = mount({ text: "Still going.", streaming: true, durationLabel: "Thought for 4s" });
49-
expect(streaming.button().textContent).toBe("Thinking…");
53+
expect(streaming.summary()?.textContent).toBe("Thinking…");
5054
streaming.unmount();
5155
});
5256

5357
test("idle without a duration label uses Thinking, not a cute fallback", () => {
5458
const idle = mount({ text: "Done thinking." });
55-
expect(idle.button().textContent).toBe("Thinking");
56-
expect(idle.button().textContent).not.toContain("Thought about this");
59+
expect(idle.summary()?.textContent).toBe("Thinking");
60+
expect(idle.summary()?.textContent).not.toContain("Thought about this");
5761
idle.unmount();
5862
});
5963

60-
test("two mounted blocks have distinct region ids matching their toggles", () => {
61-
const first = mount({ text: "First thought.", defaultOpen: true });
62-
const second = mount({ text: "Second thought.", defaultOpen: true });
63-
64-
const firstControls = first.button().getAttribute("aria-controls");
65-
const secondControls = second.button().getAttribute("aria-controls");
64+
test("controlled open state reports toggles through onOpenChange", () => {
65+
let seen: boolean | undefined;
66+
const { details, unmount } = mount({
67+
text: "Controlled.",
68+
open: true,
69+
onOpenChange: (next) => {
70+
seen = next;
71+
},
72+
});
73+
expect(details()?.hasAttribute("open")).toBe(true);
6674

67-
expect(firstControls).toBeTruthy();
68-
expect(secondControls).toBeTruthy();
69-
expect(firstControls).not.toBe(secondControls);
70-
expect(first.region()?.id ?? null).toBe(firstControls);
71-
expect(second.region()?.id ?? null).toBe(secondControls);
75+
const el = details() as HTMLDetailsElement;
76+
act(() => {
77+
el.open = false;
78+
el.dispatchEvent(new Event("toggle"));
79+
});
7280

73-
first.unmount();
74-
second.unmount();
81+
expect(seen).toBe(false);
82+
unmount();
7583
});
7684
});

‎src/ui/reasoning-block.tsx‎

Lines changed: 24 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,14 @@
11
import { ChevronRight } from "lucide-react";
2-
import { useId } from "react";
32

43
import { useControllableState } from "../hooks/use-controllable-state.js";
54
import { cn } from "../lib/utils.js";
65

76
export type ReasoningBlockProps = {
87
/** The agent's thinking. Rendered as text — it is not markup. */
98
readonly text: string;
10-
/** Still streaming. Changes the summary wording and marks the region busy. */
9+
/** Still streaming. Changes the summary wording and marks the body busy. */
1110
readonly streaming?: boolean;
12-
/** e.g. "Thought for 4s". Shown once reasoning has stopped streaming. */
11+
/** e.g. "Thought for 4s". Becomes the summary once reasoning has stopped streaming. */
1312
readonly durationLabel?: string;
1413
readonly defaultOpen?: boolean;
1514
/** Controlled open state. Pair with `onOpenChange` to lift it to a parent. */
@@ -25,9 +24,9 @@ export type ReasoningBlockProps = {
2524
* reader who wants it and noise for one who does not, and a block that
2625
* expands itself mid-stream makes the transcript jump under the reader.
2726
*
28-
* A `button` + region pair rather than a native `<details>` — the duration
29-
* label needs to sit next to the summary text without becoming part of it,
30-
* which `<summary>` cannot do while keeping a single accessible name.
27+
* A native `<details>`, not a div with state: it gets the disclosure
28+
* semantics, keyboard behaviour and find-in-page expansion from the browser,
29+
* and those are three things a hand-rolled version gets wrong.
3130
*/
3231
export function ReasoningBlock({
3332
text,
@@ -44,35 +43,33 @@ export function ReasoningBlock({
4443
onChange: onOpenChange,
4544
name: "ReasoningBlock",
4645
});
47-
const regionId = useId();
4846

4947
if (text.trim().length === 0) return null;
5048

5149
const summary = streaming ? "Thinking…" : (durationLabel ?? "Thinking");
5250

5351
return (
54-
<div data-slot="reasoning-block" className={cn("text-xs", className)}>
55-
<button
56-
type="button"
57-
onClick={() => setOpen((value) => !value)}
58-
aria-expanded={open}
59-
aria-controls={regionId}
60-
className="flex w-full items-center gap-1.5 rounded-md px-2 py-1.5 text-left text-muted-foreground transition-colors hover:bg-muted hover:text-foreground"
52+
<details
53+
data-slot="reasoning-block"
54+
open={open}
55+
onToggle={(event) => setOpen((event.currentTarget as HTMLDetailsElement).open)}
56+
className={cn("text-xs", className)}
57+
>
58+
<summary
59+
className={cn(
60+
"flex cursor-pointer list-none items-center gap-1.5 rounded-md px-2 py-1.5 text-muted-foreground transition-colors hover:bg-muted hover:text-foreground",
61+
"[&::-webkit-details-marker]:hidden",
62+
)}
6163
>
6264
<ChevronRight className={cn("size-3.5 shrink-0 transition-transform", open && "rotate-90")} aria-hidden />
6365
<span aria-live={streaming ? "polite" : undefined}>{summary}</span>
64-
</button>
65-
{open ? (
66-
<p
67-
id={regionId}
68-
role="region"
69-
aria-label="Reasoning"
70-
aria-busy={streaming}
71-
className="mt-1 ml-4 border-l border-border pl-3 leading-relaxed whitespace-pre-wrap text-muted-foreground"
72-
>
73-
{text}
74-
</p>
75-
) : null}
76-
</div>
66+
</summary>
67+
<p
68+
aria-busy={streaming}
69+
className="mt-1 ml-4 border-l border-border pl-3 leading-relaxed whitespace-pre-wrap text-muted-foreground"
70+
>
71+
{text}
72+
</p>
73+
</details>
7774
);
7875
}

0 commit comments

Comments
 (0)