Skip to content

Commit 488ceb6

Browse files
mafredrimafredri
authored
refactor(site/src/pages/AgentsPage): clean up RenderBlock types and dead fields (coder#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 coder#23168
1 parent 481c132 commit 488ceb6

9 files changed

Lines changed: 106 additions & 267 deletions

File tree

‎site/src/pages/AgentsPage/AgentDetail.stories.tsx‎

Lines changed: 4 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -653,7 +653,7 @@ export const WithSubagentCards: Story = {
653653
},
654654
};
655655

656-
/** Reasoning part without title renders inline (no disclosure). */
656+
/** Completed reasoning part renders inline. */
657657
export const WithReasoningInline: Story = {
658658
parameters: {
659659
queries: buildQueries(
@@ -687,7 +687,7 @@ export const WithReasoningInline: Story = {
687687
play: async ({ canvasElement }) => {
688688
const canvas = within(canvasElement);
689689

690-
// Reasoning text renders inline, not behind a disclosure.
690+
// Reasoning text renders inline.
691691
expect(canvas.getByText("Reasoning body")).toBeInTheDocument();
692692
expect(canvas.queryByRole("button", { name: "Thinking" })).toBeNull();
693693
},
@@ -991,10 +991,9 @@ export const SidebarWithSingleRepo: Story = {
991991
},
992992
};
993993
/**
994-
* Streaming reasoning part via WebSocket — renders collapsed and
995-
* can be expanded on click.
994+
* Streaming reasoning part via WebSocket — renders inline text.
996995
*/
997-
export const StreamedReasoningCollapsed: Story = {
996+
export const StreamedReasoning: Story = {
998997
parameters: {
999998
queries: buildQueries(
1000999
{
@@ -1015,7 +1014,6 @@ export const StreamedReasoningCollapsed: Story = {
10151014
message_part: {
10161015
part: {
10171016
type: "reasoning",
1018-
title: "Plan migration",
10191017
text: "Streaming reasoning body",
10201018
},
10211019
},
@@ -1026,16 +1024,7 @@ export const StreamedReasoningCollapsed: Story = {
10261024
},
10271025
play: async ({ canvasElement }) => {
10281026
const canvas = within(canvasElement);
1029-
const user = userEvent.setup();
1030-
1031-
const reasoningToggle = await canvas.findByRole("button", {
1032-
name: "Plan migration",
1033-
});
1034-
expect(reasoningToggle).toHaveAttribute("aria-expanded", "false");
1035-
1036-
await user.click(reasoningToggle);
10371027

1038-
expect(reasoningToggle).toHaveAttribute("aria-expanded", "true");
10391028
await expect(
10401029
canvas.findByText("Streaming reasoning body"),
10411030
).resolves.toBeInTheDocument();

‎site/src/pages/AgentsPage/AgentDetail/ConversationTimeline.tsx‎

Lines changed: 15 additions & 60 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ import {
1717
TooltipContent,
1818
TooltipTrigger,
1919
} from "components/Tooltip/Tooltip";
20-
import { ChevronDownIcon, PencilIcon } from "lucide-react";
20+
import { PencilIcon } from "lucide-react";
2121
import {
2222
type FC,
2323
Fragment,
@@ -43,12 +43,10 @@ import type {
4343

4444
const ReasoningDisclosure: FC<{
4545
id: string;
46-
title?: string;
4746
text: string;
4847
isStreaming?: boolean;
4948
urlTransform?: UrlTransform;
50-
}> = ({ id, title, text, isStreaming = false, urlTransform }) => {
51-
const [isOpen, setIsOpen] = useState(false);
49+
}> = ({ id, text, isStreaming = false, urlTransform }) => {
5250
const { visibleText } = useSmoothStreamingText({
5351
fullText: text,
5452
isStreaming,
@@ -57,10 +55,8 @@ const ReasoningDisclosure: FC<{
5755
});
5856
const displayText = isStreaming ? visibleText : text;
5957
const hasText = displayText.trim().length > 0;
60-
const label = title ?? "Thinking";
61-
const showStreamingPlaceholder = isStreaming && !hasText;
6258

63-
if (!title && hasText) {
59+
if (hasText) {
6460
return (
6561
<div className="w-full">
6662
<Response
@@ -72,49 +68,14 @@ const ReasoningDisclosure: FC<{
7268
</div>
7369
);
7470
}
75-
const labelContent = (
76-
<span className="text-sm">
77-
{showStreamingPlaceholder ? (
78-
<Shimmer as="span">Thinking...</Shimmer>
79-
) : (
80-
label
81-
)}
82-
</span>
83-
);
8471

8572
return (
8673
<div className="w-full">
87-
{hasText ? (
88-
<button
89-
type="button"
90-
aria-expanded={isOpen}
91-
aria-controls={id}
92-
className="flex items-center gap-2 bg-transparent border-0 p-0 text-content-secondary transition-colors hover:text-content-primary cursor-pointer"
93-
onClick={() => setIsOpen((prev) => !prev)}
94-
>
95-
{labelContent}
96-
<ChevronDownIcon
97-
className={cn(
98-
"h-3 w-3 shrink-0 text-content-secondary transition-transform",
99-
isOpen ? "rotate-0" : "-rotate-90",
100-
)}
101-
/>
102-
</button>
103-
) : (
104-
<div className="flex items-center gap-2 text-content-secondary transition-colors hover:text-content-primary">
105-
{labelContent}
106-
</div>
107-
)}
108-
{isOpen && hasText ? (
109-
<div id={id} className="mt-1.5">
110-
<Response
111-
className="text-[11px] text-content-secondary"
112-
urlTransform={urlTransform}
113-
>
114-
{displayText}
115-
</Response>
116-
</div>
117-
) : null}
74+
<div className="flex items-center gap-2 text-content-secondary transition-colors hover:text-content-primary">
75+
<span className="text-sm">
76+
{isStreaming ? <Shimmer as="span">Thinking...</Shimmer> : "Thinking"}
77+
</span>
78+
</div>
11879
</div>
11980
);
12081
};
@@ -190,7 +151,6 @@ function renderBlockList({
190151
<ReasoningDisclosure
191152
key={`${keyPrefix}-thinking-${index}`}
192153
id={`${keyPrefix}-thinking-${index}`}
193-
title={block.title}
194154
text={block.text}
195155
isStreaming={isStreaming}
196156
urlTransform={urlTransform}
@@ -203,16 +163,11 @@ function renderBlockList({
203163
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"
204164
>
205165
<span className="shrink-0 text-xs font-medium text-content-link">
206-
{block.fileName}:
207-
{block.startLine === block.endLine
208-
? block.startLine
209-
: `${block.startLine}\u2013${block.endLine}`}
166+
{block.file_name}:
167+
{block.start_line === block.end_line
168+
? block.start_line
169+
: `${block.start_line}\u2013${block.end_line}`}
210170
</span>
211-
{block.text && (
212-
<span className="text-sm text-content-primary">
213-
{block.text}
214-
</span>
215-
)}
216171
</div>
217172
);
218173
case "tool": {
@@ -408,9 +363,9 @@ const ChatMessageItem = memo<{
408363
) : (
409364
<FileReferenceChip
410365
key={i}
411-
fileName={block.fileName}
412-
startLine={block.startLine}
413-
endLine={block.endLine}
366+
fileName={block.file_name}
367+
startLine={block.start_line}
368+
endLine={block.end_line}
414369
/>
415370
),
416371
)

‎site/src/pages/AgentsPage/AgentDetail/blockUtils.test.ts‎

Lines changed: 5 additions & 62 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,8 @@ describe("appendTextBlock", () => {
5050
});
5151

5252
it("appends a new thinking block to an empty list", () => {
53-
const result = appendTextBlock([], "thinking", "pondering", "Deep thought");
54-
expect(result).toEqual([
55-
{ type: "thinking", text: "pondering", title: "Deep thought" },
56-
]);
53+
const result = appendTextBlock([], "thinking", "pondering");
54+
expect(result).toEqual([{ type: "thinking", text: "pondering" }]);
5755
});
5856

5957
it("merges consecutive response blocks", () => {
@@ -63,29 +61,13 @@ describe("appendTextBlock", () => {
6361
expect(result[0]).toEqual({ type: "response", text: "aaabbb" });
6462
});
6563

66-
it("merges consecutive thinking blocks with compatible titles", () => {
67-
const blocks: RenderBlock[] = [
68-
{ type: "thinking", text: "part1", title: "Reasoning" },
69-
];
70-
const result = appendTextBlock(blocks, "thinking", "part2", "Reasoning");
64+
it("merges consecutive thinking blocks", () => {
65+
const blocks: RenderBlock[] = [{ type: "thinking", text: "part1" }];
66+
const result = appendTextBlock(blocks, "thinking", "part2");
7167
expect(result).toHaveLength(1);
7268
expect(result[0]).toEqual({
7369
type: "thinking",
7470
text: "part1part2",
75-
title: "Reasoning",
76-
});
77-
});
78-
79-
it("merges thinking blocks with different titles using the new title", () => {
80-
const blocks: RenderBlock[] = [
81-
{ type: "thinking", text: "part1", title: "Analyzing" },
82-
];
83-
const result = appendTextBlock(blocks, "thinking", "part2", "Planning");
84-
expect(result).toHaveLength(1);
85-
expect(result[0]).toEqual({
86-
type: "thinking",
87-
text: "part1part2",
88-
title: "Planning",
8971
});
9072
});
9173

@@ -96,7 +78,6 @@ describe("appendTextBlock", () => {
9678
expect(result[1]).toEqual({
9779
type: "thinking",
9880
text: "hmm",
99-
title: undefined,
10081
});
10182
});
10283

@@ -107,49 +88,11 @@ describe("appendTextBlock", () => {
10788
expect(result[1]).toEqual({ type: "response", text: "after tool" });
10889
});
10990

110-
it("uses the custom joinText function when merging", () => {
111-
const blocks: RenderBlock[] = [{ type: "response", text: "line1" }];
112-
const join = (a: string, b: string) => `${a}\n${b}`;
113-
const result = appendTextBlock(
114-
blocks,
115-
"response",
116-
"line2",
117-
undefined,
118-
join,
119-
);
120-
expect(result).toHaveLength(1);
121-
expect(result[0]).toEqual({ type: "response", text: "line1\nline2" });
122-
});
123-
12491
it("does not mutate the original blocks array", () => {
12592
const blocks: RenderBlock[] = [{ type: "response", text: "original" }];
12693
const result = appendTextBlock(blocks, "response", " added");
12794
expect(blocks).toHaveLength(1);
12895
expect((blocks[0] as { text: string }).text).toBe("original");
12996
expect(result).not.toBe(blocks);
13097
});
131-
132-
it("merges thinking block and uses new title", () => {
133-
const blocks: RenderBlock[] = [
134-
{ type: "thinking", text: "a", title: "Think" },
135-
];
136-
const result = appendTextBlock(blocks, "thinking", "b", "Thinking deeply");
137-
expect(result).toHaveLength(1);
138-
expect(result[0]).toEqual({
139-
type: "thinking",
140-
text: "ab",
141-
title: "Thinking deeply",
142-
});
143-
});
144-
145-
it("merges thinking blocks when both have no title", () => {
146-
const blocks: RenderBlock[] = [{ type: "thinking", text: "a" }];
147-
const result = appendTextBlock(blocks, "thinking", "b");
148-
expect(result).toHaveLength(1);
149-
expect(result[0]).toEqual({
150-
type: "thinking",
151-
text: "ab",
152-
title: undefined,
153-
});
154-
});
15598
});

‎site/src/pages/AgentsPage/AgentDetail/blockUtils.ts‎

Lines changed: 4 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,6 @@
11
import { asString } from "components/ai-elements/runtimeTypeUtils";
22
import type { RenderBlock } from "./types";
33

4-
const createBlock = (
5-
type: "response" | "thinking",
6-
text: string,
7-
title?: string,
8-
): RenderBlock =>
9-
type === "thinking" ? { type, text, title } : { type, text };
10-
114
export const asNonEmptyString = (value: unknown): string | undefined => {
125
const next = asString(value).trim();
136
return next.length > 0 ? next : undefined;
@@ -16,34 +9,24 @@ export const asNonEmptyString = (value: unknown): string | undefined => {
169
/**
1710
* Append a text or thinking block to a render block list, merging
1811
* with the previous block when the types match.
19-
*
20-
* @param joinText Controls how existing and new text are concatenated
21-
* when merging into an existing block. Callers that process
22-
* complete message blocks typically join with a newline, while
23-
* streaming callers concatenate directly.
2412
*/
2513
export const appendTextBlock = (
2614
blocks: RenderBlock[],
2715
type: "response" | "thinking",
2816
text: string,
29-
title?: string,
30-
joinText: (current: string, next: string) => string = (a, b) => `${a}${b}`,
3117
): RenderBlock[] => {
3218
if (!text.trim()) {
3319
return blocks;
3420
}
3521
const nextBlocks = [...blocks];
3622
const last = nextBlocks[nextBlocks.length - 1];
3723
if (last && last.type === type) {
38-
nextBlocks[nextBlocks.length - 1] = createBlock(
24+
nextBlocks[nextBlocks.length - 1] = {
3925
type,
40-
joinText(last.text, text),
41-
type === "thinking" && last.type === "thinking"
42-
? (title ?? last.title)
43-
: undefined,
44-
);
26+
text: `${last.text}${text}`,
27+
};
4528
return nextBlocks;
4629
}
47-
nextBlocks.push(createBlock(type, text, title));
30+
nextBlocks.push({ type, text });
4831
return nextBlocks;
4932
};

0 commit comments

Comments
 (0)