fix: name each tool once, and stream the reasoning live (#18)

src/__tests__/markdown.test.ts

@@ -233,7 +233,7 @@ describe("buildThreadMarkdown", () => {
     expect(markdown).toContain("[https://example.com](https://example.com)");
   });
 
-  it("lists tools on one quiet line, flagging failures", () => {
+  it("lists tools on one quiet line, each named once, flagging failures", () => {
     const markdown = buildThreadMarkdown([
       message({
         role: "assistant",
@@ -245,11 +245,50 @@ describe("buildThreadMarkdown", () => {
       }),
     ]);
 
-    expect(markdown).toContain("_Tools: `web_search`, `web_fetch` (failed)_");
+    expect(markdown).toContain("_Tools: `web_search`, `web_fetch` (1 failed)_");
     // Italic, not a bold header, so it stays below the answer.
     expect(markdown).not.toContain("**Tools");
   });
 
+  it("names a tool once however many times it was called", () => {
+    // The reported case: a research turn listed the same two tools twenty times
+    // over three lines, which is a wall of text that says nothing a name does
+    // not. Names in order of first use, no repetition.
+    const markdown = buildThreadMarkdown([
+      message({
+        role: "assistant",
+        text: "a",
+        toolCalls: [
+          { name: "web_search", arguments: {}, result: "ok", isError: false },
+          { name: "web_fetch", arguments: {}, result: "ok", isError: false },
+          { name: "web_search", arguments: {}, result: "ok", isError: false },
+          { name: "web_search", arguments: {}, result: "ok", isError: false },
+        ],
+      }),
+    ]);
+
+    expect(markdown).toContain("_Tools: `web_search`, `web_fetch`_");
+    expect(markdown.match(/web_search/g)).toHaveLength(1);
+  });
+
+  it("counts every failed call, not just the failed tools", () => {
+    const markdown = buildThreadMarkdown([
+      message({
+        role: "assistant",
+        text: "a",
+        toolCalls: [
+          { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+          { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+          { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+          { name: "web_search", arguments: {}, result: "ok", isError: false },
+        ],
+      }),
+    ]);
+
+    expect(markdown).toContain("_Tools: `web_fetch`, `web_search` (3 failed)_");
+    expect(markdown.match(/web_fetch/g)).toHaveLength(1);
+  });
+
   it("omits tools and reasoning when the preferences are off", () => {
     const messages = [
       message({
@@ -307,13 +346,30 @@ describe("buildThreadMarkdown", () => {
       }),
     ]);
 
-    expect(markdown).toContain("`web_search` (repeated, stopped)");
-    expect(markdown).not.toContain("(failed)");
+    expect(markdown).toContain("_Tools: `web_search` (stopped: the same call kept repeating)_");
+    expect(markdown).not.toContain("failed");
   });
 
-  it("shows a failed call even when tool calls are hidden", () => {
-    // Hiding the line would leave the user with an answer they cannot account
-    // for, which is worse than a quiet italic line they did not ask for.
+  it("says a call failed and a call repeated, when both happened", () => {
+    const markdown = buildThreadMarkdown([
+      message({
+        role: "assistant",
+        text: "a",
+        toolCalls: [
+          { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+          { name: "web_search", arguments: {}, result: "Refused", isError: true, repeated: true },
+        ],
+      }),
+    ]);
+
+    expect(markdown).toContain(
+      "_Tools: `web_fetch`, `web_search` (1 failed, stopped: the same call kept repeating)_",
+    );
+  });
+
+  it("hides a failed call's name, but still says that a call failed", () => {
+    // Off means the list is off, not that a failure goes unmentioned: the answer
+    // alone would not account for a tool that did not work.
     const markdown = buildThreadMarkdown(
       [
         message({
@@ -325,12 +381,32 @@ describe("buildThreadMarkdown", () => {
       { showToolCalls: false, showReasoning: false },
     );
 
-    expect(markdown).toContain("`web_fetch` (failed)");
+    expect(markdown).toContain("_1 tool call failed._");
+    expect(markdown).not.toContain("web_fetch");
+    expect(markdown).not.toContain("_Tools:");
   });
 
-  it("shows a refused repeat even when tool calls are hidden", () => {
-    // This is the one the user most needs to see: a model going in circles is
-    // exactly the thing a hidden tool list would conceal.
+  it("counts the failures rather than naming every failed call when hidden", () => {
+    const markdown = buildThreadMarkdown(
+      [
+        message({
+          role: "assistant",
+          text: "a",
+          toolCalls: [
+            { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+            { name: "web_search", arguments: {}, result: "boom", isError: true },
+          ],
+        }),
+      ],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown).toContain("_2 tool calls failed._");
+  });
+
+  it("says a turn was cut short by a repeat, without naming the call", () => {
+    // Still the one the user most needs to know about: a model going in circles
+    // is exactly the thing a hidden tool list would conceal.
     const markdown = buildThreadMarkdown(
       [
         message({
@@ -344,7 +420,26 @@ describe("buildThreadMarkdown", () => {
       { showToolCalls: false, showReasoning: false },
     );
 
-    expect(markdown).toContain("`web_search` (repeated, stopped)");
+    expect(markdown).toContain("_a call kept repeating, so the turn was cut short._");
+    expect(markdown).not.toContain("web_search");
+  });
+
+  it("says both when a call failed and another kept repeating, with tools hidden", () => {
+    const markdown = buildThreadMarkdown(
+      [
+        message({
+          role: "assistant",
+          text: "a",
+          toolCalls: [
+            { name: "web_fetch", arguments: {}, result: "boom", isError: true },
+            { name: "web_search", arguments: {}, result: "Refused", isError: true, repeated: true },
+          ],
+        }),
+      ],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown).toContain("_1 tool call failed and a call kept repeating, so the turn was cut short._");
   });
 
   it("still hides the ordinary calls when tool calls are off", () => {
@@ -368,6 +463,73 @@ describe("buildThreadMarkdown", () => {
       "Reasoning",
     );
   });
+
+  it("streams the reasoning live even when Show Reasoning is off", () => {
+    // While a turn is in flight the reasoning is the only sign of progress, so
+    // it is shown whatever the preference says. It is the live view only.
+    const markdown = buildThreadMarkdown(
+      [message({ role: "assistant", text: "", isStreaming: true, thinking: "weighing the options" })],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown).toContain("weighing the options");
+    expect(markdown).toContain("Reasoning");
+    // The cursor trails the reasoning, because nothing else is arriving.
+    expect(markdown).toContain(`> weighing the options ${STREAMING_CURSOR}`);
+    // And the "Thinking…" stand-in is dropped, since the reasoning says it.
+    expect(markdown).not.toContain("Thinking…");
+  });
+
+  it("drops the reasoning when the turn ends and Show Reasoning is off", () => {
+    const markdown = buildThreadMarkdown(
+      [message({ role: "assistant", text: "the answer", thinking: "weighing the options" })],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown).toContain("the answer");
+    expect(markdown).not.toContain("weighing the options");
+    expect(markdown).not.toContain("Reasoning");
+  });
+
+  it("keeps the reasoning under the answer when the turn ends and Show Reasoning is on", () => {
+    const markdown = buildThreadMarkdown([
+      message({ role: "assistant", text: "the answer", thinking: "weighing the options" }),
+    ]);
+
+    expect(markdown).toContain("the answer");
+    expect(markdown).toContain("weighing the options");
+    expect(markdown.indexOf("the answer")).toBeLessThan(markdown.indexOf("weighing the options"));
+  });
+
+  it("starts the live reasoning without a leading blank line", () => {
+    // Nothing else is on screen while the model thinks, so the reasoning is the
+    // first block, and a stray blank line above it would read as a gap.
+    const markdown = buildThreadMarkdown(
+      [message({ role: "assistant", text: "", isStreaming: true, thinking: "weighing the options" })],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown.startsWith("_Reasoning_")).toBe(true);
+  });
+
+  it("rides the answer, not the reasoning, once an answer has arrived", () => {
+    // The cursor rides the answer once there is one, even while reasoning from
+    // the round before it is still on screen.
+    const markdown = buildThreadMarkdown(
+      [
+        message({
+          role: "assistant",
+          text: "partial answer",
+          isStreaming: true,
+          thinking: "weighing the options",
+        }),
+      ],
+      { showToolCalls: false, showReasoning: false },
+    );
+
+    expect(markdown).toContain(`partial answer ${STREAMING_CURSOR}`);
+    expect(markdown).not.toContain(`weighing the options ${STREAMING_CURSOR}`);
+  });
 });
 
 describe("sessionToPlainText", () => {

src/__tests__/thinking.test.ts

@@ -0,0 +1,54 @@
+import { describe, expect, it } from "vitest";
+import { EMPTY_TRACE, afterToolCall, withFinalTrace, withThinking } from "../lib/thinking";
+
+describe("ThinkingTrace", () => {
+  it("starts empty", () => {
+    expect(EMPTY_TRACE).toEqual({ full: "", current: "" });
+  });
+
+  it("appends a delta to both the full trace and the current segment", () => {
+    const trace = withThinking(withThinking(EMPTY_TRACE, "let me "), "think");
+
+    expect(trace).toEqual({ full: "let me think", current: "let me think" });
+  });
+
+  it("keeps the full trace when a tool starts, and clears the current segment", () => {
+    // The reported case: the model thinks, calls a tool, and thinks again. The
+    // live view shows only the new thinking; the stored turn keeps both rounds.
+    const first = withThinking(EMPTY_TRACE, "I should search");
+    const cleared = afterToolCall(first);
+
+    expect(cleared.full).toBe("I should search");
+    expect(cleared.current).toBe("");
+
+    const second = withThinking(cleared, "now I know");
+    expect(second.full).toBe("I should searchnow I know");
+    expect(second.current).toBe("now I know");
+  });
+
+  it("does not mutate the trace it was given", () => {
+    const trace = withThinking(EMPTY_TRACE, "a");
+    afterToolCall(trace);
+    withThinking(trace, "b");
+
+    expect(trace).toEqual({ full: "a", current: "a" });
+  });
+
+  it("clears each round separately across several tool calls", () => {
+    let trace = EMPTY_TRACE;
+    for (const round of ["one", "two", "three"]) {
+      trace = withThinking(trace, round);
+      trace = afterToolCall(trace);
+    }
+
+    expect(trace.full).toBe("onetwothree");
+    expect(trace.current).toBe("");
+  });
+
+  it("adopts the authoritative trace the turn ended with", () => {
+    const trace = withFinalTrace("the whole thing");
+
+    expect(trace.full).toBe("the whole thing");
+    expect(trace.current).toBe("");
+  });
+});