singularity/singularity-forge
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions 1

fix(tui): render assistant tool calls inline with text instead of grouped at end

Browse source 
Previously the chat-controller created one AssistantMessageComponent per
assistant message and removed/re-appended it to the chat container's tail
on every tool block, forcing all narration after every tool execution
regardless of stream order. Users had to scroll up to read text that was
written before each tool call.

Replace the reorder hack with a stream-order segment walker that walks
content[] left-to-right, collapses contiguous text/thinking blocks into
text-run segments, emits one segment per tool block, and append-only adds
new segments to chatContainer. AssistantMessageComponent gains a
ContentRange API so a single message can spawn multiple text-run
components, plus a separate showMetadata flag so timestamp/error footers
render only on the trailing segment without duplicating earlier text.

Adds a regression test that streams [text, tool, text, tool, text] and
asserts both interleaved order and per-segment rendered text content.

Closes #4144
This commit is contained in:
Jeremy 2026-04-13 17:23:17 -05:00
parent 1ec1a8c4c4
commit c8c416802f
3 changed files with 352 additions and 51 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

206
packages/pi-coding-agent/src/core/chat-controller-ordering.test.ts
View file

@ -94,7 +94,7 @@ function createHost() {
return host;
}
test("chat-controller keeps tool output ahead of delayed assistant text for external tool streams", async () => {
test("chat-controller renders content blocks in content[] index order (tool-first stream)", async () => {
// ToolExecutionComponent uses the global theme singleton.
// Install a minimal no-op theme implementation for this unit test.
(globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = {
@ -116,7 +116,6 @@ test("chat-controller keeps tool output ahead of delayed assistant text for exte
await handleAgentEvent(host, { type: "message_start", message: makeAssistant([]) } as any);
assert.equal(host.streamingComponent, undefined, "assistant component should be deferred at message_start");
assert.equal(host.chatContainer.children.length, 0, "nothing should render before content arrives");
await handleAgentEvent(
@ -140,11 +139,10 @@ test("chat-controller keeps tool output ahead of delayed assistant text for exte
} as any,
);
assert.equal(host.streamingComponent, undefined, "assistant text container should remain deferred for tool-only updates");
// content[0] = toolCall → ToolExecutionComponent renders first
assert.equal(host.chatContainer.children.length, 1, "tool execution block should render immediately");
assert.equal(host.chatContainer.children[0]?.constructor?.name, "ToolExecutionComponent");
// Re-assert required host method before the text-bearing update path.
host.getMarkdownThemeWithSettings = () => ({});
await handleAgentEvent(
@ -161,12 +159,13 @@ test("chat-controller keeps tool output ahead of delayed assistant text for exte
} as any,
);
assert.equal(host.chatContainer.children.length, 2, "assistant content should render after existing tool output");
// content[0]=toolCall, content[1]=text → order: tool, then text
assert.equal(host.chatContainer.children.length, 2, "text run should render after tool in content[] order");
assert.equal(host.chatContainer.children[0]?.constructor?.name, "ToolExecutionComponent");
assert.equal(host.chatContainer.children[1]?.constructor?.name, "AssistantMessageComponent");
});
test("chat-controller keeps serverToolUse output ahead of assistant text when external results arrive", async () => {
test("chat-controller renders serverToolUse before trailing text matching content[] index order", async () => {
(globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = {
fg: (_key: string, text: string) => text,
bg: (_key: string, text: string) => text,
@ -199,7 +198,7 @@ test("chat-controller keeps serverToolUse output ahead of assistant text when ex
} as any,
);
assert.equal(host.streamingComponent, undefined, "assistant content should stay deferred while only tool content streams");
// content[0] = serverToolUse → ToolExecutionComponent renders first
assert.equal(host.chatContainer.children.length, 1, "server tool block should render immediately");
assert.equal(host.chatContainer.children[0]?.constructor?.name, "ToolExecutionComponent");
@ -229,7 +228,8 @@ test("chat-controller keeps serverToolUse output ahead of assistant text when ex
} as any,
);
assert.equal(host.chatContainer.children.length, 2, "assistant text should render after existing server tool output");
// content[0]=serverToolUse, content[1]=text → order: tool, then text
assert.equal(host.chatContainer.children.length, 2, "text run should render after server tool in content[] order");
assert.equal(host.chatContainer.children[0]?.constructor?.name, "ToolExecutionComponent");
assert.equal(host.chatContainer.children[1]?.constructor?.name, "AssistantMessageComponent");
});
@ -466,3 +466,193 @@ test("chat-controller does not pin when there are no tool calls", async () => {
assert.equal(host.pinnedMessageContainer.children.length, 0, "pinned zone should stay empty without tool calls");
});
// Regression test for issue #4144: interleaved text/tool content must render in content[] index order.
// Stream: [text "A", toolCall T1, text "B", toolCall T2, text "C"]
// Expected chatContainer order: textRun(A), toolExec(T1), textRun(B), toolExec(T2), textRun(C)
// Each AssistantMessageComponent must render ONLY its own text — no duplication after message_end.
test("chat-controller renders interleaved text and tool blocks in content[] index order (#4144)", async () => {
(globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = {
fg: (_key: string, text: string) => text,
bg: (_key: string, text: string) => text,
bold: (text: string) => text,
italic: (text: string) => text,
truncate: (text: string) => text,
};
const host = createHost();
host.getMarkdownThemeWithSettings = () => ({});
const t1 = { type: "toolCall", id: "t1", name: "tool_one", arguments: {} };
const t2 = { type: "toolCall", id: "t2", name: "tool_two", arguments: {} };
await handleAgentEvent(host, { type: "message_start", message: makeAssistant([]) } as any);
// Stream text "A" at index 0
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }]),
assistantMessageEvent: {
type: "text_delta",
contentIndex: 0,
delta: "A",
partial: makeAssistant([{ type: "text", text: "A" }]),
},
} as any);
// Stream toolCall T1 at index 1
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }, t1]),
assistantMessageEvent: {
type: "toolcall_end",
contentIndex: 1,
toolCall: {
...t1,
externalResult: { content: [{ type: "text", text: "result1" }], details: {}, isError: false },
},
partial: makeAssistant([{ type: "text", text: "A" }, t1]),
},
} as any);
// Stream text "B" at index 2
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }]),
assistantMessageEvent: {
type: "text_delta",
contentIndex: 2,
delta: "B",
partial: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }]),
},
} as any);
// Stream toolCall T2 at index 3
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }, t2]),
assistantMessageEvent: {
type: "toolcall_end",
contentIndex: 3,
toolCall: {
...t2,
externalResult: { content: [{ type: "text", text: "result2" }], details: {}, isError: false },
},
partial: makeAssistant([{ type: "text", text: "A" }, t1, { type: "text", text: "B" }, t2]),
},
} as any);
// Stream text "C" at index 4
const finalContent = [
{ type: "text", text: "A" }, t1, { type: "text", text: "B" }, t2, { type: "text", text: "C" },
];
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant(finalContent),
assistantMessageEvent: {
type: "text_delta",
contentIndex: 4,
delta: "C",
partial: makeAssistant(finalContent),
},
} as any);
// Finalize — exercises the message_end path where a buggy setRange(undefined) would cause duplication
await handleAgentEvent(host, { type: "message_end", message: makeAssistant(finalContent) } as any);
// Assert interleaved order: textRun(A), toolExec(T1), textRun(B), toolExec(T2), textRun(C)
assert.equal(host.chatContainer.children.length, 5, "should have 5 children in interleaved order");
assert.equal(host.chatContainer.children[0]?.constructor?.name, "AssistantMessageComponent", "index 0: text run A");
assert.equal(host.chatContainer.children[1]?.constructor?.name, "ToolExecutionComponent", "index 1: tool T1");
assert.equal(host.chatContainer.children[2]?.constructor?.name, "AssistantMessageComponent", "index 2: text run B");
assert.equal(host.chatContainer.children[3]?.constructor?.name, "ToolExecutionComponent", "index 3: tool T2");
assert.equal(host.chatContainer.children[4]?.constructor?.name, "AssistantMessageComponent", "index 4: text run C");
// Helper: collect the text of all Markdown children inside an AssistantMessageComponent.
// Structure: AssistantMessageComponent (Container) -> contentContainer (children[0]) -> Markdown nodes.
function getRenderedTexts(comp: any): string[] {
const contentContainer = comp.children?.[0];
if (!contentContainer) return [];
return (contentContainer.children ?? [])
.filter((c: any) => c.constructor?.name === "Markdown")
.map((c: any) => (c as any).text as string);
}
// Each text-run component must contain only its own text — no cross-contamination after message_end
assert.deepEqual(getRenderedTexts(host.chatContainer.children[0]), ["A"], "text run A must contain only 'A'");
assert.deepEqual(getRenderedTexts(host.chatContainer.children[2]), ["B"], "text run B must contain only 'B'");
assert.deepEqual(getRenderedTexts(host.chatContainer.children[4]), ["C"], "text run C must contain only 'C'");
});
test("chat-controller does not duplicate text when content is [text, tool, text] (interleaved stream)", async () => {
(globalThis as any)[Symbol.for("@gsd/pi-coding-agent:theme")] = {
fg: (_key: string, text: string) => text,
bg: (_key: string, text: string) => text,
bold: (text: string) => text,
italic: (text: string) => text,
truncate: (text: string) => text,
};
const host = createHost();
host.getMarkdownThemeWithSettings = () => ({});
const t1 = { type: "toolCall", id: "t1", name: "tool_one", arguments: {} };
await handleAgentEvent(host, { type: "message_start", message: makeAssistant([]) } as any);
// Step 1: text "A" at index 0
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }]),
assistantMessageEvent: {
type: "text_delta",
contentIndex: 0,
delta: "A",
partial: makeAssistant([{ type: "text", text: "A" }]),
},
} as any);
// Step 2: toolCall at index 1
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant([{ type: "text", text: "A" }, t1]),
assistantMessageEvent: {
type: "toolcall_end",
contentIndex: 1,
toolCall: {
...t1,
externalResult: { content: [{ type: "text", text: "result1" }], details: {}, isError: false },
},
partial: makeAssistant([{ type: "text", text: "A" }, t1]),
},
} as any);
// Step 3: text "B" at index 2
const finalContent = [{ type: "text", text: "A" }, t1, { type: "text", text: "B" }];
await handleAgentEvent(host, {
type: "message_update",
message: makeAssistant(finalContent),
assistantMessageEvent: {
type: "text_delta",
contentIndex: 2,
delta: "B",
partial: makeAssistant(finalContent),
},
} as any);
assert.equal(host.chatContainer.children.length, 3);
assert.equal(host.chatContainer.children[0]?.constructor?.name, "AssistantMessageComponent");
assert.equal(host.chatContainer.children[1]?.constructor?.name, "ToolExecutionComponent");
assert.equal(host.chatContainer.children[2]?.constructor?.name, "AssistantMessageComponent");
const firstText = host.chatContainer.children[0];
const secondText = host.chatContainer.children[2];
assert.notEqual(firstText, secondText, "text-before-tool and text-after-tool must be separate component instances");
assert.deepEqual((firstText as any).range, { startIndex: 0, endIndex: 0 }, "first text-run covers only content[0]");
assert.deepEqual((secondText as any).range, { startIndex: 2, endIndex: 2 }, "second text-run covers only content[2]");
// Finalize — regression guard: range must NOT be cleared on message_end (would cause duplication)
await handleAgentEvent(host, { type: "message_end", message: makeAssistant(finalContent) } as any);
assert.deepEqual((secondText as any).range, { startIndex: 2, endIndex: 2 }, "range must not be cleared on message_end (would cause duplication)");
});

88
packages/pi-coding-agent/src/modes/interactive/components/assistant-message.ts
View file

@ -3,8 +3,15 @@ import { Container, Markdown, type MarkdownTheme, Spacer, Text } from "@gsd/pi-t
import { getMarkdownTheme, theme } from "../theme/theme.js";
import { formatTimestamp, type TimestampFormat } from "./timestamp.js";
export interface ContentRange {
startIndex: number;
endIndex: number;
}
/**
* Component that renders a complete assistant message
* Component that renders a complete assistant message, or a sub-range of its content[].
* When `range` is provided, only content[startIndex..endIndex] (inclusive) is rendered.
* Non-text/thinking blocks within the range are silently skipped.
*/
export class AssistantMessageComponent extends Container {
private contentContainer: Container;
@ -12,18 +19,26 @@ export class AssistantMessageComponent extends Container {
private markdownTheme: MarkdownTheme;
private lastMessage?: AssistantMessage;
private timestampFormat: TimestampFormat;
private range?: ContentRange;
private showMetadata: boolean;
constructor(
message?: AssistantMessage,
hideThinkingBlock = false,
markdownTheme: MarkdownTheme = getMarkdownTheme(),
timestampFormat: TimestampFormat = "date-time-iso",
range?: ContentRange,
) {
super();
this.hideThinkingBlock = hideThinkingBlock;
this.markdownTheme = markdownTheme;
this.timestampFormat = timestampFormat;
this.range = range;
// No range = legacy full-message rendering; show metadata by default.
// Ranged (interleaved) instances start with metadata hidden; chat-controller
// calls setShowMetadata(true) on the last segment at message_end.
this.showMetadata = !range;
// Container for text/thinking content
this.contentContainer = new Container();
@ -34,6 +49,20 @@ export class AssistantMessageComponent extends Container {
}
}
setRange(range: ContentRange | undefined): void {
this.range = range;
if (this.lastMessage) {
this.updateContent(this.lastMessage);
}
}
setShowMetadata(show: boolean): void {
this.showMetadata = show;
if (this.lastMessage) {
this.updateContent(this.lastMessage);
}
}
override invalidate(): void {
super.invalidate();
if (this.lastMessage) {
@ -51,7 +80,11 @@ export class AssistantMessageComponent extends Container {
// Clear content container
this.contentContainer.clear();
const hasVisibleContent = message.content.some(
const start = this.range?.startIndex ?? 0;
const end = this.range?.endIndex ?? message.content.length - 1;
const slice = message.content.slice(start, end + 1);
const hasVisibleContent = slice.some(
(c) => (c.type === "text" && c.text.trim()) || (c.type === "thinking" && c.thinking.trim()),
);
@ -59,9 +92,9 @@ export class AssistantMessageComponent extends Container {
this.contentContainer.addChild(new Spacer(1));
}
// Render content in order
for (let i = 0; i < message.content.length; i++) {
const content = message.content[i];
// Render content in order; non-text/thinking blocks are silently skipped
for (let i = 0; i < slice.length; i++) {
const content = slice[i];
if (content.type === "text" && content.text.trim()) {
// Assistant text messages with no background - trim the text
// Set paddingY=0 to avoid extra spacing before tool executions
@ -69,7 +102,7 @@ export class AssistantMessageComponent extends Container {
} else if (content.type === "thinking" && content.thinking.trim()) {
// Add spacing only when another visible assistant content block follows.
// This avoids a superfluous blank line before separately-rendered tool execution blocks.
const hasVisibleContentAfter = message.content
const hasVisibleContentAfter = slice
.slice(i + 1)
.some((c) => (c.type === "text" && c.text.trim()) || (c.type === "thinking" && c.thinking.trim()));
@ -94,30 +127,33 @@ export class AssistantMessageComponent extends Container {
}
}
// Check if aborted - show after partial content
// But only if there are no tool calls (tool execution components will show the error)
const hasToolCalls = message.content.some((c) => c.type === "toolCall");
if (!hasToolCalls) {
if (message.stopReason === "aborted") {
const abortMessage =
message.errorMessage && message.errorMessage !== "Request was aborted"
? message.errorMessage
: "Operation aborted";
if (hasVisibleContent) {
// Metadata (errors, timestamp): gated on showMetadata so ranged instances stay clean
// until chat-controller explicitly enables it on the last segment at message_end.
if (this.showMetadata) {
// Check if aborted - show after partial content
// But only if there are no tool calls (tool execution components will show the error)
const hasToolCalls = message.content.some((c) => c.type === "toolCall");
if (!hasToolCalls) {
if (message.stopReason === "aborted") {
const abortMessage =
message.errorMessage && message.errorMessage !== "Request was aborted"
? message.errorMessage
: "Operation aborted";
if (hasVisibleContent) {
this.contentContainer.addChild(new Spacer(1));
}
this.contentContainer.addChild(new Text(theme.fg("error", abortMessage), 1, 0));
} else if (message.stopReason === "error") {
const errorMsg = message.errorMessage || "Unknown error";
this.contentContainer.addChild(new Spacer(1));
this.contentContainer.addChild(new Text(theme.fg("error", `Error: ${errorMsg}`), 1, 0));
}
this.contentContainer.addChild(new Text(theme.fg("error", abortMessage), 1, 0));
} else if (message.stopReason === "error") {
const errorMsg = message.errorMessage || "Unknown error";
this.contentContainer.addChild(new Spacer(1));
this.contentContainer.addChild(new Text(theme.fg("error", `Error: ${errorMsg}`), 1, 0));
}
}
// Show timestamp when the message is complete (has a stop reason)
if (message.stopReason && message.timestamp) {
const timeStr = formatTimestamp(message.timestamp, this.timestampFormat);
this.contentContainer.addChild(new Text(theme.fg("dim", timeStr), 1, 0));
if (message.stopReason && message.timestamp) {
const timeStr = formatTimestamp(message.timestamp, this.timestampFormat);
this.contentContainer.addChild(new Text(theme.fg("dim", timeStr), 1, 0));
}
}
}
}

109
packages/pi-coding-agent/src/modes/interactive/controllers/chat-controller.ts
View file

@ -10,6 +10,13 @@ import { appKey } from "../components/keybinding-hints.js";
// Tracks the last processed content index to avoid re-scanning all blocks on every message_update
let lastProcessedContentIndex = 0;
// --- Segment walker state (per streaming assistant turn) ---
type RenderedSegment =
| { kind: "text-run"; startIndex: number; endIndex: number; component: AssistantMessageComponent }
| { kind: "tool"; contentIndex: number; component: ToolExecutionComponent };
let renderedSegments: RenderedSegment[] = [];
function hasVisibleAssistantContent(message: { content: Array<any> }): boolean {
return message.content.some(
(c) =>
@ -80,6 +87,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & {
lastProcessedContentIndex = 0;
lastPinnedText = "";
hasToolsInTurn = false;
renderedSegments = [];
if (pinnedBorder) pinnedBorder.stopSpinner();
pinnedBorder = undefined;
pinnedTextComponent = undefined;
@ -99,6 +107,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & {
host.pinnedMessageContainer.clear();
lastPinnedText = "";
hasToolsInTurn = false;
renderedSegments = [];
if (pinnedBorder) pinnedBorder.stopSpinner();
pinnedBorder = undefined;
pinnedTextComponent = undefined;
@ -273,24 +282,88 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & {
}
}
// Render assistant text/thinking after tool components so mixed
// streams keep chronological ordering in the chat container.
const hasToolBlocks = hasAssistantToolBlocks(host.streamingMessage);
if (!host.streamingComponent && hasVisibleAssistantContent(host.streamingMessage)) {
host.streamingComponent = new AssistantMessageComponent(
undefined,
host.hideThinkingBlock,
host.getMarkdownThemeWithSettings(),
host.settingsManager.getTimestampFormat(),
);
host.chatContainer.addChild(host.streamingComponent);
}
if (host.streamingComponent) {
if (hasToolBlocks) {
host.chatContainer.removeChild(host.streamingComponent);
host.chatContainer.addChild(host.streamingComponent);
// Segment walker: render content blocks in stream order, append-only.
// Build desired segment plan from content[].
{
const blocks = host.streamingMessage.content;
type DesiredSegment =
| { kind: "text-run"; startIndex: number; endIndex: number }
| { kind: "tool"; contentIndex: number; toolId: string };
const desired: DesiredSegment[] = [];
let runStart = -1;
for (let i = 0; i < blocks.length; i++) {
const b = blocks[i];
const isText = b.type === "text" || b.type === "thinking";
const isTool = b.type === "toolCall" || b.type === "serverToolUse";
if (isText) {
if (runStart === -1) runStart = i;
} else {
if (runStart !== -1) {
desired.push({ kind: "text-run", startIndex: runStart, endIndex: i - 1 });
runStart = -1;
}
if (isTool) {
desired.push({ kind: "tool", contentIndex: i, toolId: b.id });
}
}
}
if (runStart !== -1) {
desired.push({ kind: "text-run", startIndex: runStart, endIndex: blocks.length - 1 });
}
// Append any newly needed segments (never reorder existing ones).
for (const seg of desired) {
if (seg.kind === "tool") {
// Tool segments are already handled above via pendingTools; just
// register them in renderedSegments if not yet tracked.
const existing = renderedSegments.find(
(s) => s.kind === "tool" && s.contentIndex === seg.contentIndex,
);
if (!existing) {
const comp = host.pendingTools.get(seg.toolId);
if (comp) {
renderedSegments.push({ kind: "tool", contentIndex: seg.contentIndex, component: comp });
}
}
} else {
// text-run segment
const existing = renderedSegments.find(
(s) => s.kind === "text-run" && s.startIndex === seg.startIndex,
);
if (!existing) {
const comp = new AssistantMessageComponent(
undefined,
host.hideThinkingBlock,
host.getMarkdownThemeWithSettings(),
host.settingsManager.getTimestampFormat(),
{ startIndex: seg.startIndex, endIndex: seg.endIndex },
);
host.chatContainer.addChild(comp);
renderedSegments.push({ kind: "text-run", startIndex: seg.startIndex, endIndex: seg.endIndex, component: comp });
host.streamingComponent = comp;
}
}
}
// Update all trailing text-run segments with the latest message so
// streaming text grows in place.
for (const seg of renderedSegments) {
if (seg.kind === "text-run") {
// Find corresponding desired segment to get current endIndex
const d = desired.find((ds) => ds.kind === "text-run" && ds.startIndex === seg.startIndex);
if (d && d.kind === "text-run" && d.endIndex !== seg.endIndex) {
seg.endIndex = d.endIndex;
seg.component.setRange({ startIndex: seg.startIndex, endIndex: seg.endIndex });
}
seg.component.updateContent(host.streamingMessage);
}
}
// Keep streamingComponent pointing at the last text-run for message_end compatibility.
const lastTextSeg = [...renderedSegments].reverse().find((s) => s.kind === "text-run");
if (lastTextSeg && lastTextSeg.kind === "text-run") {
host.streamingComponent = lastTextSeg.component;
}
host.streamingComponent.updateContent(host.streamingMessage);
}
// Update index: fully processed blocks won't need re-scanning.
@ -376,6 +449,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & {
host.chatContainer.addChild(host.streamingComponent);
}
if (host.streamingComponent) {
host.streamingComponent.setShowMetadata(true);
host.streamingComponent.updateContent(host.streamingMessage);
}
@ -399,6 +473,7 @@ export async function handleAgentEvent(host: InteractiveModeStateHost & {
}
host.streamingComponent = undefined;
host.streamingMessage = undefined;
renderedSegments = [];
// Clear pinned output once the message is finalized in the chat
// container — prevents duplicate display when the agent continues
// (e.g. form elicitation) after the assistant message ends.