diff options
| -rw-r--r-- | backend-handoff.md | 22 | ||||
| -rw-r--r-- | biome.json | 2 | ||||
| -rw-r--r-- | src/features/chat/logic/tool-summary.test.ts | 14 | ||||
| -rw-r--r-- | src/features/chat/logic/tool-summary.ts | 4 | ||||
| -rw-r--r-- | src/features/chat/ui/ChatView.test.ts | 61 | ||||
| -rw-r--r-- | src/features/chat/ui/ToolCard.svelte | 71 |
6 files changed, 150 insertions, 24 deletions
diff --git a/backend-handoff.md b/backend-handoff.md index 4c46e7d..1109f8b 100644 --- a/backend-handoff.md +++ b/backend-handoff.md @@ -18,6 +18,28 @@ > ?archived=`) was made in the same branch and the FE consumes it via the `file:` deps (dists > rebuilt). Recorded below. +## Milestone: Tool-call cards final visual follow-up (`feature/tool-call-cards`, 2026-10-14) + +**Slice status:** COMPLETE (frontend-only). + +- Failed `edit_file` entries no longer carry header diffstats; attempted old/new content remains + available in the expanded Call section. Regression covers pending edit → failed result. +- Orphan explanation is a muted italic note, without a Call heading: “This call is earlier in the + conversation. Show earlier messages to see it.” No storage jargon in the user-facing copy. +- Pending headers now show muted `→ running…` on the right; it is replaced by the result on arrival. +- `edit_file` call detail is now a unified diff: one tinted row per line with its own −/+ gutter + marker (classes `edit-diff*` — NOT `.diff`, which collides with DaisyUI 5's `diff` component). + The output toggle is a neutral `btn-soft btn-xs`; the ↑ glyph shows only on "Show fewer lines". +- Reverted the `!scratch` / `!reports` Biome exclusions at the user's request. Moved only our + generated TS/JSON probe artifacts from gitignored reports to `/tmp/tool-call-polish-artifacts`; + the temporary harness has since been deleted. +- Verification: typecheck **0 errors/0 warnings**; full tests **1,931 passed ×2** (111 files); + build succeeds (existing warnings only). `bun run check` reports ONLY `scratch/harness.ts` + formatting/non-null assertions, (harness since deleted; check is now fully clean apart from the two existing config infos). + Separate Biome check of all `src` and root source/config files passes with no errors/warnings. +- Screenshots at 1280px and 420px confirm the failed edit, orphan note, and pending header. +- Backend, contracts, mirrors and open asks: unchanged; no backend asks. + ## Milestone: Tool-call card visual polish (`feature/tool-call-cards`, 2026-10-14) **Slice status:** COMPLETE (frontend-only follow-up to the user's headless-browser visual pass). @@ -11,8 +11,6 @@ "!**/node_modules", "!**/dist", "!**/build", - "!scratch", - "!reports", "!**/*.svelte", "!**/src/themes" ] 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("<script>alert(1)</script>"); }); @@ -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 @@ <span class="result-compact">→ {card.compactResultSummary}</span> {/if} </span> + {:else} + <span class="tool-result text-xs text-base-content/60">→ running…</span> {/if} </div> {/snippet} <div class="min-w-0 space-y-4 text-xs"> <section aria-label="Call"> - <h3 class="mb-2 border-b border-base-content/20 pb-1 font-semibold text-base-content"> - Call - </h3> {#if card.callDetail.kind === "unavailable"} - <p class="text-base-content/75">{card.callDetail.text}</p> - {:else if card.callDetail.kind === "command"} + <aside + role="note" + class="border-l-2 border-base-content/20 pl-3 italic text-base-content/60" + > + {card.callDetail.text} + </aside> + {:else} + <h3 class="mb-2 border-b border-base-content/20 pb-1 font-semibold text-base-content"> + Call + </h3> + {/if} + {#if card.callDetail.kind === "command"} <pre class="overflow-x-auto">$ {card.callDetail.text}</pre> {:else if card.callDetail.kind === "edit"} <div class="mb-2 break-all font-mono">{card.callDetail.path}</div> - <div class="space-y-1"> - <div class="font-medium text-error">− old</div> - <pre class="overflow-x-auto text-error">{card.callDetail.oldText}</pre> - <div class="font-medium text-success">+ new</div> - <pre class="overflow-x-auto text-success">{card.callDetail.newText}</pre> + <!-- Unified-diff style: every removed/added line carries its own −/+ gutter + marker and a tinted row, so multi-line edits read as one diff block. --> + <div class="edit-diff overflow-x-auto rounded font-mono" role="group" aria-label="Edit diff"> + {#if card.callDetail.oldText !== ""} + {#each card.callDetail.oldText.split("\n") as line, i (i)} + <div class="edit-diff-row edit-diff-del"> + <span class="edit-diff-sign" aria-label="removed">−</span><span>{line}</span> + </div> + {/each} + {/if} + {#if card.callDetail.newText !== ""} + {#each card.callDetail.newText.split("\n") as line, i (i)} + <div class="edit-diff-row edit-diff-add"> + <span class="edit-diff-sign" aria-label="added">+</span><span>{line}</span> + </div> + {/each} + {/if} </div> - {:else} + {:else if card.callDetail.kind === "fields"} <dl class="call-fields"> {#each card.callDetail.fields as field} <dt class="font-semibold text-base-content/85">{field.key}</dt> @@ -117,13 +138,13 @@ {/if} <div class="output-controls" class:sticky-controls={showAll}> <button - class="btn btn-primary btn-sm focus-visible:outline-primary" + class="btn btn-soft btn-xs focus-visible:outline-primary" aria-expanded={showAll} onclick={() => { showAll = !showAll; }} > - <span aria-hidden="true">{showAll ? "↑" : "↕"}</span> + {#if showAll}<span aria-hidden="true">↑</span>{/if} {showAll ? "Show fewer lines" : `⋯ ${card.output.hiddenCount} more lines`} </button> </div> @@ -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); |