Skip to content

Commit e61547d

Browse files
committed
fix(message): stop showing red '! terminated' for benign stop reasons
The model finishes a response, upstream emits a non-standard finish_reason like "terminated" / "end_turn" / "stop_sequence", pi-ai surfaces it as errorMessage, and our renderer slapped a red "! terminated" under a perfectly good message — making every successful response look like a failure. Classify the errorMessage before deciding how to render. Benign stop signals on a message with text content collapse to nothing. Empty responses still get a "(empty response)" warning. Length / max_tokens become a yellow "(response truncated)" so the user knows why their message ended mid-sentence. Anything else still renders as a real error.
1 parent c12f3a3 commit e61547d

3 files changed

Lines changed: 146 additions & 3 deletions

File tree

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "codebase-cli",
3-
"version": "2.0.0-pre.43",
3+
"version": "2.0.0-pre.44",
44
"description": "Codebase CLI — a TypeScript coding agent on the pi-mono runtime. OAuth-aware, any LLM provider, single install.",
55
"keywords": [
66
"ai",

src/ui/Message-error.test.ts

Lines changed: 74 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,74 @@
1+
import type { AgentMessage } from "@earendil-works/pi-agent-core";
2+
import { describe, expect, it } from "vitest";
3+
4+
// errorMessageNote is unexported by Message.tsx but we don't want to widen
5+
// Message.tsx's public surface just for tests. Instead, exercise the same
6+
// classifier via the testable side: import-and-call. We re-export it from
7+
// a sibling test-helper-friendly file when this becomes load-bearing —
8+
// for now, replicate the cases the rendering relies on by importing the
9+
// classifier directly. The function is internal but accessible via the
10+
// CJS interop: import all named exports through the module path.
11+
import { errorMessageNote } from "./Message.js";
12+
13+
function assistant(text: string, errorMessage?: string): AgentMessage & { role: "assistant" } {
14+
return {
15+
role: "assistant",
16+
content: text ? [{ type: "text", text }] : [],
17+
api: "chat",
18+
provider: "p",
19+
model: "m",
20+
usage: {
21+
input: 0,
22+
output: 0,
23+
cacheRead: 0,
24+
cacheWrite: 0,
25+
totalTokens: 0,
26+
cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 },
27+
},
28+
stopReason: "stop",
29+
timestamp: 1,
30+
errorMessage,
31+
} as unknown as AgentMessage & { role: "assistant" };
32+
}
33+
34+
describe("errorMessageNote", () => {
35+
it("returns null when there is no errorMessage", () => {
36+
expect(errorMessageNote(assistant("hi"))).toBeNull();
37+
});
38+
39+
it("hides quirky stop reasons when the message has substantive content", () => {
40+
expect(errorMessageNote(assistant("a long response body", "terminated"))).toBeNull();
41+
expect(errorMessageNote(assistant("content", "end_turn"))).toBeNull();
42+
expect(errorMessageNote(assistant("content", "stop_sequence"))).toBeNull();
43+
expect(errorMessageNote(assistant("content", "Provider finish_reason: terminated"))).toBeNull();
44+
});
45+
46+
it("surfaces a benign-stop errorMessage as 'empty response' when the message has no text", () => {
47+
const note = errorMessageNote(assistant("", "terminated"));
48+
expect(note?.severity).toBe("warning");
49+
expect(note?.text).toMatch(/empty response/i);
50+
});
51+
52+
it("flags length / max_tokens as a truncation warning regardless of content", () => {
53+
const note = errorMessageNote(assistant("partial body...", "length"));
54+
expect(note?.severity).toBe("warning");
55+
expect(note?.text).toMatch(/truncated/i);
56+
const note2 = errorMessageNote(assistant("partial", "Provider finish_reason: max_tokens"));
57+
expect(note2?.severity).toBe("warning");
58+
});
59+
60+
it("renders unknown / unrecognized errorMessages as real errors", () => {
61+
const note = errorMessageNote(assistant("text", "Network failure: connect ECONNREFUSED"));
62+
expect(note?.severity).toBe("error");
63+
expect(note?.text).toMatch(/^error:/);
64+
});
65+
66+
it("trims surrounding whitespace before classifying", () => {
67+
expect(errorMessageNote(assistant("text", " terminated "))).toBeNull();
68+
});
69+
70+
it("treats whitespace-only content as empty for the no-content branch", () => {
71+
const note = errorMessageNote(assistant(" ", "terminated"));
72+
expect(note?.severity).toBe("warning");
73+
});
74+
});

src/ui/Message.tsx

Lines changed: 71 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -88,11 +88,19 @@ function MessageBody({
8888

8989
if (message.role === "assistant") {
9090
const rendered = renderAssistantBlocks(message.content, width, tools);
91+
const note = errorMessageNote(message);
9192
return (
9293
<>
9394
{rendered}
94-
{message.errorMessage ? (
95-
<WrappedLines text={`! ${message.errorMessage}`} width={width} keyPrefix="err" color="red" />
95+
{note ? (
96+
<WrappedLines
97+
text={note.text}
98+
width={width}
99+
keyPrefix="err"
100+
color={note.severity === "error" ? "red" : undefined}
101+
dimColor={note.severity === "info"}
102+
italic={note.severity === "info"}
103+
/>
96104
) : null}
97105
</>
98106
);
@@ -228,6 +236,67 @@ function UserBlocks({ blocks, width }: { blocks: unknown; width: number }) {
228236
return <>{rows}</>;
229237
}
230238

239+
/**
240+
* Stop-reason strings that are upstream-provider bookkeeping ("the model
241+
* said it's done"), not real errors. Different providers use different
242+
* wording for the same "I finished" signal; the pi-ai layer surfaces them
243+
* uniformly as errorMessage which makes everything look red. Treat these
244+
* as quiet end-of-turn markers instead.
245+
*/
246+
const QUIET_END_PATTERNS = [
247+
/^terminated$/i,
248+
/^end[_ ]turn$/i,
249+
/^stop$/i,
250+
/^stop[_ ]sequence$/i,
251+
/^Provider finish_reason: (?:terminated|end_turn|stop|stop_sequence)$/i,
252+
];
253+
254+
/**
255+
* Stop-reason strings that mean "the response was truncated against the
256+
* user's intent" — content was cut off by a length limit. The user should
257+
* see this so they understand why the message ends abruptly.
258+
*/
259+
const TRUNCATION_PATTERNS = [/^length$/i, /^max[_ ]tokens$/i, /^Provider finish_reason: (?:length|max_tokens)$/i];
260+
261+
interface ErrorMessageNote {
262+
text: string;
263+
severity: "info" | "warning" | "error";
264+
}
265+
266+
/**
267+
* Decide how to render the assistant message's errorMessage tail. A
268+
* model that finished with substantive text content but a quirky
269+
* non-standard stop reason shouldn't get a red "!" — that reads as a
270+
* failure. Real failures (empty response, content filter, network) do
271+
* get the loud treatment. Truncation gets a middle ground.
272+
*/
273+
export function errorMessageNote(message: AgentMessage & { role: "assistant" }): ErrorMessageNote | null {
274+
const raw = message.errorMessage;
275+
if (!raw) return null;
276+
const trimmed = raw.trim();
277+
const hasContent = messageHasVisibleText(message);
278+
279+
if (TRUNCATION_PATTERNS.some((re) => re.test(trimmed))) {
280+
return { text: "(response truncated — model hit its output length limit)", severity: "warning" };
281+
}
282+
if (QUIET_END_PATTERNS.some((re) => re.test(trimmed))) {
283+
// Benign stop signal with content — drop entirely. Without content
284+
// it's an empty response, which IS noteworthy.
285+
return hasContent ? null : { text: "(empty response)", severity: "warning" };
286+
}
287+
// Anything else: real error. Loud and explicit.
288+
return { text: `error: ${trimmed}`, severity: "error" };
289+
}
290+
291+
function messageHasVisibleText(message: AgentMessage): boolean {
292+
if (typeof message.content === "string") return message.content.trim().length > 0;
293+
if (!Array.isArray(message.content)) return false;
294+
for (const block of message.content) {
295+
if (block.type === "text" && typeof block.text === "string" && block.text.trim().length > 0) return true;
296+
}
297+
return false;
298+
}
299+
231300
function formatBytes(n: number): string {
232301
if (n < 1024) return `${n} B`;
233302
if (n < 1024 * 1024) return `${(n / 1024).toFixed(1)} KB`;

0 commit comments

Comments
 (0)