refactor(site/src/pages/AgentsPage): clean up RenderBlock types and dead fields (#23175)

RenderBlock's file-reference variant diverged from the API (camelCase
vs snake_case), and both file variants were defined inline duplicating
the generated ChatFilePart and ChatFileReferencePart types. The
thinking and file-reference variants carried dead fields (title, text)
that were never populated by the backend.

Replace inline definitions with references to generated types, remove
dead fields, and simplify ReasoningDisclosure (disclosure button path
was dead without title).

Refs #23168
This commit is contained in:
Mathias Fredriksson
2026-03-18 12:25:05 +00:00
committed by GitHub
parent 481c132135
commit 488ceb6e58
9 changed files with 106 additions and 267 deletions
@@ -653,7 +653,7 @@ export const WithSubagentCards: Story = {
},
};
/** Reasoning part without title renders inline (no disclosure). */
/** Completed reasoning part renders inline. */
export const WithReasoningInline: Story = {
parameters: {
queries: buildQueries(
@@ -687,7 +687,7 @@ export const WithReasoningInline: Story = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
// Reasoning text renders inline, not behind a disclosure.
// Reasoning text renders inline.
expect(canvas.getByText("Reasoning body")).toBeInTheDocument();
expect(canvas.queryByRole("button", { name: "Thinking" })).toBeNull();
},
@@ -991,10 +991,9 @@ export const SidebarWithSingleRepo: Story = {
},
};
/**
* Streaming reasoning part via WebSocket — renders collapsed and
* can be expanded on click.
* Streaming reasoning part via WebSocket — renders inline text.
*/
export const StreamedReasoningCollapsed: Story = {
export const StreamedReasoning: Story = {
parameters: {
queries: buildQueries(
{
@@ -1015,7 +1014,6 @@ export const StreamedReasoningCollapsed: Story = {
message_part: {
part: {
type: "reasoning",
title: "Plan migration",
text: "Streaming reasoning body",
},
},
@@ -1026,16 +1024,7 @@ export const StreamedReasoningCollapsed: Story = {
},
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
const user = userEvent.setup();
const reasoningToggle = await canvas.findByRole("button", {
name: "Plan migration",
});
expect(reasoningToggle).toHaveAttribute("aria-expanded", "false");
await user.click(reasoningToggle);
expect(reasoningToggle).toHaveAttribute("aria-expanded", "true");
await expect(
canvas.findByText("Streaming reasoning body"),
).resolves.toBeInTheDocument();
@@ -17,7 +17,7 @@ import {
TooltipContent,
TooltipTrigger,
} from "components/Tooltip/Tooltip";
import { ChevronDownIcon, PencilIcon } from "lucide-react";
import { PencilIcon } from "lucide-react";
import {
type FC,
Fragment,
@@ -43,12 +43,10 @@ import type {
const ReasoningDisclosure: FC<{
id: string;
title?: string;
text: string;
isStreaming?: boolean;
urlTransform?: UrlTransform;
}> = ({ id, title, text, isStreaming = false, urlTransform }) => {
const [isOpen, setIsOpen] = useState(false);
}> = ({ id, text, isStreaming = false, urlTransform }) => {
const { visibleText } = useSmoothStreamingText({
fullText: text,
isStreaming,
@@ -57,10 +55,8 @@ const ReasoningDisclosure: FC<{
});
const displayText = isStreaming ? visibleText : text;
const hasText = displayText.trim().length > 0;
const label = title ?? "Thinking";
const showStreamingPlaceholder = isStreaming && !hasText;
if (!title && hasText) {
if (hasText) {
return (
<div className="w-full">
<Response
@@ -72,49 +68,14 @@ const ReasoningDisclosure: FC<{
</div>
);
}
const labelContent = (
<span className="text-sm">
{showStreamingPlaceholder ? (
<Shimmer as="span">Thinking...</Shimmer>
) : (
label
)}
</span>
);
return (
<div className="w-full">
{hasText ? (
<button
type="button"
aria-expanded={isOpen}
aria-controls={id}
className="flex items-center gap-2 bg-transparent border-0 p-0 text-content-secondary transition-colors hover:text-content-primary cursor-pointer"
onClick={() => setIsOpen((prev) => !prev)}
>
{labelContent}
<ChevronDownIcon
className={cn(
"h-3 w-3 shrink-0 text-content-secondary transition-transform",
isOpen ? "rotate-0" : "-rotate-90",
)}
/>
</button>
) : (
<div className="flex items-center gap-2 text-content-secondary transition-colors hover:text-content-primary">
{labelContent}
</div>
)}
{isOpen && hasText ? (
<div id={id} className="mt-1.5">
<Response
className="text-[11px] text-content-secondary"
urlTransform={urlTransform}
>
{displayText}
</Response>
</div>
) : null}
<div className="flex items-center gap-2 text-content-secondary transition-colors hover:text-content-primary">
<span className="text-sm">
{isStreaming ? <Shimmer as="span">Thinking...</Shimmer> : "Thinking"}
</span>
</div>
</div>
);
};
@@ -190,7 +151,6 @@ function renderBlockList({
<ReasoningDisclosure
key={`${keyPrefix}-thinking-${index}`}
id={`${keyPrefix}-thinking-${index}`}
title={block.title}
text={block.text}
isStreaming={isStreaming}
urlTransform={urlTransform}
@@ -203,16 +163,11 @@ function renderBlockList({
className="my-1 flex items-start gap-2 rounded-md border border-content-link/20 bg-content-link/5 px-2.5 py-1.5"
>
<span className="shrink-0 text-xs font-medium text-content-link">
{block.fileName}:
{block.startLine === block.endLine
? block.startLine
: `${block.startLine}\u2013${block.endLine}`}
{block.file_name}:
{block.start_line === block.end_line
? block.start_line
: `${block.start_line}\u2013${block.end_line}`}
</span>
{block.text && (
<span className="text-sm text-content-primary">
{block.text}
</span>
)}
</div>
);
case "tool": {
@@ -408,9 +363,9 @@ const ChatMessageItem = memo<{
) : (
<FileReferenceChip
key={i}
fileName={block.fileName}
startLine={block.startLine}
endLine={block.endLine}
fileName={block.file_name}
startLine={block.start_line}
endLine={block.end_line}
/>
),
)
@@ -50,10 +50,8 @@ describe("appendTextBlock", () => {
});
it("appends a new thinking block to an empty list", () => {
const result = appendTextBlock([], "thinking", "pondering", "Deep thought");
expect(result).toEqual([
{ type: "thinking", text: "pondering", title: "Deep thought" },
]);
const result = appendTextBlock([], "thinking", "pondering");
expect(result).toEqual([{ type: "thinking", text: "pondering" }]);
});
it("merges consecutive response blocks", () => {
@@ -63,29 +61,13 @@ describe("appendTextBlock", () => {
expect(result[0]).toEqual({ type: "response", text: "aaabbb" });
});
it("merges consecutive thinking blocks with compatible titles", () => {
const blocks: RenderBlock[] = [
{ type: "thinking", text: "part1", title: "Reasoning" },
];
const result = appendTextBlock(blocks, "thinking", "part2", "Reasoning");
it("merges consecutive thinking blocks", () => {
const blocks: RenderBlock[] = [{ type: "thinking", text: "part1" }];
const result = appendTextBlock(blocks, "thinking", "part2");
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
type: "thinking",
text: "part1part2",
title: "Reasoning",
});
});
it("merges thinking blocks with different titles using the new title", () => {
const blocks: RenderBlock[] = [
{ type: "thinking", text: "part1", title: "Analyzing" },
];
const result = appendTextBlock(blocks, "thinking", "part2", "Planning");
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
type: "thinking",
text: "part1part2",
title: "Planning",
});
});
@@ -96,7 +78,6 @@ describe("appendTextBlock", () => {
expect(result[1]).toEqual({
type: "thinking",
text: "hmm",
title: undefined,
});
});
@@ -107,20 +88,6 @@ describe("appendTextBlock", () => {
expect(result[1]).toEqual({ type: "response", text: "after tool" });
});
it("uses the custom joinText function when merging", () => {
const blocks: RenderBlock[] = [{ type: "response", text: "line1" }];
const join = (a: string, b: string) => `${a}\n${b}`;
const result = appendTextBlock(
blocks,
"response",
"line2",
undefined,
join,
);
expect(result).toHaveLength(1);
expect(result[0]).toEqual({ type: "response", text: "line1\nline2" });
});
it("does not mutate the original blocks array", () => {
const blocks: RenderBlock[] = [{ type: "response", text: "original" }];
const result = appendTextBlock(blocks, "response", " added");
@@ -128,28 +95,4 @@ describe("appendTextBlock", () => {
expect((blocks[0] as { text: string }).text).toBe("original");
expect(result).not.toBe(blocks);
});
it("merges thinking block and uses new title", () => {
const blocks: RenderBlock[] = [
{ type: "thinking", text: "a", title: "Think" },
];
const result = appendTextBlock(blocks, "thinking", "b", "Thinking deeply");
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
type: "thinking",
text: "ab",
title: "Thinking deeply",
});
});
it("merges thinking blocks when both have no title", () => {
const blocks: RenderBlock[] = [{ type: "thinking", text: "a" }];
const result = appendTextBlock(blocks, "thinking", "b");
expect(result).toHaveLength(1);
expect(result[0]).toEqual({
type: "thinking",
text: "ab",
title: undefined,
});
});
});
@@ -1,13 +1,6 @@
import { asString } from "components/ai-elements/runtimeTypeUtils";
import type { RenderBlock } from "./types";
const createBlock = (
type: "response" | "thinking",
text: string,
title?: string,
): RenderBlock =>
type === "thinking" ? { type, text, title } : { type, text };
export const asNonEmptyString = (value: unknown): string | undefined => {
const next = asString(value).trim();
return next.length > 0 ? next : undefined;
@@ -16,18 +9,11 @@ export const asNonEmptyString = (value: unknown): string | undefined => {
/**
* Append a text or thinking block to a render block list, merging
* with the previous block when the types match.
*
* @param joinText Controls how existing and new text are concatenated
* when merging into an existing block. Callers that process
* complete message blocks typically join with a newline, while
* streaming callers concatenate directly.
*/
export const appendTextBlock = (
blocks: RenderBlock[],
type: "response" | "thinking",
text: string,
title?: string,
joinText: (current: string, next: string) => string = (a, b) => `${a}${b}`,
): RenderBlock[] => {
if (!text.trim()) {
return blocks;
@@ -35,15 +21,12 @@ export const appendTextBlock = (
const nextBlocks = [...blocks];
const last = nextBlocks[nextBlocks.length - 1];
if (last && last.type === type) {
nextBlocks[nextBlocks.length - 1] = createBlock(
nextBlocks[nextBlocks.length - 1] = {
type,
joinText(last.text, text),
type === "thinking" && last.type === "thinking"
? (title ?? last.title)
: undefined,
);
text: `${last.text}${text}`,
};
return nextBlocks;
}
nextBlocks.push(createBlock(type, text, title));
nextBlocks.push({ type, text });
return nextBlocks;
};
@@ -115,11 +115,11 @@ describe("parseMessageContent", () => {
it("parses a reasoning block", () => {
const result = parseMessageContent([
{ type: "reasoning", text: "Let me think...", title: "Reasoning" },
{ type: "reasoning", text: "Let me think..." },
]);
expect(result.reasoning).toBe("Let me think...");
expect(result.blocks).toEqual([
{ type: "thinking", text: "Let me think...", title: "Reasoning" },
{ type: "thinking", text: "Let me think..." },
]);
});
@@ -263,17 +263,15 @@ describe("parseMessageContent", () => {
start_line: 10,
end_line: 15,
content: "some added code lines",
text: "Consider using a constant here.",
},
]);
expect(result.blocks).toHaveLength(1);
expect(result.blocks[0]).toEqual({
type: "file-reference",
fileName: "src/main.go",
startLine: 10,
endLine: 15,
file_name: "src/main.go",
start_line: 10,
end_line: 15,
content: "some added code lines",
text: "Consider using a constant here.",
});
});
@@ -283,12 +281,11 @@ describe("parseMessageContent", () => {
type: "file-reference",
file_name: "bare.ts",
content: "bare content",
text: "No line info.",
},
]);
const ref = result.blocks[0] as { startLine: number; endLine: number };
expect(ref.startLine).toBe(0);
expect(ref.endLine).toBe(0);
const ref = result.blocks[0] as { start_line: number; end_line: number };
expect(ref.start_line).toBe(0);
expect(ref.end_line).toBe(0);
});
it("does not affect markdown when file-reference blocks are present", () => {
@@ -300,7 +297,6 @@ describe("parseMessageContent", () => {
start_line: 1,
end_line: 2,
content: "nit code content",
text: "Nit.",
},
]);
expect(result.markdown).toBe("Hello");
@@ -308,11 +304,10 @@ describe("parseMessageContent", () => {
expect(result.blocks[0]).toEqual({ type: "response", text: "Hello" });
expect(result.blocks[1]).toEqual({
type: "file-reference",
fileName: "a.go",
startLine: 1,
endLine: 2,
file_name: "a.go",
start_line: 1,
end_line: 2,
content: "nit code content",
text: "Nit.",
});
});
@@ -1,6 +1,6 @@
import type * as TypesGen from "api/typesGenerated";
import { asRecord, asString } from "components/ai-elements/runtimeTypeUtils";
import { appendTextBlock, asNonEmptyString } from "./blockUtils";
import { appendTextBlock } from "./blockUtils";
import type {
MergedTool,
ParsedMessageContent,
@@ -18,9 +18,6 @@ const appendText = (current: string, next: string): string => {
return `${current}${next}`;
};
export const asOptionalTitle = (value: unknown): string | undefined =>
asNonEmptyString(value);
const isSubagentToolName = (name: string): boolean =>
name === "spawn_agent" || name === "wait_agent" || name === "message_agent";
@@ -72,15 +69,6 @@ const emptyParsedMessageContent = (): ParsedMessageContent => ({
sources: [],
});
/** Wraps appendTextBlock using the same direct concatenation as
* the streaming path so both produce identical markdown. */
const appendParsedTextBlock = (
blocks: RenderBlock[],
type: "response" | "thinking",
text: string,
title?: string,
): RenderBlock[] => appendTextBlock(blocks, type, text, title);
export const ensureToolBlock = (
blocks: RenderBlock[],
id: string,
@@ -140,7 +128,7 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
for (const [index, block] of content.entries()) {
if (typeof block === "string") {
parsed.markdown = appendText(parsed.markdown, block);
parsed.blocks = appendParsedTextBlock(parsed.blocks, "response", block);
parsed.blocks = appendTextBlock(parsed.blocks, "response", block);
continue;
}
@@ -153,23 +141,13 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
case "text": {
const text = asString(typedBlock.text);
parsed.markdown = appendText(parsed.markdown, text);
parsed.blocks = appendParsedTextBlock(
parsed.blocks,
"response",
text,
);
parsed.blocks = appendTextBlock(parsed.blocks, "response", text);
break;
}
case "reasoning": {
const text = asString(typedBlock.text);
const title = asOptionalTitle(typedBlock.title);
parsed.reasoning = appendText(parsed.reasoning, text);
parsed.blocks = appendParsedTextBlock(
parsed.blocks,
"thinking",
text,
title,
);
parsed.blocks = appendTextBlock(parsed.blocks, "thinking", text);
break;
}
case "tool-call": {
@@ -191,18 +169,16 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
break;
}
case "file-reference": {
const text = asString(typedBlock.text);
const fileName = asString(typedBlock.file_name);
const startLine = Number(typedBlock.start_line) || 0;
const endLine = Number(typedBlock.end_line) || startLine;
const contentStr = asString(typedBlock.content);
const content = asString(typedBlock.content);
parsed.blocks.push({
type: "file-reference",
fileName,
startLine,
endLine,
content: contentStr,
text,
file_name: fileName,
start_line: startLine,
end_line: endLine,
content,
});
break;
}
@@ -224,17 +200,18 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
parsed.blocks = ensureToolBlock(parsed.blocks, id);
break;
}
case "file":
if (
typedBlock.media_type &&
(typedBlock.data || typedBlock.file_id)
) {
case "file": {
const mediaType = asString(typedBlock.media_type);
const data = asString(typedBlock.data) || undefined;
const fileId = asString(typedBlock.file_id) || undefined;
if (mediaType && (data || fileId)) {
parsed.blocks = [
...parsed.blocks,
typedBlock as Extract<RenderBlock, { type: "file" }>,
{ type: "file", media_type: mediaType, data, file_id: fileId },
];
}
break;
}
case "source": {
const url = asString(typedBlock.url);
const title = asString(typedBlock.title);
@@ -265,11 +242,7 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
default: {
const text = asString(typedBlock.text);
parsed.markdown = appendText(parsed.markdown, text);
parsed.blocks = appendParsedTextBlock(
parsed.blocks,
"response",
text,
);
parsed.blocks = appendTextBlock(parsed.blocks, "response", text);
break;
}
}
@@ -287,7 +260,7 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
return {
...emptyParsedMessageContent(),
markdown,
blocks: appendParsedTextBlock([], "response", markdown),
blocks: appendTextBlock([], "response", markdown),
};
}
@@ -300,7 +273,7 @@ export const parseMessageContent = (content: unknown): ParsedMessageContent => {
return {
...emptyParsedMessageContent(),
markdown,
blocks: appendParsedTextBlock([], "response", markdown),
blocks: appendTextBlock([], "response", markdown),
};
};
@@ -56,15 +56,14 @@ describe("applyMessagePartToStreamState", () => {
const result = applyMessagePartToStreamState(null, {
type: "reasoning",
text: "Let me reason...",
title: "Analysis",
});
expect(result).not.toBeNull();
expect(result!.blocks).toEqual([
{ type: "thinking", text: "Let me reason...", title: "Analysis" },
{ type: "thinking", text: "Let me reason..." },
]);
});
it("returns prev for reasoning part with no text and no title", () => {
it("returns prev for reasoning part with empty text", () => {
const prev = createEmptyStreamState();
const result = applyMessagePartToStreamState(prev, {
type: "reasoning",
@@ -73,16 +72,6 @@ describe("applyMessagePartToStreamState", () => {
expect(result).toBe(prev);
});
it("returns prev for reasoning part with only title and no text", () => {
const prev = createEmptyStreamState();
const result = applyMessagePartToStreamState(prev, {
type: "reasoning",
text: "",
title: "Some Title",
});
expect(result).toBe(prev);
});
it("creates tool call entry from tool-call part", () => {
const result = applyMessagePartToStreamState(null, {
type: "tool-call",
@@ -330,6 +319,40 @@ describe("applyMessagePartToStreamState", () => {
expect(state).toBe(afterFirst);
expect(state!.sources).toHaveLength(1);
});
it("produces correct tool-result shape with is_error through buildStreamTools", () => {
let state: StreamState | null = null;
state = applyMessagePartToStreamState(state, {
type: "tool-call",
tool_name: "bash",
tool_call_id: "tc-1",
args: { command: "rm -rf /" },
});
state = applyMessagePartToStreamState(state, {
type: "tool-result",
tool_name: "bash",
tool_call_id: "tc-1",
result: { error: "permission denied" },
is_error: true,
});
expect(state).not.toBeNull();
expect(state!.toolResults["tc-1"]).toMatchObject({
id: "tc-1",
name: "bash",
result: { error: "permission denied" },
isError: true,
});
const tools = buildStreamTools(state);
expect(tools).toHaveLength(1);
expect(tools[0]).toEqual({
id: "tc-1",
name: "bash",
args: { command: "rm -rf /" },
result: { error: "permission denied" },
isError: true,
status: "error",
});
});
});
describe("buildStreamTools", () => {
@@ -1,10 +1,6 @@
import { asString } from "components/ai-elements/runtimeTypeUtils";
import { appendTextBlock } from "./blockUtils";
import {
asOptionalTitle,
ensureToolBlock,
parseToolResultIsError,
} from "./messageParsing";
import { ensureToolBlock, parseToolResultIsError } from "./messageParsing";
import { mergeStreamPayload } from "./streamingJson";
import type { MergedTool, RenderBlock, StreamState } from "./types";
@@ -17,9 +13,6 @@ export const createEmptyStreamState = (): StreamState => ({
sources: [],
});
/** Streaming variant — uses direct concatenation (the default joinText). */
const appendStreamTextBlock = appendTextBlock;
export const applyMessagePartToStreamState = (
prev: StreamState | null,
part: Record<string, unknown>,
@@ -35,7 +28,7 @@ export const applyMessagePartToStreamState = (
}
return {
...nextState,
blocks: appendStreamTextBlock(nextState.blocks, "response", text),
blocks: appendTextBlock(nextState.blocks, "response", text),
};
}
case "reasoning": {
@@ -43,15 +36,9 @@ export const applyMessagePartToStreamState = (
if (!text) {
return prev;
}
const title = asOptionalTitle(part.title);
return {
...nextState,
blocks: appendStreamTextBlock(
nextState.blocks,
"thinking",
text,
title,
),
blocks: appendTextBlock(nextState.blocks, "thinking", text),
};
}
case "tool-call": {
@@ -137,17 +124,21 @@ export const applyMessagePartToStreamState = (
},
};
}
case "file":
if (!part.media_type || (!part.data && !part.file_id)) {
case "file": {
const mediaType = asString(part.media_type);
const data = asString(part.data) || undefined;
const fileId = asString(part.file_id) || undefined;
if (!mediaType || (!data && !fileId)) {
return prev;
}
return {
...nextState,
blocks: [
...nextState.blocks,
part as Extract<RenderBlock, { type: "file" }>,
{ type: "file", media_type: mediaType, data, file_id: fileId },
],
};
}
case "source": {
const url = asString(part.url);
const title = asString(part.title);
+2 -15
View File
@@ -30,26 +30,13 @@ export type RenderBlock =
| {
type: "thinking";
text: string;
title?: string;
}
| {
type: "tool";
id: string;
}
| {
type: "file";
media_type: string;
data?: string; // base64, absent when file_id is available
file_id?: string;
}
| {
type: "file-reference";
fileName: string;
startLine: number;
endLine: number;
content: string;
text: string;
}
| TypesGen.ChatFilePart
| TypesGen.ChatFileReferencePart
| {
type: "sources";
sources: Array<{ url: string; title: string }>;