From d594de6934a4c5f20079b86f7a76b7f8ad0cb512 Mon Sep 17 00:00:00 2001 From: Adam Malczewski Date: Thu, 15 Oct 2026 08:59:06 +0900 Subject: feat(tool-call-cards): final visual polish — unified edit diff, quiet failed/orphan/pending headers MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- src/features/chat/logic/tool-summary.test.ts | 14 +++++- src/features/chat/logic/tool-summary.ts | 4 +- src/features/chat/ui/ChatView.test.ts | 61 +++++++++++++++++++++--- src/features/chat/ui/ToolCard.svelte | 71 +++++++++++++++++++++++----- 4 files changed, 128 insertions(+), 22 deletions(-) (limited to 'src') diff --git a/src/features/chat/logic/tool-summary.test.ts b/src/features/chat/logic/tool-summary.test.ts index 381842a..876ede2 100644 --- a/src/features/chat/logic/tool-summary.test.ts +++ b/src/features/chat/logic/tool-summary.test.ts @@ -85,6 +85,18 @@ describe("tool summaries", () => { ]); expect(view("edit_file", { path: "a" }).callBadges).toEqual([]); }); + it("omits edit diffstats after failure while retaining attempted edits in call details", () => { + const input = { path: "src/missing.ts", oldString: "foo", newString: "bar" }; + const card = view("edit_file", input, "Error: oldString not found", true); + expect(card.callBadges).toEqual([]); + expect(card.callDetail).toEqual({ + kind: "edit", + path: input.path, + oldText: "foo", + newText: "bar", + }); + expect(card.resultSummary).toBe("failed"); + }); it("preserves exact shell commands in the expanded detail", () => { expect(view("run_shell", { command: "ls\n pwd" }).callDetail).toEqual({ kind: "command", @@ -228,7 +240,7 @@ describe("tool summaries", () => { }); expect(viewToolEntry(e).callDetail).toEqual({ kind: "unavailable", - text: "The call is in earlier, unloaded history. Its response is shown below.", + text: "This call is earlier in the conversation. Show earlier messages to see it.", }); }); }); diff --git a/src/features/chat/logic/tool-summary.ts b/src/features/chat/logic/tool-summary.ts index 2e7680c..3ea4a8e 100644 --- a/src/features/chat/logic/tool-summary.ts +++ b/src/features/chat/logic/tool-summary.ts @@ -251,7 +251,7 @@ export function viewToolEntry(entry: ToolEntry): ToolCardView { ? "Call in earlier history" : oneLine(builtInSummary || generic || serialize(input)); const callBadges: ToolCardView["callBadges"][number][] = []; - if (entry.toolName === "edit_file" && entry.call !== null) { + if (entry.toolName === "edit_file" && entry.call !== null && !entry.result?.isError) { if (typeof args.oldString === "string" && typeof args.newString === "string") { callBadges.push( { label: `+${lineCount(args.newString)}`, tone: "success" }, @@ -272,7 +272,7 @@ export function viewToolEntry(entry: ToolEntry): ToolCardView { entry.call === null ? { kind: "unavailable", - text: "The call is in earlier, unloaded history. Its response is shown below.", + text: "This call is earlier in the conversation. Show earlier messages to see it.", } : detail(entry.toolName, input, args), resultSummary, diff --git a/src/features/chat/ui/ChatView.test.ts b/src/features/chat/ui/ChatView.test.ts index 412bf2d..9ac4986 100644 --- a/src/features/chat/ui/ChatView.test.ts +++ b/src/features/chat/ui/ChatView.test.ts @@ -220,14 +220,15 @@ describe("ChatView — tool cards", () => { 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) } }); + const { rerender } = render(ChatView, { props: { chunks: shellChunks(null) } }); expect(screen.getByLabelText("Running")).toHaveClass("loading-spinner"); - expect(container.querySelector(".collapse-title")).not.toHaveTextContent("→"); + expect(screen.getByText("→ running…")).toHaveClass("tool-result", "text-base-content/60"); 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.queryByText("→ running…")).toBeNull(); expect(screen.getByLabelText("Succeeded")).toBeInTheDocument(); expect(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })).toBe(box); expect(box).toBeChecked(); @@ -258,7 +259,11 @@ describe("ChatView — tool cards", () => { expect(screen.getByLabelText("Succeeded")).toHaveClass("opacity-60"); const box = screen.getByRole("checkbox", { name: "Toggle tool call run_shell" }); await userEvent.click(box); - expect(screen.getByText(/The call is in earlier, unloaded history/)).toBeInTheDocument(); + expect(screen.getByRole("note")).toHaveTextContent( + "This call is earlier in the conversation. Show earlier messages to see it.", + ); + expect(screen.getByRole("note")).toHaveClass("italic", "text-base-content/60"); + expect(screen.queryByRole("heading", { name: "Call" })).toBeNull(); expect(screen.getByText("contents")).toBeInTheDocument(); await rerender({ chunks }); expect(screen.getByRole("checkbox", { name: "Toggle tool call run_shell" })).toBe(box); @@ -283,7 +288,7 @@ describe("ChatView — tool cards", () => { expect(response.querySelectorAll("pre")).toHaveLength(1); expect(response.querySelector("pre")?.textContent).toBe(output); const fewer = screen.getByRole("button", { name: "Show fewer lines" }); - expect(fewer).toHaveClass("btn-primary", "btn-sm"); + expect(fewer).toHaveClass("btn-soft", "btn-xs"); expect(fewer.parentElement).toHaveClass("sticky-controls"); const fullOutput = response.querySelector("pre"); if (fullOutput === null) throw new Error("expected full output"); @@ -345,8 +350,12 @@ describe("ChatView — tool cards", () => { expect(screen.getByText("−1")).toHaveClass("text-error"); 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"); + // Unified diff: one tinted row per line, each with its own −/+ gutter marker. + const diff = screen.getByRole("group", { name: "Edit diff" }); + const removed = diff.querySelectorAll(".edit-diff-del"); + const added = diff.querySelectorAll(".edit-diff-add"); + expect(Array.from(removed, (r) => r.textContent?.trim())).toEqual(["−before"]); + expect(Array.from(added, (r) => r.textContent?.trim())).toEqual(["+after", "+second"]); expect(container.querySelector("script")).toBeNull(); expect(container).toHaveTextContent(""); }); @@ -389,3 +398,43 @@ it("renders compact, aligned generic fields with legible headings and inline sca expect(container.querySelectorAll("dd")).toHaveLength(5); expect(screen.getByText("options").nextElementSibling).toHaveTextContent('"nested": true'); }); + +it("removes a pending edit's diffstat when it fails, without hiding the attempted edit", async () => { + const call: RenderedChunk = { + seq: null, + role: "assistant", + provisional: true, + chunk: { + type: "tool-call", + toolCallId: "failed-edit", + toolName: "edit_file", + input: { path: "src/missing.ts", oldString: "foo", newString: "bar" }, + }, + }; + const result: RenderedChunk = { + seq: null, + role: "tool", + provisional: true, + chunk: { + type: "tool-result", + toolCallId: "failed-edit", + toolName: "edit_file", + content: "Error: oldString not found", + isError: true, + }, + }; + const { container, rerender } = render(ChatView, { props: { chunks: [call] } }); + expect(screen.getByText("+1")).toBeInTheDocument(); + expect(screen.getByText("−1")).toBeInTheDocument(); + expect(screen.getByText("→ running…")).toBeInTheDocument(); + await userEvent.click(screen.getByRole("checkbox", { name: "Toggle tool call edit_file" })); + await rerender({ chunks: [call, result] }); + expect(screen.queryByText("+1")).toBeNull(); + expect(screen.queryByText("−1")).toBeNull(); + expect(screen.queryByText("→ running…")).toBeNull(); + expect(container.querySelector(".tool-header")).toHaveTextContent("src/missing.ts"); + expect(container.querySelector(".tool-header")).toHaveTextContent("→ failed"); + expect(screen.getByText("foo")).toBeInTheDocument(); + expect(screen.getByText("bar")).toBeInTheDocument(); + expect(screen.getByText("Error: oldString not found")).toBeInTheDocument(); +}); diff --git a/src/features/chat/ui/ToolCard.svelte b/src/features/chat/ui/ToolCard.svelte index f1a56cb..75c562d 100644 --- a/src/features/chat/ui/ToolCard.svelte +++ b/src/features/chat/ui/ToolCard.svelte @@ -67,27 +67,48 @@ → {card.compactResultSummary} {/if} + {:else} + → running… {/if} {/snippet}
-

- Call -

{#if card.callDetail.kind === "unavailable"} -

{card.callDetail.text}

- {:else if card.callDetail.kind === "command"} + + {:else} +

+ Call +

+ {/if} + {#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}
+ +
+ {#if card.callDetail.oldText !== ""} + {#each card.callDetail.oldText.split("\n") as line, i (i)} +
+ −{line} +
+ {/each} + {/if} + {#if card.callDetail.newText !== ""} + {#each card.callDetail.newText.split("\n") as line, i (i)} +
+ +{line} +
+ {/each} + {/if}
- {:else} + {:else if card.callDetail.kind === "fields"}
{#each card.callDetail.fields as field}
{field.key}
@@ -117,13 +138,13 @@ {/if}
@@ -188,6 +209,30 @@ max-width: 10rem; overflow-wrap: anywhere; } + .edit-diff { + background: var(--color-base-300); + padding-block: 0.25rem; + } + .edit-diff-row { + display: flex; + min-width: max-content; + white-space: pre; + padding-inline-end: 0.75rem; + } + .edit-diff-sign { + flex-shrink: 0; + width: 1.75rem; + text-align: center; + user-select: none; + } + .edit-diff-del { + color: var(--color-error); + background: color-mix(in oklab, var(--color-error) 12%, transparent); + } + .edit-diff-add { + color: var(--color-success); + background: color-mix(in oklab, var(--color-success) 12%, transparent); + } .output-controls { padding-block: 0.5rem; color: var(--color-base-content); -- cgit v1.2.3