From 9bc43b61bf60e46dd52be7f5c73a0443ec4f6975 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sun, 5 Apr 2026 20:53:17 +0100 Subject: [PATCH] refactor: share assistant phase helpers --- src/agents/openai-ws-message-conversion.ts | 45 ++--------------- src/agents/openai-ws-stream.ts | 19 ++----- ...pi-embedded-subscribe.handlers.messages.ts | 46 ++--------------- src/agents/pi-embedded-utils.ts | 36 ++------------ src/gateway/server-methods/chat.ts | 31 +----------- .../server.chat.gateway-server-chat.test.ts | 23 +++++++++ src/shared/chat-message-content.test.ts | 49 ++++++++++++++++++- src/shared/chat-message-content.ts | 46 +++++++++++++++-- 8 files changed, 133 insertions(+), 162 deletions(-) diff --git a/src/agents/openai-ws-message-conversion.ts b/src/agents/openai-ws-message-conversion.ts index 48586ce5122..a3db979c8af 100644 --- a/src/agents/openai-ws-message-conversion.ts +++ b/src/agents/openai-ws-message-conversion.ts @@ -1,6 +1,11 @@ import { randomUUID } from "node:crypto"; import type { Context, Message, StopReason } from "@mariozechner/pi-ai"; import type { AssistantMessage } from "@mariozechner/pi-ai"; +import { + encodeAssistantTextSignature, + normalizeAssistantPhase, + parseAssistantTextSignature, +} from "../shared/chat-message-content.js"; import { normalizeOpenAIStrictToolParameters, resolveOpenAIStrictToolFlagForInventory, @@ -38,46 +43,6 @@ function toNonEmptyString(value: unknown): string | null { return trimmed.length > 0 ? trimmed : null; } -function normalizeAssistantPhase(value: unknown): OpenAIResponsesAssistantPhase | undefined { - return value === "commentary" || value === "final_answer" ? value : undefined; -} - -function encodeAssistantTextSignature(params: { - id: string; - phase?: OpenAIResponsesAssistantPhase; -}): string { - return JSON.stringify({ - v: 1, - id: params.id, - ...(params.phase ? { phase: params.phase } : {}), - }); -} - -function parseAssistantTextSignature( - value: unknown, -): { id: string; phase?: OpenAIResponsesAssistantPhase } | null { - if (typeof value !== "string" || value.trim().length === 0) { - return null; - } - if (!value.startsWith("{")) { - return { id: value }; - } - try { - const parsed = JSON.parse(value) as { v?: unknown; id?: unknown; phase?: unknown }; - if (parsed.v !== 1 || typeof parsed.id !== "string") { - return null; - } - return { - id: parsed.id, - ...(normalizeAssistantPhase(parsed.phase) - ? { phase: normalizeAssistantPhase(parsed.phase) } - : {}), - }; - } catch { - return null; - } -} - function supportsImageInput(modelOverride?: ReplayModelInfo): boolean { return !Array.isArray(modelOverride?.input) || modelOverride.input.includes("image"); } diff --git a/src/agents/openai-ws-stream.ts b/src/agents/openai-ws-stream.ts index 4e453fdeef1..94e0afc1529 100644 --- a/src/agents/openai-ws-stream.ts +++ b/src/agents/openai-ws-stream.ts @@ -35,6 +35,10 @@ import { resolveProviderWebSocketSessionPolicyWithPlugin, } from "../plugins/provider-runtime.js"; import type { ProviderRuntimeModel, ProviderTransportTurnState } from "../plugins/types.js"; +import { + encodeAssistantTextSignature, + normalizeAssistantPhase, +} from "../shared/chat-message-content.js"; import { resolveOpenAIStrictToolSetting } from "./openai-tool-schema.js"; import { getOpenAIWebSocketErrorDetails, @@ -102,21 +106,6 @@ type OpenAIWsStreamDeps = { type AssistantMessageWithPhase = AssistantMessage & { phase?: OpenAIResponsesAssistantPhase }; -function normalizeAssistantPhase(value: unknown): OpenAIResponsesAssistantPhase | undefined { - return value === "commentary" || value === "final_answer" ? value : undefined; -} - -function encodeAssistantTextSignature(params: { - id: string; - phase?: OpenAIResponsesAssistantPhase; -}): string { - return JSON.stringify({ - v: 1, - id: params.id, - ...(params.phase ? { phase: params.phase } : {}), - }); -} - const defaultOpenAIWsStreamDeps: OpenAIWsStreamDeps = { createManager: (options) => new OpenAIWebSocketManager(options), createHttpFallbackStreamFn: (model) => createBoundaryAwareStreamFnForModel(model), diff --git a/src/agents/pi-embedded-subscribe.handlers.messages.ts b/src/agents/pi-embedded-subscribe.handlers.messages.ts index dc9dd02c972..ba2f56369bc 100644 --- a/src/agents/pi-embedded-subscribe.handlers.messages.ts +++ b/src/agents/pi-embedded-subscribe.handlers.messages.ts @@ -5,6 +5,7 @@ import { parseReplyDirectives } from "../auto-reply/reply/reply-directives.js"; import { isSilentReplyText, SILENT_REPLY_TOKEN } from "../auto-reply/tokens.js"; import { emitAgentEvent } from "../infra/agent-events.js"; import { createInlineCodeState } from "../markdown/code-spans.js"; +import { resolveAssistantMessagePhase } from "../shared/chat-message-content.js"; import { isMessagingToolDuplicateNormalized, normalizeTextForComparison, @@ -26,8 +27,6 @@ import { promoteThinkingTagsToBlocks, } from "./pi-embedded-utils.js"; -type AssistantDeliveryPhase = "commentary" | "final_answer"; - const stripTrailingDirective = (text: string): string => { const openIndex = text.lastIndexOf("[["); if (openIndex < 0) { @@ -68,47 +67,8 @@ const coerceText = (value: unknown): string => { return ""; }; -function normalizeAssistantDeliveryPhase(value: unknown): AssistantDeliveryPhase | undefined { - return value === "commentary" || value === "final_answer" ? value : undefined; -} - -function resolveAssistantDeliveryPhase( - message: AgentMessage | undefined, -): AssistantDeliveryPhase | undefined { - if (!message || message.role !== "assistant") { - return undefined; - } - const directPhase = normalizeAssistantDeliveryPhase((message as { phase?: unknown }).phase); - if (directPhase) { - return directPhase; - } - if (!Array.isArray(message.content)) { - return undefined; - } - const explicitStructuredPhases = new Set(); - for (const part of message.content) { - if (!part || typeof part !== "object") { - continue; - } - const block = part as { type?: unknown; textSignature?: unknown }; - if (block.type !== "text" || typeof block.textSignature !== "string") { - continue; - } - try { - const parsed = JSON.parse(block.textSignature) as { phase?: unknown }; - const phase = normalizeAssistantDeliveryPhase(parsed.phase); - if (phase) { - explicitStructuredPhases.add(phase); - } - } catch { - continue; - } - } - return explicitStructuredPhases.size === 1 ? [...explicitStructuredPhases][0] : undefined; -} - function shouldSuppressAssistantVisibleOutput(message: AgentMessage | undefined): boolean { - return resolveAssistantDeliveryPhase(message) === "commentary"; + return resolveAssistantMessagePhase(message) === "commentary"; } function isTranscriptOnlyOpenClawAssistantMessage(message: AgentMessage | undefined): boolean { @@ -330,7 +290,7 @@ export function handleMessageUpdate( ? (assistantRecord.partial as AssistantMessage) : msg; const phaseAwareVisibleText = coerceText(extractAssistantVisibleText(partialAssistant)).trim(); - const deliveryPhase = resolveAssistantDeliveryPhase(partialAssistant); + const deliveryPhase = resolveAssistantMessagePhase(partialAssistant); if (deliveryPhase === "commentary" && !phaseAwareVisibleText) { return; } diff --git a/src/agents/pi-embedded-utils.ts b/src/agents/pi-embedded-utils.ts index c4d016b5a4f..ea936b751aa 100644 --- a/src/agents/pi-embedded-utils.ts +++ b/src/agents/pi-embedded-utils.ts @@ -1,6 +1,11 @@ import type { AgentMessage } from "@mariozechner/pi-agent-core"; import type { AssistantMessage } from "@mariozechner/pi-ai"; import { extractTextFromChatContent } from "../shared/chat-content.js"; +import { + normalizeAssistantPhase, + parseAssistantTextSignature, + type AssistantPhase, +} from "../shared/chat-message-content.js"; import { stripReasoningTagsFromText } from "../shared/text/reasoning-tags.js"; import { sanitizeUserFacingText } from "./pi-embedded-helpers.js"; import { formatToolDetail, resolveToolDisplay } from "./tool-display.js"; @@ -233,37 +238,6 @@ export function stripThinkingTagsFromText(text: string): string { return stripReasoningTagsFromText(text, { mode: "strict", trim: "both" }); } -type AssistantPhase = "commentary" | "final_answer"; - -function normalizeAssistantPhase(value: unknown): AssistantPhase | undefined { - return value === "commentary" || value === "final_answer" ? value : undefined; -} - -function parseAssistantTextSignature( - value: unknown, -): { id?: string; phase?: AssistantPhase } | null { - if (typeof value !== "string" || value.trim().length === 0) { - return null; - } - if (!value.startsWith("{")) { - return { id: value }; - } - try { - const parsed = JSON.parse(value) as { id?: unknown; phase?: unknown; v?: unknown }; - if (parsed.v !== 1) { - return null; - } - return { - ...(typeof parsed.id === "string" ? { id: parsed.id } : {}), - ...(normalizeAssistantPhase(parsed.phase) - ? { phase: normalizeAssistantPhase(parsed.phase) } - : {}), - }; - } catch { - return null; - } -} - function sanitizeAssistantText(text: string): string { return stripThinkingTagsFromText( stripDowngradedToolCallText(stripModelSpecialTokens(stripMinimaxToolCallXml(text))), diff --git a/src/gateway/server-methods/chat.ts b/src/gateway/server-methods/chat.ts index 38036494fab..b76ebbfe983 100644 --- a/src/gateway/server-methods/chat.ts +++ b/src/gateway/server-methods/chat.ts @@ -19,6 +19,7 @@ import { normalizeInputProvenance, type InputProvenance } from "../../sessions/i import { resolveSendPolicy } from "../../sessions/send-policy.js"; import { parseAgentSessionKey } from "../../sessions/session-key-utils.js"; import { emitSessionTranscriptUpdate } from "../../sessions/transcript-events.js"; +import { extractAssistantVisibleText } from "../../shared/chat-message-content.js"; import { stripInlineDirectiveTagsForDisplay, stripInlineDirectiveTagsFromMessageForDisplay, @@ -680,35 +681,7 @@ function sanitizeChatHistoryMessage( * dropping messages that carry real text alongside a stale `content: "NO_REPLY"`. */ function extractAssistantTextForSilentCheck(message: unknown): string | undefined { - if (!message || typeof message !== "object") { - return undefined; - } - const entry = message as Record; - if (entry.role !== "assistant") { - return undefined; - } - if (typeof entry.text === "string") { - return entry.text; - } - if (typeof entry.content === "string") { - return entry.content; - } - if (!Array.isArray(entry.content) || entry.content.length === 0) { - return undefined; - } - - const texts: string[] = []; - for (const block of entry.content) { - if (!block || typeof block !== "object") { - return undefined; - } - const typed = block as { type?: unknown; text?: unknown }; - if (typed.type !== "text" || typeof typed.text !== "string") { - return undefined; - } - texts.push(typed.text); - } - return texts.length > 0 ? texts.join("\n") : undefined; + return extractAssistantVisibleText(message); } function sanitizeChatHistoryMessages(messages: unknown[], maxChars: number): unknown[] { diff --git a/src/gateway/server.chat.gateway-server-chat.test.ts b/src/gateway/server.chat.gateway-server-chat.test.ts index c2bed9917fa..ded0068f0aa 100644 --- a/src/gateway/server.chat.gateway-server-chat.test.ts +++ b/src/gateway/server.chat.gateway-server-chat.test.ts @@ -542,6 +542,29 @@ describe("gateway server chat", () => { expect(textValues).toEqual(["hello", "real reply", "real text field reply", "NO_REPLY"]); }); + test("chat.history hides commentary-only assistant entries", async () => { + const historyMessages = await loadChatHistoryWithMessages([ + { + role: "user", + content: [{ type: "text", text: "hello" }], + timestamp: 1, + }, + { + role: "assistant", + phase: "commentary", + content: [{ type: "text", text: "thinking like caveman" }], + timestamp: 2, + }, + { + role: "assistant", + content: [{ type: "text", text: "real reply" }], + timestamp: 3, + }, + ]); + + expect(collectHistoryTextValues(historyMessages)).toEqual(["hello", "real reply"]); + }); + test("routes chat.send slash commands without agent runs", async () => { await withMainSessionStore(async () => { const spy = vi.mocked(agentCommand); diff --git a/src/shared/chat-message-content.test.ts b/src/shared/chat-message-content.test.ts index f3b72c17ee9..1e4c15babb4 100644 --- a/src/shared/chat-message-content.test.ts +++ b/src/shared/chat-message-content.test.ts @@ -1,5 +1,9 @@ import { describe, expect, it } from "vitest"; -import { extractAssistantVisibleText, extractFirstTextBlock } from "./chat-message-content.js"; +import { + extractAssistantVisibleText, + extractFirstTextBlock, + resolveAssistantMessagePhase, +} from "./chat-message-content.js"; describe("shared/chat-message-content", () => { it("extracts the first text block from array content", () => { @@ -110,3 +114,46 @@ describe("extractAssistantVisibleText", () => { ).toBe("Actual final answer"); }); }); + +describe("resolveAssistantMessagePhase", () => { + it("prefers the top-level assistant phase when present", () => { + expect(resolveAssistantMessagePhase({ role: "assistant", phase: "commentary" })).toBe( + "commentary", + ); + }); + + it("resolves a single explicit phase from textSignature metadata", () => { + expect( + resolveAssistantMessagePhase({ + role: "assistant", + content: [ + { + type: "text", + text: "Actual final answer", + textSignature: JSON.stringify({ v: 1, id: "msg_final", phase: "final_answer" }), + }, + ], + }), + ).toBe("final_answer"); + }); + + it("returns undefined when text blocks contain mixed explicit phases", () => { + expect( + resolveAssistantMessagePhase({ + role: "assistant", + content: [ + { + type: "text", + text: "Working...", + textSignature: JSON.stringify({ v: 1, id: "msg_commentary", phase: "commentary" }), + }, + { + type: "text", + text: "Done.", + textSignature: JSON.stringify({ v: 1, id: "msg_final", phase: "final_answer" }), + }, + ], + }), + ).toBeUndefined(); + }); +}); diff --git a/src/shared/chat-message-content.ts b/src/shared/chat-message-content.ts index 07cc6c95912..0361ca84146 100644 --- a/src/shared/chat-message-content.ts +++ b/src/shared/chat-message-content.ts @@ -17,13 +17,13 @@ export function extractFirstTextBlock(message: unknown): string | undefined { return typeof text === "string" ? text : undefined; } -type AssistantPhase = "commentary" | "final_answer"; +export type AssistantPhase = "commentary" | "final_answer"; -function normalizeAssistantPhase(value: unknown): AssistantPhase | undefined { +export function normalizeAssistantPhase(value: unknown): AssistantPhase | undefined { return value === "commentary" || value === "final_answer" ? value : undefined; } -function parseAssistantTextSignature( +export function parseAssistantTextSignature( value: unknown, ): { id?: string; phase?: AssistantPhase } | null { if (typeof value !== "string" || value.trim().length === 0) { @@ -48,6 +48,46 @@ function parseAssistantTextSignature( } } +export function encodeAssistantTextSignature(params: { + id: string; + phase?: AssistantPhase; +}): string { + return JSON.stringify({ + v: 1, + id: params.id, + ...(params.phase ? { phase: params.phase } : {}), + }); +} + +export function resolveAssistantMessagePhase(message: unknown): AssistantPhase | undefined { + if (!message || typeof message !== "object") { + return undefined; + } + const entry = message as { phase?: unknown; content?: unknown }; + const directPhase = normalizeAssistantPhase(entry.phase); + if (directPhase) { + return directPhase; + } + if (!Array.isArray(entry.content)) { + return undefined; + } + const explicitPhases = new Set(); + for (const block of entry.content) { + if (!block || typeof block !== "object") { + continue; + } + const record = block as { type?: unknown; textSignature?: unknown }; + if (record.type !== "text") { + continue; + } + const phase = parseAssistantTextSignature(record.textSignature)?.phase; + if (phase) { + explicitPhases.add(phase); + } + } + return explicitPhases.size === 1 ? [...explicitPhases][0] : undefined; +} + function extractAssistantTextForPhase( message: unknown, phase?: AssistantPhase,