mirror of
https://github.com/block/buzz.git
synced 2026-11-03 07:51:22 +00:00
fix(desktop): paint ACP summary titles on action-bearing tool rows
Marge's review caught that the friendly summaries never reached the screen for the tools that matter most: getCompactToolActionLabel took the structured action-descriptor branch first and never consulted the label where summaryTitle lived, so shell/file-read rows kept rendering raw 'Ran <command>' descriptors after the friendly title landed. buildCompactToolSummary now exposes summaryTitle (null when the row failed) and failed alongside label, and CompactToolSummaryRow renders the friendly phrase as the visible row label ahead of the action descriptor. The exact command/path receipt stays on the hover title and on expand. Failed rows bypass the descriptor branch entirely so the failure label is painted instead of the descriptor repainting 'Ran <command>' over it — this also fixes the previously-dead failure precedence for action-bearing tools that Marge flagged as a minor. New render-layer tests (renderToStaticMarkup through the real buildCompactToolSummary pipeline) assert the user-visible HTML: the friendly title paints for shell and file-read rows, failure beats the friendly phrase, and rows without a summary keep today's descriptor label. Co-authored-by: Taylor Ho <taylorkmho@gmail.com> Signed-off-by: Taylor Ho <taylorkmho@gmail.com>
This commit is contained in:
parent
a04972c448
commit
8720b5d1ef
@@ -0,0 +1,121 @@
|
||||
import assert from "node:assert/strict";
|
||||
import test from "node:test";
|
||||
|
||||
import React from "react";
|
||||
import { renderToStaticMarkup } from "react-dom/server";
|
||||
|
||||
import { buildCompactToolSummary } from "../agentSessionToolSummary.ts";
|
||||
import { CompactToolSummaryRow } from "./CompactToolSummaryRow.tsx";
|
||||
|
||||
const baseTimestamp = "2026-06-14T19:00:00.000Z";
|
||||
|
||||
function makeTool(overrides = {}) {
|
||||
return {
|
||||
id: "tool:1",
|
||||
type: "tool",
|
||||
title: "Tool call",
|
||||
toolName: "shell",
|
||||
buzzToolName: null,
|
||||
status: "completed",
|
||||
args: {},
|
||||
result: "",
|
||||
isError: false,
|
||||
timestamp: baseTimestamp,
|
||||
startedAt: baseTimestamp,
|
||||
completedAt: "2026-06-14T19:00:01.000Z",
|
||||
...overrides,
|
||||
};
|
||||
}
|
||||
|
||||
/** Build the row via the real summary pipeline, then render it to HTML. */
|
||||
function renderRow(item) {
|
||||
const summary = buildCompactToolSummary(item);
|
||||
return renderToStaticMarkup(
|
||||
React.createElement(CompactToolSummaryRow, {
|
||||
action: summary.action,
|
||||
duration: null,
|
||||
failed: summary.failed,
|
||||
fileEditSummary: summary.fileEditSummary,
|
||||
kind: summary.kind,
|
||||
label: summary.label,
|
||||
preview: summary.preview,
|
||||
summaryTitle: summary.summaryTitle,
|
||||
thumbnailSrc: summary.thumbnailSrc,
|
||||
}),
|
||||
);
|
||||
}
|
||||
|
||||
test("render: friendly summary title is the visible label for an action-bearing shell row", () => {
|
||||
const html = renderRow(
|
||||
makeTool({
|
||||
toolName: "developer__shell",
|
||||
args: { command: "git status" },
|
||||
summaryTitle: "checking repository state",
|
||||
}),
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
html.includes("checking repository state"),
|
||||
`friendly title must be painted, got: ${html}`,
|
||||
);
|
||||
// The structured "Ran <command>" descriptor must not win the row label.
|
||||
assert.ok(
|
||||
!html.includes(">Ran<"),
|
||||
`descriptor verb must not render: ${html}`,
|
||||
);
|
||||
// Receipt stays available: the exact command rides the hover title.
|
||||
assert.ok(
|
||||
html.includes('title="git status"'),
|
||||
`exact command receipt must stay on hover: ${html}`,
|
||||
);
|
||||
});
|
||||
|
||||
test("render: friendly summary title is the visible label for an action-bearing file-read row", () => {
|
||||
const html = renderRow(
|
||||
makeTool({
|
||||
toolName: "read_file",
|
||||
args: { path: "crates/buzz-agent/src/agent.rs" },
|
||||
summaryTitle: "reading agent source module",
|
||||
}),
|
||||
);
|
||||
|
||||
assert.ok(html.includes("reading agent source module"));
|
||||
assert.ok(
|
||||
!html.includes(">Read<"),
|
||||
`descriptor verb must not render: ${html}`,
|
||||
);
|
||||
});
|
||||
|
||||
test("render: failed shell row ignores the friendly title and paints the failure label", () => {
|
||||
const html = renderRow(
|
||||
makeTool({
|
||||
toolName: "developer__shell",
|
||||
args: { command: "false" },
|
||||
status: "failed",
|
||||
isError: true,
|
||||
result: "exit 1",
|
||||
summaryTitle: "running a quick command",
|
||||
}),
|
||||
);
|
||||
|
||||
assert.ok(
|
||||
!html.includes("running a quick command"),
|
||||
`friendly phrase must not mask a failure: ${html}`,
|
||||
);
|
||||
assert.ok(
|
||||
html.includes("failed"),
|
||||
`failure label must be visible, got: ${html}`,
|
||||
);
|
||||
});
|
||||
|
||||
test("render: rows without a summary keep today's descriptor label", () => {
|
||||
const html = renderRow(
|
||||
makeTool({
|
||||
toolName: "developer__shell",
|
||||
args: { command: "git status" },
|
||||
}),
|
||||
);
|
||||
|
||||
assert.ok(html.includes(">Ran<"), `descriptor verb should render: ${html}`);
|
||||
assert.ok(html.includes("git status"));
|
||||
});
|
||||
@@ -22,18 +22,22 @@ export function compactSummaryTone() {
|
||||
export function CompactToolSummaryRow({
|
||||
action,
|
||||
duration,
|
||||
failed,
|
||||
fileEditSummary,
|
||||
kind,
|
||||
label,
|
||||
preview,
|
||||
summaryTitle,
|
||||
thumbnailSrc,
|
||||
}: {
|
||||
action: AgentActivityAction | null;
|
||||
duration: string | null;
|
||||
failed: boolean;
|
||||
fileEditSummary: CompactFileEditSummary | null;
|
||||
kind: CompactToolKind;
|
||||
label: string;
|
||||
preview: string | null;
|
||||
summaryTitle: string | null;
|
||||
thumbnailSrc: string | null;
|
||||
}) {
|
||||
const [thumbnailFailed, setThumbnailFailed] = React.useState(false);
|
||||
@@ -44,14 +48,30 @@ export function CompactToolSummaryRow({
|
||||
if (!thumbnailSrc || thumbnailFailed) return null;
|
||||
return resolveToolImageSrc(thumbnailSrc);
|
||||
}, [thumbnailFailed, thumbnailSrc]);
|
||||
const actionLabel = fileEditSummary
|
||||
? null
|
||||
: getCompactToolActionLabel(action, kind, label, preview);
|
||||
// Failed rows bypass the structured action descriptor entirely: the
|
||||
// descriptor would repaint "Ran <command>" and swallow the failure label.
|
||||
const actionLabel =
|
||||
fileEditSummary || failed
|
||||
? null
|
||||
: getCompactToolActionLabel(action, kind, label, preview);
|
||||
|
||||
return (
|
||||
<>
|
||||
{fileEditSummary ? (
|
||||
<CompactFileEditSummaryView summary={fileEditSummary} />
|
||||
) : summaryTitle && !failed ? (
|
||||
// The agent-provided friendly phrase wins the visible row label.
|
||||
// The exact command/path receipt stays on hover and on expand.
|
||||
<span
|
||||
className={cn(
|
||||
"min-w-0 truncate font-semibold",
|
||||
isCompactPreview ? "text-xs" : "text-sm",
|
||||
mutedTone,
|
||||
)}
|
||||
title={preview ?? undefined}
|
||||
>
|
||||
{summaryTitle}
|
||||
</span>
|
||||
) : actionLabel ? (
|
||||
<ActivityRowLabel
|
||||
object={actionLabel.object}
|
||||
|
||||
@@ -120,9 +120,11 @@ export function ToolItem({
|
||||
<CompactToolSummaryRow
|
||||
action={compactSummary.action}
|
||||
duration={duration}
|
||||
failed={compactSummary.failed}
|
||||
fileEditSummary={compactSummary.fileEditSummary}
|
||||
kind={compactSummary.kind}
|
||||
preview={compactSummary.preview}
|
||||
summaryTitle={compactSummary.summaryTitle}
|
||||
thumbnailSrc={compactSummary.thumbnailSrc}
|
||||
label={compactSummary.label}
|
||||
/>
|
||||
|
||||
@@ -42,6 +42,14 @@ export type CompactToolSummary = {
|
||||
action: AgentActivityAction | null;
|
||||
kind: CompactToolKind;
|
||||
label: string;
|
||||
/**
|
||||
* Agent-provided friendly phrase (Buzz ACP tool summary) when it won the
|
||||
* row label. Null for failed rows — failure labels always win. The render
|
||||
* layer must prefer this over the structured `action` descriptor.
|
||||
*/
|
||||
summaryTitle: string | null;
|
||||
/** True when the tool failed; the render layer must keep failure visible. */
|
||||
failed: boolean;
|
||||
preview: string | null;
|
||||
fileEditSummary: FileEditDiffSummary | null;
|
||||
fileEditDiff: FileEditDiff | null;
|
||||
@@ -81,12 +89,14 @@ export function buildCompactToolSummary(item: ToolItem): CompactToolSummary {
|
||||
const statusLabel = labelForStatus(descriptor, item.status, failed, running);
|
||||
// Prefer the agent-provided friendly phrase (Buzz ACP tool summary) as the
|
||||
// row label — but failure labels always win so errors stay unmistakable.
|
||||
const summaryTitle = item.summaryTitle?.trim();
|
||||
const label = !failed && summaryTitle ? summaryTitle : statusLabel;
|
||||
const summaryTitle = (!failed && item.summaryTitle?.trim()) || null;
|
||||
const label = summaryTitle ?? statusLabel;
|
||||
return {
|
||||
action: descriptor.action ?? null,
|
||||
kind: descriptor.renderClass,
|
||||
label,
|
||||
summaryTitle,
|
||||
failed,
|
||||
preview: fileEditSummary?.filename ?? descriptor.preview,
|
||||
fileEditSummary,
|
||||
fileEditDiff,
|
||||
|
||||
Reference in New Issue
Block a user