Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mafredri merged 1 commit into from
Mar 18, 2026
Merged

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 4 additions & 15 deletions site/src/pages/AgentsPage/AgentDetail.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down Expand Up @@ -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();
},
Expand Down Expand Up @@ -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(
{
Expand All @@ -1015,7 +1014,6 @@ export const StreamedReasoningCollapsed: Story = {
message_part: {
part: {
type: "reasoning",
title: "Plan migration",
text: "Streaming reasoning body",
},
},
Expand All @@ -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();
Expand Down
75 changes: 15 additions & 60 deletions site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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,
Expand All @@ -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
Expand All @@ -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}
Comment on lines 85 to -117
Copy link
Copy Markdown
Contributor

@DanielleMaywood DanielleMaywood Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Assuming you've verified we haven't lost any useful functionality here, I'm happy

Copy link
Copy Markdown
Member Author

@mafredri mafredri Mar 18, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't find any issues, this should all be dead code (e.g. reasoning title never set).

<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>
);
};
Expand Down Expand Up @@ -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}
Expand All @@ -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": {
Expand Down Expand Up @@ -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}
/>
),
)
Expand Down
67 changes: 5 additions & 62 deletions site/src/pages/AgentsPage/AgentDetail/blockUtils.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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", () => {
Expand All @@ -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",
});
});

Expand All @@ -96,7 +78,6 @@ describe("appendTextBlock", () => {
expect(result[1]).toEqual({
type: "thinking",
text: "hmm",
title: undefined,
});
});

Expand All @@ -107,49 +88,11 @@ 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");
expect(blocks).toHaveLength(1);
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,
});
});
});
25 changes: 4 additions & 21 deletions site/src/pages/AgentsPage/AgentDetail/blockUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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;
Expand All @@ -16,34 +9,24 @@ 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;
}
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;
};
Loading
Loading