diff --git a/CLAUDE.md b/CLAUDE.md index 595b26a..18211d5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,15 +201,27 @@ Instructions loaded on demand, in the standard `SKILL.md` format — full detail under `agent_model_settings`). The active thread is NOT in a context — it is derived from the URL by `useActiveThreadId()`, so there is one source of truth. - **Custom Hooks**: `useChatThread`, `useMCPTools`, `useThreads` for data domains -- **Message Components**: Separate components for AI/Human/Tool/Error message types +- **Message Components**: `AIMessage` (prose only — tool calls riding on it are rendered by + `ToolActivityGroup`, not here), `HumanMessage`, `ErrorMessage` - **Tool rendering** (`src/components/toolRenderers/`): `config.ts` maps each **built-in** tool to a view for its **arguments** and its **result** (`sql`, `table`, `receipt`, field grids); anything not listed — every MCP tool, anything added later — falls back to `json`. A closed catalog with an open fallback: the payload SELECTS a client-owned renderer, it never describes one. Keyed on tool name rather than sniffed from the payload, because sniffing predicates are a second contract that drifts. `config.test.ts` pins every key to a really-registered tool, so a rename can't silently - drop a tool back to JSON. A tool call's args render in `ToolCallDisplay`, its result in - `ToolMessage` — two different renders of one operation. + drop a tool back to JSON. +- **A call and its result render as ONE card** (`ToolActivityGroup.tsx`). The wire delivers them as + two messages — the call on an AI message, the result as its own `tool` message — and the template + drew two cards for them. `buildThreadItems()` (`src/services/toolActivity.ts`) pairs them by + `tool_call_id` (never by adjacency: results arrive when they resolve, and a paused call never gets + one) and groups CONSECUTIVE calls into one card. Collapsed by default. + - **Header label**: a single call gets its own gist (`run_sql · 37 rows`); several get a generic + `N tools called`, because no summary spans differently-shaped payloads. `summarizeResult` takes + the tool name so `ok: true` reads as "written" only for a **mutating** tool. + - **Two things are never collapsed**: a gated call's ARGUMENTS (you cannot approve what you cannot + read) and a CHART (it is the answer, not a detail — and it renders with no JSON receipt under + it). A drawn chart leaves the collapsed list entirely. + - There is no global "hide tools" toggle any more; collapsing per card replaced it. - **Design tokens** (`src/app/globals.css`): paper ground / ink text / one amber `--brand`, mapped ONTO shadcn's semantic names so `ui/*` inherits them. Amber marks the approval boundary and nothing else. `--muted-foreground` must stay ≥4.5:1 on paper (`--faint` is the decorative-only @@ -297,7 +309,7 @@ Instructions loaded on demand, in the standard `SKILL.md` format — full detail the SSE contract is otherwise unchanged. Position in the message list is NOT a pending signal: tool results and later AI text both arrive after the call, so "is it last?" is wrong in both directions — it hid the gate on a real pause and offered APPROVE on read-only tools. -- `ToolCallDisplay` additionally requires `isMutatingTool(name)` before rendering the amber panel. +- `ToolActivityGroup` additionally requires `isMutatingTool(name)` before rendering the amber panel. Two independent conditions, because that panel claims "this writes to your data". - `src/services/agentService.ts` translates the wire signal into a HITL resume: it reads the pending request via `agent.graph.getState()` and resumes with `Command({ resume: { decisions } })` — one diff --git a/src/components/AIMessage.tsx b/src/components/AIMessage.tsx index c03dce4..97b1393 100644 --- a/src/components/AIMessage.tsx +++ b/src/components/AIMessage.tsx @@ -1,34 +1,22 @@ -import type { MessageResponse, ToolApprovalCallbacks } from "@/types/message"; +import type { MessageResponse } from "@/types/message"; import { Bot } from "lucide-react"; import rehypeKatex from "rehype-katex"; import { cn } from "@/lib/utils"; -import { - getMessageContent, - hasToolCalls, - getToolCalls, - getPendingToolCallIds, -} from "@/services/messageUtils"; -import { ToolCallDisplay } from "./ToolCallDisplay"; -import { useUISettings } from "@/contexts/UISettingsContext"; +import { getMessageContent } from "@/services/messageUtils"; import MDEditor from "@uiw/react-md-editor"; interface AIMessageProps { message: MessageResponse; - approvalCallbacks?: ToolApprovalCallbacks; } -export const AIMessage = ({ message, approvalCallbacks }: AIMessageProps) => { +/** + * The agent's prose. Tool calls ride along on these messages but are NOT rendered here — they are + * paired with their results and drawn by ToolActivityGroup, so one operation is one card. + */ +export const AIMessage = ({ message }: AIMessageProps) => { const messageContent = getMessageContent(message); - const hasTools = hasToolCalls(message); - const toolCalls = getToolCalls(message); - const pendingToolCallIds = getPendingToolCallIds(message); - const { hideToolMessages } = useUISettings(); - // If tool messages are hidden and there's no text content, don't render anything - const shouldShowTools = hasTools && !hideToolMessages; - const hasVisibleContent = messageContent || shouldShowTools; - - if (!hasVisibleContent) { + if (!messageContent) { return null; } @@ -37,38 +25,24 @@ export const AIMessage = ({ message, approvalCallbacks }: AIMessageProps) => {
- {/* Prose stays in a readable column; tool cards run full width so a call lines up with - the result card that follows it as a sibling message. */}
- {messageContent && ( -
-
- -
-
- )} - - {shouldShowTools && ( -
- +
+
- )} +
); diff --git a/src/components/MessageInput.tsx b/src/components/MessageInput.tsx index 73c4eb6..f7386dd 100644 --- a/src/components/MessageInput.tsx +++ b/src/components/MessageInput.tsx @@ -1,6 +1,6 @@ import { FormEvent, useEffect, useRef, useState } from "react"; import { Button } from "./ui/button"; -import { ArrowUp, Loader2, Eye, EyeOff, Paperclip, X, ChevronDown } from "lucide-react"; +import { ArrowUp, Loader2, Paperclip, X, ChevronDown } from "lucide-react"; import { MessageOptions, FileAttachment } from "@/types/message"; import { ModelConfiguration } from "./ModelConfiguration"; import { useUISettings } from "@/contexts/UISettingsContext"; @@ -23,16 +23,8 @@ export const MessageInput = ({ const [attachments, setAttachments] = useState([]); const [isUploading, setIsUploading] = useState(false); - const { - hideToolMessages, - toggleToolMessages, - provider, - setProvider, - model, - setModel, - approveAllTools, - setApproveAllTools, - } = useUISettings(); + const { provider, setProvider, model, setModel, approveAllTools, setApproveAllTools } = + useUISettings(); const textareaRef = useRef(null); const fileInputRef = useRef(null); @@ -252,22 +244,6 @@ export const MessageInput = ({ )} attach - -
diff --git a/src/components/MessageList.tsx b/src/components/MessageList.tsx index a3e1129..baa19ea 100644 --- a/src/components/MessageList.tsx +++ b/src/components/MessageList.tsx @@ -2,19 +2,21 @@ import type { MessageResponse, ToolApprovalCallbacks } from "@/types/message"; import { HumanMessage } from "./HumanMessage"; import { AIMessage } from "./AIMessage"; import { ErrorMessage } from "./ErrorMessage"; -import { useEffect, useRef } from "react"; -import { getMessageId } from "@/services/messageUtils"; +import { useEffect, useMemo, useRef } from "react"; +import { buildThreadItems } from "@/services/toolActivity"; import dynamic from "next/dynamic"; -import { useUISettings } from "@/contexts/UISettingsContext"; -const ToolMessage = dynamic(() => import("./ToolMessage").then((m) => m.ToolMessage), { - ssr: false, - loading: () => ( -
- Loading tool output… -
- ), -}); +const ToolActivityGroupCard = dynamic( + () => import("./ToolActivityGroup").then((m) => m.ToolActivityGroupCard), + { + ssr: false, + loading: () => ( +
+ loading tools… +
+ ), + }, +); interface MessageListProps { messages: MessageResponse[]; @@ -23,7 +25,6 @@ interface MessageListProps { const MessageList = ({ messages, approveToolExecution }: MessageListProps) => { const bottomRef = useRef(null); - const { hideToolMessages } = useUISettings(); useEffect(() => { bottomRef.current?.scrollIntoView({ behavior: "smooth" }); @@ -36,32 +37,30 @@ const MessageList = ({ messages, approveToolExecution }: MessageListProps) => { onDeny: (toolCallId: string) => approveToolExecution(toolCallId, "deny"), } : undefined; - // Deduplicate by type+id: ids are only unique WITHIN a type (an AI message's synthetic id and a - // tool message's call id come from different namespaces), so keying on id alone can drop a - // distinct message. Messages without an id are always kept — they cannot be compared. - const seen = new Set(); - const uniqueMessages = messages.filter((message) => { - const id = message.data?.id; - if (!id) return true; - const key = `${message.type}:${id}`; - if (seen.has(key)) return false; - seen.add(key); - return true; - }); + + // Dedup, plus pairing of each call with its result; see services/toolActivity.ts. + const items = useMemo(() => buildThreadItems(messages), [messages]); return ( -
- {uniqueMessages.map((message, index) => { - // Key by type+id for the same reason the dedup does; index backs up an id-less message. - const key = `${message.type}:${getMessageId(message) || index}`; +
+ {items.map((item) => { + if (item.kind === "tools") { + return ( + + ); + } + + const { message } = item; if (message.type === "human") { - return ; + return ; } else if (message.type === "ai") { - return ; - } else if (message.type === "tool" && !hideToolMessages) { - return ; + return ; } else if (message.type === "error") { - return ; + return ; } return null; })} diff --git a/src/components/Thread.tsx b/src/components/Thread.tsx index 8c6e425..78db7b6 100644 --- a/src/components/Thread.tsx +++ b/src/components/Thread.tsx @@ -61,8 +61,12 @@ export const Thread = ({ threadId }: ThreadProps) => { {messages.length > 0 ? ( <>
- -
+ {/* Radix renders its viewport as `display: table; min-width: 100%`, and a table sizes + to its widest content — so a wide tool payload stretches the whole thread column + instead of scrolling inside its own box. Forcing the inner table to `block` keeps + the column at the viewport width. */} + +
diff --git a/src/components/ToolActivityGroup.tsx b/src/components/ToolActivityGroup.tsx new file mode 100644 index 0000000..01875c6 --- /dev/null +++ b/src/components/ToolActivityGroup.tsx @@ -0,0 +1,197 @@ +"use client"; + +import React, { useState } from "react"; +import { ChevronDown, ChevronRight, Check, X, Lock } from "lucide-react"; +import type { ToolApprovalCallbacks } from "@/types/message"; +import type { ToolActivity } from "@/services/toolActivity"; +import { summarizeActivities, summarizeResult } from "@/services/toolSummary"; +import { ToolArgs, ToolResult, renderersFor } from "./toolRenderers"; +import { Chart } from "./charts/Chart"; +import type { ChartPayload } from "./charts/types"; + +/** + * One card for a run of tool calls. A call and its result are ONE operation, so they render + * together — the split across two components was inherited from the template and cost two + * full-width cards per call. + * + * Collapsed by default: for most tools the arguments and the payload are machinery, not answers. + * Two things are never hidden, because hiding them would hide the product: + * - a GATED call's arguments — you cannot approve what you cannot read; + * - a CHART — it is the answer itself, not a detail of one. + */ + +const isChartPayload = (v: unknown): v is ChartPayload => { + const p = v as ChartPayload | undefined; + return !!p && typeof p === "object" && !!p.spec && Array.isArray(p.rows); +}; + +const contentOf = (activity: ToolActivity): string => { + const content = activity.result?.data?.content; + if (!content) return ""; + return typeof content === "string" ? content : JSON.stringify(content, null, 2); +}; + +/** The chart an activity should draw outside the collapse, if any. */ +function chartOf(activity: ToolActivity): ChartPayload | null { + if (renderersFor(activity.name).result !== "chart") return null; + const artifact = (activity.result?.data as { artifact?: unknown } | undefined)?.artifact; + return isChartPayload(artifact) ? artifact : null; +} + +/** The amber approval gate. The one place amber appears: it means "this touches money". */ +const ApprovalGate = ({ + activity, + callbacks, +}: { + activity: ToolActivity; + callbacks: ToolApprovalCallbacks; +}) => { + const [responded, setResponded] = useState(false); + + return ( +
+
+ + + APPROVAL REQUIRED + + + {activity.name} + +
+ +
+
+ Cameron wants to run a tool that{" "} + writes to your data. +
+ + {/* Never collapsed — the arguments ARE the thing being approved. */} + + +
+ + + + nothing is written until you approve + +
+
+
+ ); +}; + +/** The expanded detail for one call: its arguments, then whatever came back. */ +const ActivityDetail = ({ activity }: { activity: ToolActivity }) => { + const content = contentOf(activity); + const gist = activity.result ? summarizeResult(content, activity.name) : null; + + return ( +
+
+ {activity.name} + {gist && · {gist}} +
+ + + + {activity.result ? ( + + ) : ( + + {activity.isPending ? "awaiting approval" : "running…"} + + )} +
+ ); +}; + +interface ToolActivityGroupProps { + activities: ToolActivity[]; + approvalCallbacks?: ToolApprovalCallbacks; +} + +export const ToolActivityGroupCard = ({ + activities, + approvalCallbacks, +}: ToolActivityGroupProps) => { + const [open, setOpen] = useState(false); + + // Charts and gates escape the collapse; everything else lives behind the toggle. A drawn chart + // leaves the collapsed list entirely — showing "render_chart · 5 rows" above its own chart is + // the duplication this refactor removes. + const charts = activities + .map((activity) => ({ activity, chart: chartOf(activity) })) + .filter((c): c is { activity: ToolActivity; chart: ChartPayload } => c.chart !== null); + const drawn = new Set(charts.map((c) => c.activity.callId)); + const isGated = (a: ToolActivity) => a.isPending && a.isMutating; + const gated = approvalCallbacks ? activities.filter(isGated) : []; + const collapsible = activities.filter((a) => !isGated(a) && !drawn.has(a.callId)); + + return ( + // w-full + min-w-0: the card is a direct child of the message list now (it used to sit inside + // AIMessage's flex column), so nothing else constrains it — without this a wide payload widens + // the whole thread column. +
+ {collapsible.length > 0 && ( +
+ + + {/* min-w-0 + overflow-hidden: without them a wide SQL block or JSON dump stretches the + card and scrolls the whole PAGE sideways instead of scrolling inside its own box. */} + {open && ( +
+ {collapsible.map((activity) => ( + + ))} +
+ )} +
+ )} + + {/* A chart is the answer — always visible, never behind the toggle, and with no JSON + receipt underneath it. */} + {charts.map(({ activity, chart }) => ( + + ))} + + {gated.map((activity) => ( + + ))} +
+ ); +}; diff --git a/src/components/ToolCallDisplay.tsx b/src/components/ToolCallDisplay.tsx deleted file mode 100644 index 7dca7d0..0000000 --- a/src/components/ToolCallDisplay.tsx +++ /dev/null @@ -1,167 +0,0 @@ -import React, { useState } from "react"; -import { ChevronDown, ChevronRight, Check, X, Lock } from "lucide-react"; -import type { ToolCall, FunctionCall, ToolApprovalCallbacks } from "@/types/message"; -import { isMutatingTool } from "@/lib/agent/mutatingTools"; -import { ToolArgs } from "./toolRenderers"; - -interface ToolCallDisplayProps { - toolCalls?: ToolCall[]; - functionCalls?: FunctionCall[]; - approvalCallbacks?: ToolApprovalCallbacks; - /** Ids the server reported as paused by the gate. Nothing else may open the approval panel. */ - pendingToolCallIds?: string[]; -} - -const renderArgs = (name: string, args: Record | string) => { - const parsed = typeof args === "string" ? (JSON.parse(args) as Record) : args; - return ; -}; - -const ToolCallItem: React.FC<{ - name: string; - args: Record; - id?: string; - approvalCallbacks?: ToolApprovalCallbacks; - isPending?: boolean; -}> = ({ name, args, id, approvalCallbacks, isPending }) => { - const [isExpanded, setIsExpanded] = useState(false); - const [responded, setResponded] = useState(false); - - // Two independent conditions, both required. `isPending` is the graph's actual paused state; - // `isMutatingTool` is the boundary itself. A read-only tool must never render the gate even if - // the server somehow reports it pending — the amber panel claims "this writes to your data". - const isPendingApproval = Boolean( - isPending && isMutatingTool(name) && id && approvalCallbacks && !responded, - ); - - // A gate awaiting a decision is the one place amber appears: it means "this touches money". - if (isPendingApproval) { - return ( -
-
- - - APPROVAL REQUIRED - - - {name} - -
- -
-
- Cameron wants to run a tool that{" "} - writes to your data. -
- - {renderArgs(name, args)} - -
- - - - nothing is written until you approve - -
-
-
- ); - } - - const isMutating = isMutatingTool(name); - - return ( - /* Same shell as ToolMessage: a call and its result are one operation and should read - as one family, even though they arrive as two messages. */ -
- - - {isExpanded && ( -
{renderArgs(name, args)}
- )} -
- ); -}; - -export const ToolCallDisplay: React.FC = ({ - toolCalls = [], - functionCalls = [], - approvalCallbacks, - pendingToolCallIds = [], -}) => { - const pending = new Set(pendingToolCallIds); - const hasToolCalls = toolCalls.length > 0; - const hasFunctionCalls = functionCalls.length > 0; - - if (!hasToolCalls && !hasFunctionCalls) { - return null; - } - - return ( -
- {hasToolCalls && ( -
- {toolCalls.map((toolCall, index) => ( - - ))} -
- )} - - {hasFunctionCalls && ( -
- {functionCalls.map((functionCall, index) => ( - - ))} -
- )} -
- ); -}; diff --git a/src/components/ToolMessage.tsx b/src/components/ToolMessage.tsx deleted file mode 100644 index 3635430..0000000 --- a/src/components/ToolMessage.tsx +++ /dev/null @@ -1,132 +0,0 @@ -import React, { useState } from "react"; -import type { MessageResponse } from "@/types/message"; -import { ChevronDownIcon, ChevronRightIcon, CopyIcon, CheckIcon } from "lucide-react"; -import { getToolName } from "@/services/messageUtils"; -import { ToolResult, renderersFor } from "./toolRenderers"; -import { Chart } from "./charts/Chart"; -import type { ChartPayload } from "./charts/types"; - -/** The artifact is `unknown` off the wire, so check the shape before drawing it. */ -const isChartPayload = (v: unknown): v is ChartPayload => { - const p = v as ChartPayload | undefined; - return !!p && typeof p === "object" && !!p.spec && Array.isArray(p.rows); -}; - -interface ToolMessageProps { - message: MessageResponse; -} - -const getContentStats = (content: string): string => { - const lines = content.split("\n").length; - const chars = content.length; - return lines > 1 ? `${lines} lines, ${chars} chars` : `${chars} chars`; -}; - -/** One-line gist for the collapsed state, from the shapes the finance tools return. */ -const summarize = (content: string): string | null => { - try { - const p = JSON.parse(content) as Record; - if (typeof p.error === "string") return p.error; - if (typeof p.rowCount === "number") return `${p.rowCount} rows`; - if (typeof p.matched === "number" && typeof p.returned === "number") { - return p.returned === p.matched - ? `${p.matched} transactions` - : `${p.returned} of ${p.matched} transactions`; - } - if (p.ok === true) return "written"; - return null; - } catch { - return null; - } -}; - -const getContentAsString = ( - content: string | import("@/types/message").ContentItem[] | undefined, -): string => { - if (!content) return ""; - if (typeof content === "string") return content; - return JSON.stringify(content, null, 2); -}; - -export const ToolMessage = ({ message }: ToolMessageProps) => { - const [open, setOpen] = useState(false); - const [copied, setCopied] = useState(false); - const toolName = getToolName(message); - const content = getContentAsString(message.data?.content); - const summary = summarize(content); - // A chart is the answer, not a detail of it, so it renders outside the collapsed disclosure — - // the receipt stays inside for inspection. - const artifact = (message.data as { artifact?: unknown })?.artifact; - const chart = - renderersFor(toolName).result === "chart" && isChartPayload(artifact) ? artifact : null; - - const handleCopy = async (e: React.MouseEvent) => { - e.stopPropagation(); - try { - await navigator.clipboard.writeText(content); - setCopied(true); - setTimeout(() => setCopied(false), 2000); - } catch (err) { - console.error("Failed to copy content:", err); - } - }; - - if (chart) { - return ( -
- -
- - {toolName ?? "tool"} · result - -
- -
-
-
- ); - } - - return ( -
- - - {open && ( -
- -
- )} -
- ); -}; diff --git a/src/components/toolRenderers/config.ts b/src/components/toolRenderers/config.ts index 759b3d5..504af93 100644 --- a/src/components/toolRenderers/config.ts +++ b/src/components/toolRenderers/config.ts @@ -36,7 +36,7 @@ export const TOOL_RENDERERS: Record = { // The result is a skill's markdown instructions — no purpose-built view yet, so it shows as // JSON. Listed anyway because an unlisted registered tool is what config.test.ts guards against. load_skill: { args: "fields", result: "json" }, - // The rows arrive as an artifact, not in the result content — see ToolMessage. + // The rows arrive as an artifact, not in the result content — see ToolActivityGroup. render_chart: { args: "sql", result: "chart" }, }; diff --git a/src/contexts/UISettingsContext.tsx b/src/contexts/UISettingsContext.tsx index 68e31ab..7a0452a 100644 --- a/src/contexts/UISettingsContext.tsx +++ b/src/contexts/UISettingsContext.tsx @@ -22,8 +22,6 @@ function saveSetting(key: string, value: string | boolean) { } interface UISettingsContextType { - hideToolMessages: boolean; - toggleToolMessages: () => void; provider: string; setProvider: (provider: string) => void; model: string; @@ -39,7 +37,6 @@ interface UISettingsProviderProps { } export const UISettingsProvider = ({ children }: UISettingsProviderProps) => { - const [hideToolMessages, setHideToolMessages] = useState(false); // Defaults must match DEFAULT_MODEL_PROVIDER/NAME in lib/agent/util.ts — these are sent as // query params on every request, so they override the server's default. const [provider, setProviderState] = useState("anthropic"); @@ -53,7 +50,6 @@ export const UISettingsProvider = ({ children }: UISettingsProviderProps) => { if (typeof saved.approveAllTools === "boolean") setApproveAllToolsState(saved.approveAllTools); }, []); - const toggleToolMessages = () => setHideToolMessages((prev) => !prev); const setProvider = (v: string) => { setProviderState(v); saveSetting("provider", v); @@ -70,8 +66,6 @@ export const UISettingsProvider = ({ children }: UISettingsProviderProps) => { return ( = {}): ToolCall { + return { name, args, id, type: "tool_call" }; +} + +function toolResult(callId: string, name: string, content = '{"ok":true}'): MessageResponse { + return { + type: "tool", + data: { id: callId, content, status: "success", tool_call_id: callId, name }, + }; +} + +const groups = (items: ReturnType) => + items.filter((i): i is ToolActivityGroup => i.kind === "tools"); + +describe("buildThreadItems", () => { + it("pairs a call with its result into one activity", () => { + const items = buildThreadItems([ + ai("a1", "", [call("run_sql", "c1")]), + toolResult("c1", "run_sql", '{"rowCount":37}'), + ]); + + const [group] = groups(items); + expect(group.activities).toHaveLength(1); + expect(group.activities[0]).toMatchObject({ callId: "c1", name: "run_sql", isPending: false }); + expect(group.activities[0].result).not.toBeNull(); + }); + + it("never renders a result as its own item", () => { + // The whole point of the refactor: one operation, one card. + const items = buildThreadItems([ + ai("a1", "", [call("run_sql", "c1")]), + toolResult("c1", "run_sql"), + ]); + + expect(items).toHaveLength(1); + expect(items[0].kind).toBe("tools"); + }); + + it("groups consecutive calls into one card", () => { + const items = buildThreadItems([ + ai("a1", "", [call("describe_finance_schema", "c1")]), + toolResult("c1", "describe_finance_schema"), + ai("a2", "", [call("run_sql", "c2")]), + toolResult("c2", "run_sql"), + ai("a3", "", [call("render_chart", "c3")]), + toolResult("c3", "render_chart"), + ]); + + const g = groups(items); + expect(g).toHaveLength(1); + expect(g[0].activities.map((a) => a.name)).toEqual([ + "describe_finance_schema", + "run_sql", + "render_chart", + ]); + }); + + it("starts a new group when the agent says something between calls", () => { + const items = buildThreadItems([ + ai("a1", "", [call("run_sql", "c1")]), + toolResult("c1", "run_sql"), + ai("a2", "Here is what I found."), + ai("a3", "", [call("render_chart", "c2")]), + toolResult("c2", "render_chart"), + ]); + + expect(groups(items)).toHaveLength(2); + expect(items.map((i) => i.kind)).toEqual(["tools", "message", "tools"]); + }); + + it("marks a paused call pending and leaves its result null", () => { + // A gated call has NO result message — pairing by adjacency would mis-assign the next one. + const items = buildThreadItems([ + ai("a1", "", [call("log_expense", "c1", { amountMinor: 450 })]), + pendingAi("a1", ["c1"]), + ]); + + const [group] = groups(items); + expect(group.activities[0]).toMatchObject({ + callId: "c1", + isPending: true, + isMutating: true, + result: null, + }); + }); + + it("clears pending once the call has actually run", () => { + const items = buildThreadItems([ + ai("a1", "", [call("log_expense", "c1")]), + pendingAi("a1", ["c1"]), + toolResult("c1", "log_expense"), + ]); + + const [group] = groups(items); + // A result settles it: approving must not leave the gate showing. + expect(group.activities[0].isPending).toBe(false); + expect(group.activities[0].result).not.toBeNull(); + }); + + it("treats a call with no result yet as running, not pending", () => { + const items = buildThreadItems([ai("a1", "", [call("run_sql", "c1")])]); + const [group] = groups(items); + expect(group.activities[0]).toMatchObject({ isPending: false, result: null }); + }); + + it("keeps a result whose call never streamed rather than dropping it", () => { + const items = buildThreadItems([toolResult("orphan", "run_sql")]); + const [group] = groups(items); + expect(group.activities[0]).toMatchObject({ callId: "orphan", name: "run_sql" }); + }); + + it("renders human and error messages untouched", () => { + const items = buildThreadItems([ + { type: "human", data: { id: "h1", content: "hi" } }, + { type: "error", data: { id: "e1", content: "boom" } }, + ]); + expect(items.map((i) => i.kind)).toEqual(["message", "message"]); + }); + + it("drops an AI message that carries neither text nor calls", () => { + const items = buildThreadItems([ai("a1", "")]); + expect(items).toHaveLength(0); + }); + + it("keeps text and its own tool calls together in order", () => { + const items = buildThreadItems([ + ai("a1", "Let me check.", [call("run_sql", "c1")]), + toolResult("c1", "run_sql"), + ]); + expect(items.map((i) => i.kind)).toEqual(["message", "tools"]); + }); + + it("deduplicates by type+id without dropping a distinct message", () => { + // An AI message and a tool message can share an id; they are different namespaces. + const items = buildThreadItems([ + ai("dup", "hello"), + ai("dup", "hello"), + { type: "human", data: { id: "dup", content: "hi" } }, + ]); + expect(items).toHaveLength(2); + }); + + it("treats a replayed calls-only message as having no text", () => { + // A reloaded thread stores content as an array whose blocks include `tool_call` entries. + // Counting array length instead of extracting text would split the group and render an + // empty bubble — verified against real /api/agent/history output. + const replayed: MessageResponse = { + type: "ai", + data: { + id: "a1", + content: [{ type: "tool_call", name: "run_sql", args: {}, id: "c1" }] as never, + tool_calls: [call("run_sql", "c1")], + }, + }; + + const items = buildThreadItems([replayed, toolResult("c1", "run_sql")]); + expect(items.map((i) => i.kind)).toEqual(["tools"]); + }); + + it("keeps a replayed text+calls message as one message plus one group", () => { + const replayed: MessageResponse = { + type: "ai", + data: { + id: "a1", + content: [ + { type: "text", text: "Let me check." }, + { type: "tool_call", name: "run_sql", args: {}, id: "c1" }, + ] as never, + tool_calls: [call("run_sql", "c1")], + }, + }; + + const items = buildThreadItems([replayed, toolResult("c1", "run_sql")]); + expect(items.map((i) => i.kind)).toEqual(["message", "tools"]); + }); +}); + +describe("summarizeActivities", () => { + const activity = (over: Partial[0][number]> = {}) => ({ + callId: "c1", + name: "run_sql", + args: {}, + result: toolResult("c1", "run_sql", '{"rowCount":37}'), + isPending: false, + isMutating: false, + ...over, + }); + + it("gives a single call its own gist", () => { + expect(summarizeActivities([activity()])).toBe("run_sql · 37 rows"); + }); + + it("counts instead of guessing when several tools ran", () => { + // No summary spans differently-shaped payloads, so counting is the only honest claim. + expect(summarizeActivities([activity(), activity({ callId: "c2" })])).toBe("2 tools called"); + }); + + it("names a single call with no readable gist", () => { + expect( + summarizeActivities([ + activity({ name: "load_skill", result: toolResult("c1", "load_skill", "plain text") }), + ]), + ).toBe("load_skill"); + }); + + it("reports a pending call as awaiting approval", () => { + expect(summarizeActivities([activity({ isPending: true, result: null })])).toBe( + "awaiting approval", + ); + }); + + it("reports an unfinished call as running", () => { + expect(summarizeActivities([activity({ result: null })])).toBe("running…"); + }); + + it("says 'written' only for a tool that actually writes", () => { + // `ok: true` is the same payload either way; only the tool says whether anything was written. + expect( + summarizeActivities([ + activity({ name: "log_expense", result: toolResult("c1", "log_expense", '{"ok":true}') }), + ]), + ).toBe("log_expense · written"); + expect( + summarizeActivities([ + activity({ name: "load_skill", result: toolResult("c1", "load_skill", '{"ok":true}') }), + ]), + ).toBe("load_skill · ok"); + }); +}); diff --git a/src/services/toolActivity.ts b/src/services/toolActivity.ts new file mode 100644 index 0000000..41a0ccd --- /dev/null +++ b/src/services/toolActivity.ts @@ -0,0 +1,197 @@ +import type { MessageResponse, ToolCall } from "@/types/message"; +import { isMutatingTool } from "@/lib/agent/mutatingTools"; +import { + getMessageContent, + getPendingToolCallIds, + getToolCalls, + isToolMessage, +} from "./messageUtils"; + +/** + * Pairs each tool CALL with the RESULT that answers it, so one operation renders as one card. + * + * The wire delivers them as two separate messages — the call on an AI message, the result as its + * own `tool` message — which is why the UI used to draw two. Pairing is by `tool_call_id`, never + * by adjacency: results arrive when they resolve, and a paused call never gets one at all. + */ + +/** One tool call and whatever is known about its outcome. */ +export interface ToolActivity { + callId: string; + name: string; + args: Record; + /** The result message, or null while the call is still running or awaiting approval. */ + result: MessageResponse | null; + /** Held at the approval gate: the user must decide before anything runs. */ + isPending: boolean; + /** Writes, per `MUTATING_TOOL_NAMES`. Its arguments are always shown. */ + isMutating: boolean; +} + +/** A consecutive run of tool calls, rendered as a single collapsible card. */ +export interface ToolActivityGroup { + kind: "tools"; + id: string; + activities: ToolActivity[]; +} + +export interface PlainMessageItem { + kind: "message"; + id: string; + message: MessageResponse; +} + +export type ThreadItem = ToolActivityGroup | PlainMessageItem; + +/** + * Fold a repeated message's fields into the one already kept. Only additive fields are copied — + * a later chunk never blanks text the first one carried. + */ +function mergeInto(target: MessageResponse, extra: MessageResponse): void { + const from = extra.data as unknown as Record | undefined; + const into = target.data as unknown as Record; + if (!from) return; + if (Array.isArray(from.tool_calls) && from.tool_calls.length > 0) { + into.tool_calls = from.tool_calls; + } + if (Array.isArray(from.pendingToolCallIds) && from.pendingToolCallIds.length > 0) { + into.pendingToolCallIds = from.pendingToolCallIds; + } + if (typeof from.content === "string" && from.content.trim() !== "" && !into.content) { + into.content = from.content; + } +} + +/** + * AI text worth rendering, ignoring tool calls riding on the same message. + * + * Uses the SAME extraction the renderer uses. A replayed history stores content as an array whose + * blocks include `tool_call` entries, so checking array LENGTH would count a calls-only message as + * text — breaking the group and rendering an empty bubble. + */ +function hasVisibleText(message: MessageResponse): boolean { + return getMessageContent(message).trim() !== ""; +} + +/** + * Flattens a thread into render-ready items, grouping consecutive tool calls. + * + * Results are consumed out of the list by id, so a result never renders on its own — that + * double-rendering is what the old two-component layout did. + */ +export function buildThreadItems(input: MessageResponse[]): ThreadItem[] { + // Dedup by type+id: ids are unique only WITHIN a type (an AI message's synthetic id and a tool + // message's call id are different namespaces), so keying on id alone can drop a real message. + // + // Repeats are MERGED rather than discarded. The server sends the pending-approval marker as its + // own chunk bearing the id of the message that made the call; dropping it outright would throw + // the approval gate away in any shape where that chunk stayed a separate message. + const byKey = new Map(); + const messages: MessageResponse[] = []; + for (const message of input) { + const id = message.data?.id; + if (!id) { + messages.push(message); + continue; + } + const key = `${message.type}:${id}`; + const existing = byKey.get(key); + if (!existing) { + const copy = { ...message, data: { ...message.data } } as MessageResponse; + byKey.set(key, copy); + messages.push(copy); + continue; + } + mergeInto(existing, message); + } + + // Index every result by the call it answers, so a call can find its result wherever it landed. + const resultsByCallId = new Map(); + const knownCallIds = new Set(); + // Pending ids are collected across the WHOLE thread, not per message. Streaming merges the + // pending chunk into the message that made the call, but a replayed history can carry it on a + // separate one — reading it per message would silently lose the gate in that shape. + const pendingCallIds = new Set(); + for (const message of messages) { + if (isToolMessage(message)) resultsByCallId.set(message.data.tool_call_id, message); + else if (message.type === "ai") { + for (const call of getToolCalls(message)) knownCallIds.add(call.id); + for (const id of getPendingToolCallIds(message)) pendingCallIds.add(id); + } + } + + const items: ThreadItem[] = []; + let openGroup: ToolActivityGroup | null = null; + + const pushActivity = (activity: ToolActivity, groupId: string) => { + if (!openGroup) { + openGroup = { kind: "tools", id: groupId, activities: [] }; + items.push(openGroup); + } + openGroup.activities.push(activity); + }; + + for (const [index, message] of messages.entries()) { + if (isToolMessage(message)) { + // Already rendered inside its call's card. An orphan (a result whose call never streamed) + // still deserves a card rather than vanishing. + const callId = message.data.tool_call_id; + if (knownCallIds.has(callId)) continue; + pushActivity( + { + callId, + name: message.data.name, + args: {}, + result: message, + isPending: false, + isMutating: isMutatingTool(message.data.name), + }, + `tools-${message.data.id || index}`, + ); + continue; + } + + if (message.type === "ai") { + const toolCalls = getToolCalls(message); + + if (hasVisibleText(message)) { + // Text ends any open run: the agent said something between the calls. + openGroup = null; + items.push({ kind: "message", id: `ai-${message.data?.id || index}`, message }); + } else if (toolCalls.length === 0) { + // No text and no calls: nothing a reader can see. + continue; + } + + for (const call of toolCalls) { + pushActivity( + toActivity(call, resultsByCallId, pendingCallIds), + `tools-${message.data?.id || index}`, + ); + } + continue; + } + + openGroup = null; + items.push({ kind: "message", id: `${message.type}-${message.data?.id || index}`, message }); + } + + return items; +} + +function toActivity( + call: ToolCall, + resultsByCallId: Map, + pending: Set, +): ToolActivity { + const result = resultsByCallId.get(call.id) ?? null; + return { + callId: call.id, + name: call.name, + args: call.args ?? {}, + result, + // A result settles the question: a call that ran is no longer awaiting a decision. + isPending: result === null && pending.has(call.id), + isMutating: isMutatingTool(call.name), + }; +} diff --git a/src/services/toolSummary.ts b/src/services/toolSummary.ts new file mode 100644 index 0000000..cbc6167 --- /dev/null +++ b/src/services/toolSummary.ts @@ -0,0 +1,57 @@ +import type { MessageResponse } from "@/types/message"; +import type { ToolActivity } from "./toolActivity"; +import { isMutatingTool } from "@/lib/agent/mutatingTools"; + +/** + * The one-line gist shown on a collapsed tool card. + * + * A single call can be described precisely — its own payload says what came back. A run of several + * calls cannot: the payloads have different shapes and no useful summary spans them, so the card + * reports the COUNT instead of inventing a combined figure. + */ + +function contentToString(content: MessageResponse["data"]["content"]): string { + if (!content) return ""; + return typeof content === "string" ? content : JSON.stringify(content); +} + +/** + * Read the gist out of the shapes the finance tools actually return. + * + * `toolName` decides how `ok: true` reads: only a MUTATING tool wrote anything. Saying "written" + * for a read-only tool claims a side effect that never happened, and in a finance agent that word + * is load-bearing. + */ +export function summarizeResult(content: string, toolName?: string): string | null { + try { + const p = JSON.parse(content) as Record; + if (typeof p.error === "string") return p.error; + if (typeof p.rowCount === "number") return `${p.rowCount} rows`; + if (typeof p.matched === "number" && typeof p.returned === "number") { + return p.returned === p.matched + ? `${p.matched} transactions` + : `${p.returned} of ${p.matched} transactions`; + } + if (p.ok === true) return toolName && !isMutatingTool(toolName) ? "ok" : "written"; + return null; + } catch { + return null; + } +} + +/** Label for a group's collapsed header. */ +export function summarizeActivities(activities: ToolActivity[]): string { + if (activities.length === 0) return "no tools"; + + if (activities.length > 1) { + // Deliberately generic — see the module note. Counting is the only honest claim here. + return `${activities.length} tools called`; + } + + const [only] = activities; + if (only.isPending) return "awaiting approval"; + if (!only.result) return "running…"; + + const gist = summarizeResult(contentToString(only.result.data?.content), only.name); + return gist ? `${only.name} · ${gist}` : only.name; +}