From 8676bd29452f7539622901cb5ae703a3fdaebd07 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Thu, 15 Oct 2026 08:27:26 +0900 Subject: feat(tool-call-cards): pair calls and results in summarized collapsible bubbles --- src/features/chat/ui/ChatView.svelte | 81 ++------------ src/features/chat/ui/ChatView.test.ts | 180 ++++++++++++++++++++++++++++++- src/features/chat/ui/LazyCollapse.svelte | 2 +- src/features/chat/ui/ToolCard.svelte | 111 +++++++++++++++++++ 4 files changed, 296 insertions(+), 78 deletions(-) create mode 100644 src/features/chat/ui/ToolCard.svelte (limited to 'src/features/chat/ui') diff --git a/src/features/chat/ui/ChatView.svelte b/src/features/chat/ui/ChatView.svelte index 5e4513c..18fea7c 100644 --- a/src/features/chat/ui/ChatView.svelte +++ b/src/features/chat/ui/ChatView.svelte @@ -11,6 +11,7 @@ } from "../../../core/metrics"; import { Markdown } from "../../markdown"; import LazyCollapse from "./LazyCollapse.svelte"; + import ToolCard from "./ToolCard.svelte"; const badgeClass = { success: "badge-success", @@ -73,6 +74,7 @@ const rows = $derived(interleaveTurnMetrics(groups, turnMetrics)); + // Tool cards use toolCallId keys across pending/result/seal and orphan page-in. // Stable per-row keys. Thinking blocks get an ordinal key (`think`) that // survives the provisional→committed (seq null → seq N) transition, so the // collapse's open/close state is NOT lost when a turn seals. The ordinal @@ -89,8 +91,8 @@ } const group = row.group; let key: string; - if (group.kind === "tool-batch") { - key = `b${group.stepId}`; + if (group.kind === "tool") { + key = `t${group.entry.toolCallId}`; } else if (group.chunk.chunk.type === "thinking") { key = `think${thinking++}`; } else if (group.chunk.seq != null) { @@ -103,22 +105,6 @@ }); -{#snippet wrench()} - -{/snippet} - {#snippet chunkRow(rendered: RenderedChunk)} {#if rendered.role === "user"} -
-
- {#if rendered.chunk.type === "tool-call"} - {@const call = rendered.chunk} - - {#snippet title()} - {@render wrench()} - {call.toolName} - {/snippet} - 
{JSON.stringify(call.input, null, 2)}
- - {:else} - {@const res = rendered.chunk} - - {#snippet title()} - {@render wrench()} - {res.toolName} - {#if res.isError} - error - {/if} - {/snippet} - 
{res.content}
- - {/if} -
-
{:else} @@ -287,31 +243,10 @@ {:else if row.group.kind === "single"} {@render chunkRow(row.group.chunk)} {:else} - -
-
-
- {#each row.group.entries as entry, entryIndex (entryIndex)} - - {#snippet title()} - {@render wrench()} - {entry.call.toolName} - {#if entry.result?.isError} - error - {:else if entry.result === null} - - {/if} - {/snippet} - 
{JSON.stringify(entry.call.input, null, 2)}
- {#if entry.result} - 
{entry.result.content}
- {/if} - - {/each} -
+ +
+
+
{/if} diff --git a/src/features/chat/ui/ChatView.test.ts b/src/features/chat/ui/ChatView.test.ts index fcac312..d12088b 100644 --- a/src/features/chat/ui/ChatView.test.ts +++ b/src/features/chat/ui/ChatView.test.ts @@ -19,7 +19,7 @@ async function openAllCollapses(container: HTMLElement): Promise { // Svelte `each_key_duplicate` (which aborts the reactive flush and makes the // tab appear dead). See fix-toolcall-duplicate-keys plan Part 1. describe("ChatView — duplicate toolCallIds", () => { - it("renders a tool batch with duplicated toolCallIds without throwing, showing one card per unique call", async () => { + it("renders duplicated toolCallIds without throwing, showing one bubble per unique call", async () => { const chunks: RenderedChunk[] = [ { seq: 1, @@ -96,7 +96,7 @@ describe("ChatView — duplicate toolCallIds", () => { }, provisional: false, }, - // Duplicate results too — absorbed into the single deduped batch. + // Duplicate results too — absorbed into the corresponding cards. { seq: 7, role: "tool", @@ -128,10 +128,11 @@ describe("ChatView — duplicate toolCallIds", () => { const { container } = render(ChatView, { props: { chunks } }); await openAllCollapses(container); - // No each_key_duplicate crash. Batched calls render as collapsible cards — + // No each_key_duplicate crash. Calls render as independent collapsible cards — // exactly ONE per unique toolCallId (deduped), not 4. const collapses = container.querySelectorAll(".collapse"); expect(collapses).toHaveLength(2); + expect(container.querySelectorAll(".chat-bubble")).toHaveLength(2); // Both unique tool names shown; each result rendered once (deduped). expect(screen.getByText("read_file")).toBeInTheDocument(); @@ -161,7 +162,7 @@ describe("ChatView — lazy collapse bodies", () => { expect(screen.getByText("run_shell")).toBeInTheDocument(); expect(screen.queryByText("HUGE-OUTPUT")).toBeNull(); - const box = screen.getByRole("checkbox", { name: "Toggle tool result" }); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); await userEvent.click(box); expect(screen.getByText("HUGE-OUTPUT")).toBeInTheDocument(); @@ -170,3 +171,174 @@ describe("ChatView — lazy collapse bodies", () => { expect(container.querySelectorAll(".chat-row")).toHaveLength(1); }); }); + +function shellChunks(content: string | null, isError = false, committed = false): RenderedChunk[] { + const call: RenderedChunk = { + seq: committed ? 1 : null, + role: "assistant", + provisional: !committed, + chunk: { + type: "tool-call", + toolCallId: "shell-1", + toolName: "run_shell", + input: { command: "bun run test" }, + }, + }; + return content === null + ? [call] + : [ + call, + { + seq: committed ? 2 : null, + role: "tool", + provisional: !committed, + chunk: { + type: "tool-result", + toolCallId: "shell-1", + toolName: "run_shell", + content, + isError, + }, + }, + ]; +} + +describe("ChatView — tool cards", () => { + it("pairs a single call/result into one collapsed bubble with both summaries and a success icon", async () => { + const { container } = render(ChatView, { props: { chunks: shellChunks("first\nsecond") } }); + expect(container.querySelectorAll(".chat-bubble")).toHaveLength(1); + expect(container.querySelectorAll(".collapse")).toHaveLength(1); + expect(screen.getByText("bun run test")).toBeInTheDocument(); + expect(screen.getByText("→ 2 lines")).toBeInTheDocument(); + expect(screen.getByLabelText("Succeeded")).toHaveTextContent("✓"); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); + expect(box).not.toBeChecked(); + expect(container.querySelector("pre")).toBeNull(); + await userEvent.click(box); + expect(screen.getByText("$ bun run test")).toBeInTheDocument(); + expect(screen.getByText("first second")).toBeInTheDocument(); + expect(screen.queryByRole("checkbox", { name: /tool result/ })).toBeNull(); + }); + it("transitions pending → result → sealed without losing open state", async () => { + const { container, rerender } = render(ChatView, { props: { chunks: shellChunks(null) } }); + expect(screen.getByLabelText("Running")).toHaveClass("loading-spinner"); + expect(container.querySelector(".collapse-title")).not.toHaveTextContent("→"); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); + await userEvent.click(box); + expect(screen.getByText("running…")).toBeInTheDocument(); + await rerender({ chunks: shellChunks("done") }); + expect(screen.queryByLabelText("Running")).toBeNull(); + expect(screen.getByLabelText("Succeeded")).toBeInTheDocument(); + expect(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })).toBe(box); + expect(box).toBeChecked(); + await rerender({ chunks: shellChunks("done", false, true) }); + expect(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })).toBe(box); + expect(box).toBeChecked(); + expect(screen.getByText("done")).toBeInTheDocument(); + }); + it("keeps errors collapsed with a failure icon and tints the expanded output", async () => { + const { container } = render(ChatView, { + props: { chunks: shellChunks("Permission denied", true) }, + }); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); + expect(box).not.toBeChecked(); + expect(screen.getByLabelText("Failed")).toHaveClass("text-error"); + expect(screen.getByLabelText("Failed")).toHaveTextContent("✗"); + expect(screen.getByText("→ failed · Permission denied")).toHaveClass("text-error"); + expect(container.querySelector("pre")).toBeNull(); + await userEvent.click(box); + expect(screen.getByText("Permission denied").parentElement).toHaveClass("text-error"); + }); + it("renders an orphan result as the same card and retains state when its call is paged back in", async () => { + const chunks = shellChunks("contents", false, true); + const { container, rerender } = render(ChatView, { props: { chunks: chunks.slice(1) } }); + expect(container.querySelectorAll(".chat-bubble")).toHaveLength(1); + expect(screen.getByText("call unavailable")).toBeInTheDocument(); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); + await userEvent.click(box); + expect(screen.getByText(/Call unavailable \(earlier history unloaded\)/)).toBeInTheDocument(); + expect(screen.getByText("contents")).toBeInTheDocument(); + await rerender({ chunks }); + expect(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })).toBe(box); + expect(box).toBeChecked(); + expect(screen.getByText("$ bun run test")).toBeInTheDocument(); + }); + it("folds 13 output lines as 6 + toggle + 6, expands and collapses without closing the card", async () => { + const output = Array.from({ length: 13 }, (_, i) => `output-${i + 1}`).join("\n"); + const { rerender } = render(ChatView, { props: { chunks: shellChunks(output) } }); + const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); + await userEvent.click(box); + const response = screen.getByRole("region", { name: "Response" }); + expect(response).toHaveTextContent("output-1"); + expect(response).toHaveTextContent("output-6"); + expect(response).not.toHaveTextContent("output-7"); + expect(response).toHaveTextContent("output-8"); + expect(response).toHaveTextContent("output-13"); + const toggle = screen.getByRole("button", { name: "⋯ 1 more lines" }); + expect(toggle).toHaveAttribute("aria-expanded", "false"); + await userEvent.click(toggle); + expect(response).toHaveTextContent("output-7"); + expect(response.querySelectorAll("pre")).toHaveLength(1); + expect(response.querySelector("pre")?.textContent).toBe(output); + expect(box).toBeChecked(); + expect(screen.getByRole("button", { name: "Show fewer lines" })).toHaveAttribute( + "aria-expanded", + "true", + ); + await userEvent.click(screen.getByRole("button", { name: "Show fewer lines" })); + expect(response).not.toHaveTextContent("output-7"); + expect(box).toBeChecked(); + // The separate output-fold state also survives sealing. + await userEvent.click(screen.getByRole("button", { name: "⋯ 1 more lines" })); + expect(response).toHaveTextContent("output-7"); + await rerender({ chunks: shellChunks(output, false, true) }); + expect(box).toBeChecked(); + expect(screen.getByRole("button", { name: "Show fewer lines" })).toHaveAttribute( + "aria-expanded", + "true", + ); + expect(response).toHaveTextContent("output-7"); + }); + it("shows 12 output lines without an inner toggle", async () => { + const output = Array.from({ length: 12 }, (_, i) => `output-${i + 1}`).join("\n"); + render(ChatView, { props: { chunks: shellChunks(output) } }); + await userEvent.click(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })); + expect(screen.queryByRole("button", { name: /more lines|fewer lines/ })).toBeNull(); + expect(screen.getByRole("region", { name: "Response" })).toHaveTextContent("output-12"); + }); + it("renders edit details and treats tool output as text, not executable markup", async () => { + const chunks: RenderedChunk[] = [ + { + seq: 1, + role: "assistant", + provisional: false, + chunk: { + type: "tool-call", + toolCallId: "edit", + toolName: "edit_file", + input: { path: "a.ts", oldString: "before", newString: "after\nsecond" }, + }, + }, + { + seq: 2, + role: "tool", + provisional: false, + chunk: { + type: "tool-result", + toolCallId: "edit", + toolName: "edit_file", + content: 'Replaced 1 occurrence(s) in "a.ts".\n', + isError: false, + }, + }, + ]; + const { container } = render(ChatView, { props: { chunks } }); + expect(screen.getByText("+2 −1")).toBeInTheDocument(); + expect(screen.getByText("→ 1 replacement · diagnostics")).toBeInTheDocument(); + await userEvent.click(screen.getByRole("checkbox", { name: "Toggle tool call edit_file" })); + expect(screen.getByText("before")).toHaveClass("text-error"); + expect(screen.getByText("after second")).toHaveClass("text-success"); + expect(container.querySelector("script")).toBeNull(); + expect(container).toHaveTextContent(""); + }); +}); diff --git a/src/features/chat/ui/LazyCollapse.svelte b/src/features/chat/ui/LazyCollapse.svelte index 0f66451..912adb4 100644 --- a/src/features/chat/ui/LazyCollapse.svelte +++ b/src/features/chat/ui/LazyCollapse.svelte @@ -33,7 +33,7 @@
-
+
{@render title()}
diff --git a/src/features/chat/ui/ToolCard.svelte b/src/features/chat/ui/ToolCard.svelte new file mode 100644 index 0000000..8c6e325 --- /dev/null +++ b/src/features/chat/ui/ToolCard.svelte @@ -0,0 +1,111 @@ + + + + {#snippet title()} + {#if card.status === "pending"} + + {:else if card.status === "error"} + ✗ + {:else} + ✓ + {/if} + + {card.name} + {card.callSummary} + {#each card.callBadges as badge} + {badge} + {/each} + {#if card.resultSummary !== null} + + → {card.resultSummary} + + {/if} + {/snippet} +
+
+
Call
+ {#if card.callDetail.kind === "command"} + 
$ {card.callDetail.text}
+ {:else if card.callDetail.kind === "edit"} +
{card.callDetail.path}
+
+
− old
+ 
{card.callDetail.oldText}
+
+ new
+ 
{card.callDetail.newText}
+
+ {:else} +
+ {#each card.callDetail.fields as field} +
+
{field.key}
+
+ 
{field.value}
+
+
+ {/each} +
+ {/if} +
+
+
Response
+ {#if card.output === null} + running… + {:else} +
+ {#if card.output.kind === "full"} + 
{card.output.lines.join("\n") || "(no output)"}
+ {:else} + {#if showAll} + 
{card.output.all.join("\n")}
+ {:else} + 
{card.output.head.join("\n")}
+ {/if} + + {#if !showAll} + 
{card.output.tail.join("\n")}
+ {/if} + {/if} +
+ {/if} +
+
+ -- cgit v1.2.3