fix: print the running tool once, not twice (#17)

This commit was merged in pull request #17.
This commit is contained in:
2026-10-05 09:49:39 +00:00
parent 26e05dea5b
commit ff99778e5d
7 changed files with 198 additions and 34 deletions
+58
View File
@@ -116,6 +116,64 @@ describe("buildThreadMarkdown", () => {
expect(markdown).not.toContain("Thinking");
});
it("says what a running tool is doing exactly once, before any text arrives", () => {
// The reported bug, from a screenshot: the activity line and the answer
// placeholder both printed "Reading a page", one directly above the other.
const markdown = buildThreadMarkdown([
message({
role: "assistant",
isStreaming: true,
text: "",
activity: { label: "Reading a page", count: 1 },
}),
]);
expect(markdown.match(/Reading a page/g)).toHaveLength(1);
// With nothing to read yet, the activity line is the stand-in, so the cursor
// rides on it rather than sitting on a line of its own.
expect(markdown).toContain(`_Reading a page…_ ${STREAMING_CURSOR}`);
});
it("keeps the activity line once the answer has started arriving", () => {
// The other half of the fix: a tool that starts mid-answer has to stay on
// screen, which is why the line is not just a stand-in for missing text.
const markdown = buildThreadMarkdown([
message({
role: "assistant",
isStreaming: true,
text: "Here is what I found.",
activity: { label: "Reading a page", count: 1 },
}),
]);
expect(markdown).toContain("_Reading a page…_");
expect(markdown).toContain("Here is what I found.");
// And the cursor follows the answer, not the activity line.
expect(markdown).toContain(`Here is what I found. ${STREAMING_CURSOR}`);
expect(markdown).not.toContain(`_Reading a page…_ ${STREAMING_CURSOR}`);
});
it("names a repeat rather than leaving a bare count to be interpreted", () => {
const markdown = buildThreadMarkdown([
message({
role: "assistant",
isStreaming: true,
activity: { label: "Searching the web", count: 2 },
}),
]);
expect(markdown).toContain("the same call again (2)");
});
it("drops the activity line when the message is no longer streaming", () => {
const markdown = buildThreadMarkdown([
message({ role: "assistant", text: "done", activity: { label: "Searching the web", count: 1 } }),
]);
expect(markdown).not.toContain("Searching the web");
expect(markdown).not.toContain(STREAMING_CURSOR);
});
it("shows a placeholder while an answer has no text yet", () => {
expect(buildThreadMarkdown([message({ role: "assistant", isStreaming: true })])).toContain("Thinking");
expect(buildThreadMarkdown([message({ role: "assistant" })])).toContain("No response");
+46 -1
View File
@@ -1,9 +1,11 @@
import { describe, expect, it } from "vitest";
import { buildThread, type LiveState } from "../lib/thread";
import { buildThread, describeActivity, type LiveState } from "../lib/thread";
import type { ChatTurn } from "../lib/types";
const idle: LiveState = { isStreaming: false, response: "", thinking: "" };
const activity = { label: "Reading a page", count: 1 };
function turn(id: string, prompt: string, response: string, overrides: Partial<ChatTurn> = {}): ChatTurn {
return {
id,
@@ -133,3 +135,46 @@ describe("buildThread", () => {
expect(thread[0].error).toBe("Request failed");
});
});
describe("carrying the running tool", () => {
const live: LiveState = { isStreaming: true, response: "", thinking: "", activity };
it("puts it on the message being streamed", () => {
const thread = buildThread([turn("t1", "q", "a")], live);
const assistant = thread.find((message) => message.role === "assistant");
expect(assistant?.activity).toEqual(activity);
});
it("keeps it off every earlier message", () => {
// Only the newest turn can be mid-flight, and an older answer carrying a
// "Searching the web" line would be a claim about a finished turn.
const thread = buildThread([turn("t1", "q", "a"), turn("t2", "q2", "a2")], live);
expect(thread.slice(2).every((message) => message.activity === undefined)).toBe(true);
});
it("drops it once the turn is over", () => {
const thread = buildThread([turn("t1", "q", "a")], { ...idle, activity });
expect(thread.every((message) => message.activity === undefined)).toBe(true);
});
it("never puts one on a question", () => {
const thread = buildThread([turn("t1", "q", "a")], live);
expect(thread.find((message) => message.role === "user")?.activity).toBeUndefined();
});
});
describe("describeActivity", () => {
it("is just the label for a first call", () => {
expect(describeActivity({ label: "Reading a page", count: 1 })).toBe("Reading a page");
});
it("says a repeat is a repeat, because a bare number reads as progress", () => {
expect(describeActivity({ label: "Reading a page", count: 3 })).toBe(
"Reading a page · the same call again (3)",
);
});
});
+15 -23
View File
@@ -2,7 +2,7 @@ import { Action, ActionPanel, Icon, LaunchType, List, launchCommand } from "@vic
import { useCallback, useEffect, useMemo, useState } from "react";
import { readAttachment } from "../attachments";
import { runAutoDelete } from "../cleanup";
import { useChat, type ToolActivity } from "../hooks/useChat";
import { useChat } from "../hooks/useChat";
import { usePreferences } from "../hooks/usePreferences";
import { useVisionSupport } from "../hooks/useVision";
import { buildThreadMarkdown, renderAttachments, type TranscriptOptions } from "../markdown";
@@ -63,18 +63,6 @@ const DEFAULT_IMAGE_PROMPT = "What is in this image?";
/** The reminder shown while a snip is attached but not yet sent. */
const PENDING_NOTE = "_Ask a question about it, or press Return to send it as it is._";
/**
* The line that shows what Lam is doing right now.
*
* A count on its own reads as progress, so the second time round says what it is:
* the same call, asked again. That is the visible half of the loop guard in
* `agent.ts`, and it is the only warning the user gets before a turn is cut short.
*/
function describeActivity(activity: ToolActivity): string {
if (activity.count <= 1) return `_${activity.label}…_`;
return `_${activity.label} · the same call again (${activity.count})_`;
}
export interface ChatViewProps {
/** Opens a specific conversation instead of resuming the latest one. */
initialSession?: Session;
@@ -138,8 +126,6 @@ function ChatSession({
const chat = useChat({ preferences, initialSession, vision: vision.support });
const { replaceSession } = chat;
const { canSeeImages } = vision;
const { activity } = chat;
const [input, setInput] = useState(initialPrompt ?? "");
const [summaries, setSummaries] = useState<SessionSummary[] | undefined>(undefined);
// One flag, not two. See the startup effect below for why that matters.
@@ -232,8 +218,16 @@ function ChatSession({
response: chat.streamingResponse,
thinking: chat.streamingThinking,
status: chat.status,
activity: chat.activity,
}),
[chat.session.turns, chat.isStreaming, chat.streamingResponse, chat.streamingThinking, chat.status],
[
chat.session.turns,
chat.isStreaming,
chat.streamingResponse,
chat.streamingThinking,
chat.status,
chat.activity,
],
);
const transcriptOptions = useMemo<TranscriptOptions>(
@@ -255,12 +249,10 @@ function ChatSession({
// The preview comes first so the captured image is visible before the user
// decides what to ask about it, rather than only after sending.
pending ? `${renderAttachments([pending])}\n\n${PENDING_NOTE}` : undefined,
// Deliberately not gated on the Show Tool Calls preference. That setting is
// about the finished transcript, where a list of names adds nothing. This
// is about the turn in flight, where the same tool coming round a second
// time is the difference between a long answer and a stuck one, and hiding
// it would leave the user watching a spinner they cannot interpret.
activity ? describeActivity(activity) : undefined,
// Deliberately nothing about the running tool here. That line is rendered by
// the transcript, on the live message, so that one fact is printed once:
// adding it here as well put "Reading a page" on screen twice, one line
// above the other, while a tool ran before any text had arrived.
].filter((note): note is string => Boolean(note));
// The placeholder is what tells a user that the search bar is the composer.
@@ -271,7 +263,7 @@ function ChatSession({
thread.length === 0 ? empty : buildThreadMarkdown(thread, transcriptOptions);
return [...notes, transcript].filter((part) => part.length > 0).join("\n\n");
}, [launchNote, snipNote, pending, activity, thread, transcriptOptions]);
}, [launchNote, snipNote, pending, thread, transcriptOptions]);
const rows: SessionRow[] = useMemo(() => {
const base = summaries ?? [];
+29 -8
View File
@@ -1,5 +1,6 @@
import { imageSource } from "./attachments";
import { repairCodeBlocks } from "./code";
import { describeActivity } from "./thread";
import type { Attachment, ChatTurn, ThreadMessage } from "./types";
/**
@@ -82,17 +83,37 @@ function renderMessage(message: ThreadMessage, options: TranscriptOptions): stri
}
// While a tool is running there is no answer text yet, so the activity line
// stands in and the user can see what Lam is actually doing.
const placeholder = message.status ? `_${message.status}…_` : "_Thinking…_";
const body = message.text.trim()
// stands in and the user can see what Lam is actually doing. It is shown
// whether or not tokens have arrived, because a tool that starts after the
// answer has begun would otherwise be invisible for the rest of the turn.
const activityLine =
message.isStreaming && message.activity ? `_${describeActivity(message.activity)}…_` : undefined;
/** The answer, or whatever stands in for it while there is none. */
const answered = message.text.trim();
const body = answered
? // Fences without a language render as a plain dark panel, and a lone
// inline command renders as a chip with no Copy button at all. Both are
// repaired before the markdown reaches the renderer.
repairCodeBlocks(message.text.trim())
: message.isStreaming
? placeholder
: "_No response._";
const sections = [message.isStreaming ? `${body} ${STREAMING_CURSOR}` : body];
repairCodeBlocks(answered)
: !message.isStreaming
? "_No response._"
: activityLine
? // The activity line above is already saying what is being waited on, so
// printing the placeholder as well put the same sentence on screen
// twice: once as a note above the transcript and once as the stand-in.
undefined
: message.status
? `_${message.status}…_`
: "_Thinking…_";
// The cursor trails whichever of the two is written last, so it always sits at
// the end of the text arriving rather than on a line of its own.
const written = [activityLine, body].filter((line): line is string => Boolean(line));
const last = written.length - 1;
const sections = written.map((line, index) =>
message.isStreaming && index === last ? `${line} ${STREAMING_CURSOR}` : line,
);
// The one thing allowed to break the plain-text rule, because it means
// something went wrong and should not be skimmed past.
+19 -1
View File
@@ -1,4 +1,4 @@
import type { ChatTurn, ThreadMessage } from "./types";
import type { ChatTurn, ThreadMessage, ToolActivity } from "./types";
/**
* Flattens stored turns into the flat message list the transcript renders.
@@ -15,6 +15,20 @@ export interface LiveState {
thinking: string;
/** Activity line, e.g. "Searching the web". Only meaningful mid-turn. */
status?: string;
/** The tool being waited on right now, which supersedes the plain status when present. */
activity?: ToolActivity;
}
/**
* One line describing a tool that is running.
*
* Says "the same call again" rather than showing a bare count, because a number
* on its own reads as progress. This is the visible half of the loop guard in
* `agent.ts`, and the only warning the user gets before a turn is cut short.
*/
export function describeActivity(activity: ToolActivity): string {
if (activity.count <= 1) return activity.label;
return `${activity.label} · the same call again (${activity.count})`;
}
export function buildThread(turns: ChatTurn[], live: LiveState): ThreadMessage[] {
@@ -43,6 +57,10 @@ export function buildThread(turns: ChatTurn[], live: LiveState): ThreadMessage[]
// Only while there is nothing to read yet. Once tokens land, the
// placeholder it feeds is gone anyway.
status: isLive && text.length === 0 ? live.status : undefined,
// The tool, on the other hand, is worth naming even after the answer has
// started arriving: a tool that begins mid-answer would otherwise be
// invisible, which was the original complaint about hidden tool calls.
activity: isLive ? live.activity : undefined,
// Only a question carries attachments, so the answer never has one.
attachments: [],
});
+28
View File
@@ -158,6 +158,24 @@ export interface SessionSummary {
*/
export type VisionSupport = "yes" | "no" | "unknown";
/**
* The tool running right now, and how stale it is getting.
*
* Lives here rather than beside the chat state machine so that `thread.ts` can
* carry it into the transcript, which is the only place it is now rendered.
*/
export interface ToolActivity {
/** Human-readable label, e.g. "Searching the web". */
label: string;
/**
* How many times this turn has now started this *identical* call.
*
* Counted by the agent over name and arguments together, so two different
* searches are two tools doing their job rather than a repeat.
*/
count: number;
}
/** A model as reported by `GET /api/tags`. */
export interface ModelInfo {
name: string;
@@ -204,6 +222,16 @@ export interface ThreadMessage {
error?: string;
/** Activity line shown while this message is streaming, e.g. "Searching the web". */
status?: string;
/**
* The tool this message is waiting on, when there is one.
*
* Separate from {@link ThreadMessage.status} because the two are rendered from
* different triggers: the status is a stand-in used only while there is no text
* to read, while a running tool is worth saying whether or not tokens have
* started arriving. Carrying both is what lets one line be printed for one
* fact, rather than the same sentence appearing twice at the top of a turn.
*/
activity?: ToolActivity;
/** Images attached to this question. Only ever non-empty on a user message. */
attachments: Attachment[];
}