Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions src/tui/diff-rows.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,64 @@ describe("diff transcript rows", () => {
}, WIDE)
})

test("a task/dispatch call paints a sentence, never the full spawn JSON (CL-5762)", async () => {
const brief = {
agent: "explore",
description: "map callers of leaveObserve",
prompt: "Find every call site of leaveObserve.\nReport paths and line numbers.",
intent: "explore",
maxTurns: 40,
success_criteria: ["list call sites", "note tests"],
do_not: ["edit code", "open PRs"],
}
const args = JSON.stringify(brief)
const row = toolCallRow({ name: "task", arguments: args })

// Structural: summary set, not raw args; detail expands with real newlines.
expect(row.summary).toBe("map callers of leaveObserve")
expect(row.verb).toBe("Explore")
expect(row.text).toBe(args) // clipboard still has raw; paint must not use it
expect(row.summary).not.toContain("success_criteria")
expect(row.summary).not.toContain("maxTurns")
// Expanded body uses real line breaks, not literal \\n escape sequences.
const detailPlain = (row.detail ?? [])
.map((line) => line.map((s) => s.text).join(""))
.join("\n")
expect(detailPlain).toContain("Find every call site of leaveObserve.")
expect(detailPlain).toContain("Report paths and line numbers.")
// A pretty-printed JSON dump would keep \\n inside the prompt string.
expect(detailPlain).not.toContain("\\n")
expect(detailPlain).toContain("list call sites")

await withTestRenderer(async (h) => {
const shell = createAppShell(h.renderer, shellOpts)
appendStreamRow(shell, row)
await settle(h)
const frame = h.captureCharFrame()
expect(frame).toContain("map callers of leaveObserve")
expect(frame).not.toContain('"maxTurns"')
expect(frame).not.toContain('"success_criteria"')
expect(frame).not.toContain(args.slice(0, 40))
}, WIDE)
})

test("a task without description still collapses — falls back to prompt, not raw JSON", () => {
const prompt = "Find every call site of leaveObserve and report them."
const args = JSON.stringify({
agent: "explore",
prompt,
intent: "explore",
success_criteria: ["list sites"],
})
const row = toolCallRow({ name: "task", arguments: args })
expect(row.summary).toBeDefined()
expect(row.summary!.length).toBeGreaterThan(0)
expect(row.summary).not.toContain("success_criteria")
expect(row.summary).not.toContain('"intent"')
// Paint layer must not fall through to raw text.
expect(row.summary).not.toBe(args)
})

test("a write_file call paints the whole body as additions", async () => {
await withTestRenderer(async (h) => {
const shell = createAppShell(h.renderer, shellOpts)
Expand Down
16 changes: 10 additions & 6 deletions src/tui/diff.ts
Original file line number Diff line number Diff line change
Expand Up @@ -488,6 +488,15 @@ export function toolCallRow(input: ToolCallRowInput): StreamRow {
// Identity of the sentence this call paints, not of its arguments: two calls
// that read the same line are what a repeat looks like to the operator.
const callKey = `${input.name} ${verb ?? ""} ${summary ?? ""}`
// Never leave `summary` unset when we have a verb or a summarised view —
// `undefined` makes the paint layer fall through to raw argument JSON
// (CL-5762). An empty string is fine: the verb alone names the call.
const paintSummary =
summary !== undefined
? summary
: call !== null || summarised !== null
? ""
: undefined
return {
role: "tool",
text,
Expand All @@ -497,12 +506,7 @@ export function toolCallRow(input: ToolCallRowInput): StreamRow {
...(input.callId !== undefined ? { callId: input.callId } : {}),
...(diff !== null ? { diff } : {}),
...(verb !== undefined ? { verb } : {}),
// A summarised call may deliberately have no subject — its verb already
// names the whole call — and that blank must survive, or the row falls
// back to painting the raw arguments.
...(summary !== undefined && (summary.length > 0 || summarised !== null)
? { summary }
: {}),
...(paintSummary !== undefined ? { summary: paintSummary } : {}),
...(stat !== undefined ? { stat } : {}),
...(detail !== undefined && detail.length > 0 ? { detail } : {}),
}
Expand Down
3 changes: 3 additions & 0 deletions src/tui/runner-host.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,9 @@ describe("rowFromTranscriptEntry", () => {
text: "{}",
meta: "grep",
verb: "Grep",
// Empty summary is intentional: without it the paint layer falls through
// to raw argument JSON (CL-5762). Verb alone names the call.
summary: "",
pending: true,
callKey: "grep Grep ",
callId: "c",
Expand Down
98 changes: 73 additions & 25 deletions src/tui/tool-args.ts
Original file line number Diff line number Diff line change
Expand Up @@ -124,29 +124,73 @@ function isScalar(value: unknown): boolean {
}

/**
* Scalar arguments as `key value` pairs with their newlines intact — a shell
* command or a prompt is written to be read as text, and pretty-printed JSON
* would hand it back with its line breaks escaped.
* Scalar (or scalar-array) arguments as `key value` pairs with their newlines
* intact — a shell command or a spawn prompt is written to be read as text, and
* pretty-printed JSON would hand it back with its line breaks escaped (CL-5762).
*
* Nested objects recurse one level so a task brief expands as fields rather than
* a JSON dump; deeper nesting collapses to a compact token.
*/
function scalarDetail(args: Record<string, unknown>): readonly StyledBodyLine[] | null {
const entries = Object.entries(args)
if (entries.length === 0 || !entries.every(([, value]) => isScalar(value))) {
return null
}
function fieldDetail(
args: Record<string, unknown>,
indent = 0,
): readonly StyledBodyLine[] {
const pad = " ".repeat(indent)
const lines: StyledBodyLine[] = []
for (const [key, value] of entries) {
const text = typeof value === "string" ? value : JSON.stringify(value)
const rows = (text ?? "null").split("\n")
rows.forEach((row, i) => {
lines.push(
i === 0
? [
{ text: `${key}: `, fg: UI.textDim },
{ text: row, fg: UI.text },
]
: [{ text: `${" ".repeat(key.length + 2)}${row}`, fg: UI.text }],
)
})
for (const [key, value] of Object.entries(args)) {
if (isScalar(value)) {
const text = typeof value === "string" ? value : JSON.stringify(value)
const rows = (text ?? "null").split("\n")
rows.forEach((row, i) => {
lines.push(
i === 0
? [
{ text: `${pad}${key}: `, fg: UI.textDim },
{ text: row, fg: UI.text },
]
: [{ text: `${pad}${" ".repeat(key.length + 2)}${row}`, fg: UI.text }],
)
})
continue
}
if (Array.isArray(value) && value.every(isScalar)) {
if (value.length === 0) {
lines.push([
{ text: `${pad}${key}: `, fg: UI.textDim },
{ text: "[]", fg: UI.text },
])
continue
}
lines.push([{ text: `${pad}${key}:`, fg: UI.textDim }])
for (const item of value) {
const text = typeof item === "string" ? item : JSON.stringify(item)
for (const row of text.split("\n")) {
lines.push([{ text: `${pad} - ${row}`, fg: UI.text }])
}
}
continue
}
if (typeof value === "object" && value !== null && !Array.isArray(value)) {
// One level of nesting is enough for a spawn brief; deeper stays compact.
if (indent === 0) {
lines.push([{ text: `${pad}${key}:`, fg: UI.textDim }])
lines.push(...fieldDetail(value as Record<string, unknown>, indent + 2))
} else {
lines.push([
{ text: `${pad}${key}: `, fg: UI.textDim },
{ text: "{…}", fg: UI.text },
])
}
continue
}
// Arrays of objects, etc. — compact rather than a wall of JSON.
lines.push([
{ text: `${pad}${key}: `, fg: UI.textDim },
{
text: Array.isArray(value) ? `[${value.length} items]` : "{…}",
fg: UI.text,
},
])
}
return lines.slice(0, MAX_DETAIL_LINES)
}
Expand Down Expand Up @@ -221,7 +265,9 @@ function subjectFor(
args: Record<string, unknown>,
): string {
const { summary } = summarizeToolArgs(name, raw)
if (!isArgumentList(args, summary)) return summary
// An empty formatter summary is not a subject — fall through to primarySubject
// so a task without description still paints its prompt rather than raw JSON.
if (summary.length > 0 && !isArgumentList(args, summary)) return summary
return primarySubject(args) ?? summary
}

Expand All @@ -246,7 +292,7 @@ export function toolArgsView(name: string, rawArgs: string): ToolArgsView | null
// Its arguments are a query, not a subject: nobody reads a transcript for
// the pagination cursor, so they belong behind the expand key or nowhere.
if (args !== null && isMcpToolName(name)) {
return withDetail("", scalarDetail(args) ?? jsonDetail(args))
return withDetail("", fieldDetail(args))
}

if (args === null && raw.length <= INLINE_MAX && !raw.includes("\n")) return null
Expand All @@ -256,8 +302,10 @@ export function toolArgsView(name: string, rawArgs: string): ToolArgsView | null
return summary.length === 0 ? null : withDetail(summary, jsonDetail(raw))
}
const subject = subjectFor(name, raw, args)
if (subject.length === 0) return null
return withDetail(subject, scalarDetail(args) ?? jsonDetail(args))
// Object args always get a summarised view — even with an empty subject the
// verb alone names the call and the body expands with real line breaks. A
// null return here is what used to dump raw argument JSON into the transcript.
return withDetail(subject, fieldDetail(args))
}

/**
Expand Down
25 changes: 25 additions & 0 deletions src/tui/tool-formatter.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -285,6 +285,18 @@ describe("describeToolCall for task tool", () => {
expect(result.summary).toBe("map all callers");
});

test("task without description falls back to the prompt subject", () => {
const prompt = "Find every call site of leaveObserve and report them.";
const args = JSON.stringify({ agent: "explore", prompt, intent: "explore" });
const result = describeToolCall("task", args);
expect(result.display).toBe("Explore");
// ARG_VALUE_MAX = 48 with ellipsis when truncated
expect(result.summary.length).toBeLessThanOrEqual(48);
expect(result.full).toBe(prompt);
expect(result.summary.startsWith("Find every call site")).toBe(true);
expect(result.summary).not.toContain("intent");
});

test("long description is abbreviated", () => {
const long = "a".repeat(100);
const args = JSON.stringify({ agent: "critique", description: long, prompt: "..." });
Expand Down Expand Up @@ -333,6 +345,19 @@ describe("task activity transcript lines", () => {
expect(s.full).toBe("map callers of leaveObserve");
});

test("summarizeToolArgs falls back to prompt when description is missing", () => {
const prompt = "Find every call site of leaveObserve and report them with paths.";
const s = summarizeToolArgs(
"task",
JSON.stringify({ agent: "explore", prompt, intent: "explore", maxTurns: 40 }),
);
expect(s.summary.length).toBeLessThanOrEqual(48);
expect(s.full).toBe(prompt);
expect(s.summary.startsWith("Find every call site")).toBe(true);
expect(s.summary).not.toContain("maxTurns");
expect(s.summary).not.toContain("intent");
});

test("describeToolCall full keeps the untrimmed description for Ctrl+O", () => {
const long = "a".repeat(80);
const d = describeToolCall(
Expand Down
24 changes: 18 additions & 6 deletions src/tui/tool-formatter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,16 +132,20 @@ export function describeToolCall(toolName: string, rawArgs: string): ToolCallDes
if (!(taskParsed instanceof type.errors)) {
const agentName = taskParsed.agent?.trim();
const description = (taskParsed.description ?? "").trim();
// description is optional on spawn; the brief's prompt is the next best
// subject so the row never falls through to raw argument JSON.
const prompt = (taskParsed.prompt ?? "").trim();
const subject = description.length > 0 ? description : prompt;
const display =
agentName !== undefined && agentName.length > 0
? agentName[0]!.toUpperCase() + agentName.slice(1)
: "Task";
// Collapsed row uses the abbreviated description; Alt+E expands to the full text.
// Collapsed row uses the abbreviated subject; Alt+E expands to the full text.
return {
display,
role: "accent",
summary: description.length > 0 ? abbreviate(description, ARG_VALUE_MAX) : "",
full: description,
summary: subject.length > 0 ? abbreviate(subject, ARG_VALUE_MAX) : "",
full: subject,
isShell: false,
};
}
Expand Down Expand Up @@ -183,7 +187,11 @@ const SearchFilesArgSchema = type({ pattern: "string", "path?": "string" });
const WebSearchArgSchema = type({ query: "string" });
const WebFetchArgSchema = type({ url: "string" });
const ShellArgSchema = type({ command: "string" });
const TaskArgSchema = type({ "agent?": "string", "description?": "string" });
const TaskArgSchema = type({
"agent?": "string",
"description?": "string",
"prompt?": "string",
});
const WebSearchResultSchema = type({ results: "unknown[]" });
const WebFetchResultSchema = type({ content: "string" });

Expand Down Expand Up @@ -234,14 +242,18 @@ export function summarizeToolArgs(toolName: string, rawArgs: string): ToolArgSum
}
case "task": {
// Spawns carry a large structured brief (prompt, intent, criteria). The
// transcript only needs the short description; Alt+E still shows the
// full description text, not every spawn field.
// transcript only needs a short subject — prefer description, then prompt —
// so the row never dumps the whole JSON payload.
const parsed = TaskArgSchema(obj);
if (!(parsed instanceof type.errors)) {
const desc = (parsed.description ?? "").trim();
if (desc.length > 0) {
return { summary: abbreviate(desc, ARG_VALUE_MAX), full: desc };
}
const prompt = (parsed.prompt ?? "").trim();
if (prompt.length > 0) {
return { summary: abbreviate(prompt, ARG_VALUE_MAX), full: prompt };
}
}
return { summary: "", full: "" };
}
Expand Down
Loading