Skip to content

bugfix: stop highlighting process tool errors #541

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
Grace-dev11 wants to merge 1 commit into from
Closed

bugfix: stop highlighting process tool errors #541

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
62 changes: 7 additions & 55 deletions App/frontend/desktop/src/pages/agent-thread-messages.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2234,7 +2234,7 @@ function ToolGroupSegmentLabel(props: { segment: ActivityToolGroupSegment }) {
/**
* Expanded tool payload. The clickable row above is already the summary, so
* the card only shows raw input/output as one bordered code card — arguments
* dimmed, result normal, errors tinted — with no form labels. This single
* dimmed, result and failures in the same ink — with no form labels. This single
* generic shape works for any tool, however exotic its payload.
*/
function ToolDetailRows(props: { item: ActivityToolStepItem }) {
Expand All @@ -2248,7 +2248,7 @@ function ToolDetailRows(props: { item: ActivityToolStepItem }) {
<div
key={`${detail.label}:${index}`}
data-detail={detail.label.toLowerCase()}
className={`agent-activity-tool-card__section${detail.tone === "error" ? " agent-activity-tool-card__section--error" : ""}`}
className="agent-activity-tool-card__section"
>
<pre className="agent-activity-tool-card__value">{detail.value}</pre>
</div>
Expand Down Expand Up @@ -2333,8 +2333,6 @@ const TRACE_CATEGORY_ICONS: Record<ToolTraceCategory, ComponentType<SVGProps<SVG

function TraceLine(props: { item: ActivityToolStepItem; t: Translate }) {
void props.t;
const phase = props.item.event.phase;
const isError = phase === "error" && !props.item.recovered;
const category = props.item.category;
const Icon = TRACE_CATEGORY_ICONS[category] ?? Wand2;
const summary = (
Expand All @@ -2345,13 +2343,10 @@ function TraceLine(props: { item: ActivityToolStepItem; t: Translate }) {
{props.item.verb ? <span className="agent-activity-timeline-item__verb">{props.item.verb}</span> : null}
{props.item.detail ? <span className="agent-activity-timeline-item__detail">{props.item.detail}</span> : null}
</p>
{isError && props.item.event?.error != null && (
<p className="agent-activity-timeline-item__error">{formatToolDetailValue(props.item.event.error)}</p>
)}
</div>
</>
);
const className = `agent-activity-timeline-item agent-activity-timeline-item--tool agent-activity-timeline-item--${category}${isError ? " agent-activity-timeline-item--error" : ""}`;
const className = `agent-activity-timeline-item agent-activity-timeline-item--tool agent-activity-timeline-item--${category}`;
if (props.item.details.length > 0) {
return (
<details className={`${className} agent-activity-tool-details`}>
Expand Down Expand Up @@ -2387,7 +2382,7 @@ function FileEditLine(props: { edit: AgentFileEdit; t: Translate }) {
const deleted = props.edit.deleted ?? 0;
const hasDiff = !isUnchanged && !props.edit.binary && (added > 0 || deleted > 0);
return (
<div className={`agent-activity-timeline-item agent-activity-timeline-item--file-edit agent-activity-timeline-item--edit${isError ? " agent-activity-timeline-item--error" : ""}`}>
<div className="agent-activity-timeline-item agent-activity-timeline-item--file-edit agent-activity-timeline-item--edit">
<Pencil size={13} aria-hidden="true" className="agent-activity-timeline-item__icon" />
<div className="agent-activity-timeline-item__body">
<p className="agent-activity-timeline-item__line">
Expand All @@ -2403,7 +2398,6 @@ function FileEditLine(props: { edit: AgentFileEdit; t: Translate }) {
<p className="agent-activity-timeline__status sr-only">
{isUnchanged ? "unchanged" : `${status} · ${props.edit.binary ? "binary" : `+${added} / -${deleted}`}`}
</p>
{props.edit.error && <p className="agent-activity-timeline-item__error">{props.edit.error}</p>}
</div>
</div>
);
Expand All @@ -2426,15 +2420,12 @@ interface ActivityToolStepItem {
category: ToolTraceCategory;
event: AgentToolProgressEvent;
details: ActivityToolDetail[];
/** A later invocation of the same tool in this activity run completed successfully. */
recovered?: boolean;
key: string;
}

interface ActivityToolDetail {
label: string;
value: string;
tone?: "error";
}

interface ActivityFileEditItem {
Expand Down Expand Up @@ -2502,48 +2493,9 @@ function buildActivitySegments(messages: AgentChatMessage[], t: Translate): Acti
appendToolGroupSegment(segments, `${messageKey}:toolgroup`, items, t);
}
});
markRecoveredToolErrors(segments);
return segments;
}

/** Keep raw failure details available on expansion, but do not present a successfully retried call as an active red error. */
function markRecoveredToolErrors(segments: ActivitySegment[]): void {
const steps = segments.flatMap((segment) => segment.type === "toolGroup"
? segment.items.filter((item): item is ActivityToolStepItem => item.type === "toolStep")
: []);
steps.forEach((step, index) => {
if (step.event.phase !== "error") return;
const name = toolEventName(step.event);
if (!name) return;
step.recovered = steps.slice(index + 1).some((candidate) => (
toolEventName(candidate.event) === name && toolEventSucceededForDisplay(candidate.event)
));
});
}

function toolEventSucceededForDisplay(event: AgentToolProgressEvent): boolean {
if (event.phase !== "end" || event.error != null) return false;
if (typeof event.result === "string") {
const result = event.result.trim();
if (/^(?:error|plugin_invalid|invalid|failed|failure)\s*:/iu.test(result)) return false;
if (result.startsWith("{")) {
try {
const parsed: unknown = JSON.parse(result);
if (parsed && typeof parsed === "object" && !Array.isArray(parsed)) {
const record = parsed as Record<string, unknown>;
if (record.ok === false || record.success === false || record.error) return false;
}
} catch {
// A non-JSON textual result may still represent a successful tool response.
}
}
} else if (event.result && typeof event.result === "object" && !Array.isArray(event.result)) {
const record = event.result as Record<string, unknown>;
if (record.ok === false || record.success === false || record.error) return false;
}
return true;
}

function appendToolGroupSegment(
segments: ActivitySegment[],
key: string,
Expand Down Expand Up @@ -2831,15 +2783,15 @@ function toolDetailRows(event: AgentToolProgressEvent, legacyLine: string | unde
appendToolDetailRow(rows, "Result", event.result);
appendToolDetailRow(rows, "Files", event.files);
appendToolDetailRow(rows, "Embeds", event.embeds);
appendToolDetailRow(rows, "Error", event.error, "error");
appendToolDetailRow(rows, "Error", event.error);
const trace = legacyLine?.trim();
if (trace && trace !== summaryLine) {
rows.push({ label: "Trace", value: truncateToolDetailValue(trace) });
}
return rows;
}

function appendToolDetailRow(rows: ActivityToolDetail[], label: string, value: unknown, tone?: ActivityToolDetail["tone"]): void {
function appendToolDetailRow(rows: ActivityToolDetail[], label: string, value: unknown): void {
if (value == null) {
return;
}
Expand All @@ -2853,7 +2805,7 @@ function appendToolDetailRow(rows: ActivityToolDetail[], label: string, value: u
if (!formatted.trim()) {
return;
}
rows.push({ label, value: formatted, tone });
rows.push({ label, value: formatted });
}

function toolEventFallbackLine(event: AgentToolProgressEvent): string {
Expand Down
38 changes: 24 additions & 14 deletions App/frontend/desktop/src/pages/tests/agent-thread-messages.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -668,52 +668,60 @@ describe("AgentThreadMessages", () => {
expect(html).not.toContain("浏览了 1 处");
});

it("de-emphasizes a tool validation error after the same tool succeeds on retry", () => {
it("keeps process tool errors in the detail card without an error treatment", () => {
const html = renderToString(
<I18nProvider language="zh-CN">
<AgentThreadMessages
chatScopeKey="chat-recovered-tool-error"
chatScopeKey="chat-process-tool-error"
messages={[{
id: "trace",
role: "tool",
kind: "trace",
content: "",
traces: [],
toolEvents: [
{ phase: "error", call_id: "call-invalid", name: "review_update_spec", error: "Invalid outputFormats" },
{ phase: "end", call_id: "call-valid", name: "review_update_spec", result: JSON.stringify({ ok: true }) }
],
toolEvents: [{ phase: "error", call_id: "call-invalid", name: "exec", error: "Error: command exited with 1 STDERR: path not found" }],
stoppedByUser: true
}]}
/>
</I18nProvider>
);

expect(html).toContain("Error: command exited with 1 STDERR: path not found");
expect(html).toContain('data-detail="error"');
expect(html).not.toContain("agent-activity-timeline-item--error");
expect(html).not.toContain("agent-activity-timeline-item__error");
expect(html).toContain("Invalid outputFormats");
expect(html).not.toContain("agent-activity-tool-card__section--error");
});

it("keeps an unrecovered tool error visibly red", () => {
it("keeps a failed file edit on one ordinary row", () => {
const html = renderToString(
<I18nProvider language="zh-CN">
<AgentThreadMessages
chatScopeKey="chat-unrecovered-tool-error"
chatScopeKey="chat-file-edit-error"
messages={[{
id: "trace",
id: "edit",
role: "tool",
kind: "trace",
content: "",
traces: [],
toolEvents: [{ phase: "error", call_id: "call-invalid", name: "review_update_spec", error: "Invalid outputFormats" }],
fileEdits: [{
call_id: "call-edit-error",
tool: "edit_file",
path: "BROKEN.md",
status: "error",
error: "permission denied while writing"
}],
stoppedByUser: true
}]}
/>
</I18nProvider>
);

expect(html).toContain("agent-activity-timeline-item--error");
expect(html).toContain("agent-activity-timeline-item__error");
expect(html).toContain("Failed");
expect(html).toContain("BROKEN.md");
expect(html).not.toContain("permission denied while writing");
expect(html).not.toContain("agent-activity-timeline-item--error");
expect(html).not.toContain("agent-activity-timeline-item__error");
});

it("folds the whole finished run — thoughts, tools, drafts — behind one worked-for header", () => {
Expand Down Expand Up @@ -1151,7 +1159,9 @@ describe("AgentThreadMessages", () => {
);

expect(html.replaceAll("&#x27;", "'")).toContain(WINDOWS_COMMAND_ERROR);
expect(html).toContain("agent-activity-timeline-item__error");
expect(html).toContain('data-detail="error"');
expect(html).not.toContain("agent-activity-timeline-item__error");
expect(html).not.toContain("agent-activity-tool-card__section--error");
expect(html).not.toContain("����");
});

Expand Down
30 changes: 4 additions & 26 deletions App/frontend/desktop/src/styles.css
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2487,10 +2487,6 @@ body.memmy-platform-windows:not(.memmy-window-fullscreen) .app-frame-main--windo
color: color-mix(in srgb, var(--color-text-ink) 58%, transparent);
}

.agent-activity-timeline-item--error .agent-activity-timeline-item__icon {
color: color-mix(in srgb, var(--color-status-error) 82%, transparent);
}

.agent-activity-timeline-item__body {
min-width: 0;
flex: 1 1 auto;
Expand Down Expand Up @@ -2531,19 +2527,6 @@ body.memmy-platform-windows:not(.memmy-window-fullscreen) .app-frame-main--windo
color: color-mix(in srgb, var(--color-text-ink) 62%, transparent);
}

.agent-activity-timeline-item--error .agent-activity-timeline-item__line,
.agent-activity-timeline-item--error .agent-activity-timeline-item__verb {
color: color-mix(in srgb, var(--color-status-error) 80%, var(--color-text-ink));
}

.agent-activity-timeline-item__error {
margin: 2px 0 0;
color: color-mix(in srgb, var(--color-status-error) 80%, var(--color-text-ink));
font-size: 12.5px;
line-height: 1.45;
overflow-wrap: anywhere;
}

.agent-activity-tool-details {
display: block;
padding: 0;
Expand Down Expand Up @@ -2576,11 +2559,10 @@ body.memmy-platform-windows:not(.memmy-window-fullscreen) .app-frame-main--windo
transform: rotate(90deg);
}

/* Expanded tool payload = ONE quiet code card (Cursor-style). The clickable
row above is already the human summary, so the card only carries raw
input/output: no ALL-CAPS form labels, sections separated by hairlines,
errors tinted, and long payloads scroll inside the card instead of
stretching the whole conversation. One generic shape fits every tool. */
/* Expanded tool payload = ONE quiet code card. The clickable row above is
already the human summary, so the card only carries raw input/output:
no form labels, sections separated by hairlines, and long payloads scroll
inside the card. Process failures use the same ink as other output. */
.agent-activity-tool-card {
margin: 4px 0 6px;
border: 1px solid color-mix(in srgb, var(--color-border-stone) 42%, transparent);
Expand Down Expand Up @@ -2616,10 +2598,6 @@ body.memmy-platform-windows:not(.memmy-window-fullscreen) .app-frame-main--windo
line-height: 1.55;
}

.agent-activity-tool-card__section--error .agent-activity-tool-card__value {
color: color-mix(in srgb, var(--color-status-error) 78%, var(--color-text-ink));
}

/* Reasoning body uses the shared markdown renderer but stays in the compact
13px muted style instead of the assistant answer's larger typography. */
.agent-message-content.agent-message-content--reasoning {
Expand Down
Loading