From d6b1a3f06e763619e32b980e1e94daf853fe2053 Mon Sep 17 00:00:00 2001 From: Leon Zhao Date: Sat, 11 Jul 2026 05:16:25 +0000 Subject: [PATCH 1/2] fix: filter empty codex reasoning comments Model: GPT-5 --- AGENTS.md | 3 + src/CodexAcpServer.ts | 6 +- src/CodexEventHandler.ts | 43 +++++++++-- src/ReasoningText.ts | 77 +++++++++++++++++++ src/ResponseItemHistoryFallback.ts | 20 +++-- .../CodexACPAgent/reasoning-events.test.ts | 53 +++++++++++++ src/__tests__/ReasoningText.test.ts | 32 ++++++++ 7 files changed, 220 insertions(+), 14 deletions(-) create mode 100644 src/ReasoningText.ts create mode 100644 src/__tests__/ReasoningText.test.ts diff --git a/AGENTS.md b/AGENTS.md index 960f4ad4..434c6cd0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -28,3 +28,6 @@ - Codex app-server usage: see https://github.com/openai/codex/blob/main/codex-rs/app-server/README.md when touching protocol/transport details, adding or consuming JSON-RPC methods, handling approvals/turn events, or updating generated schema/clients. - App-server events: prefer `thread/*`, `turn/*`, and `item/*` event surfaces; avoid the deprecated `codex/event/*` API (planned removal). Keep implementations aligned with generated types in `src/app-server` (including `v2` exports). +- Codex reasoning summaries can echo trailing empty HTML comments from model instructions. Keep + that provider-specific cleanup in `src/ReasoningText.ts` across live deltas and history replay; + do not filter assistant text, raw reasoning, or HTML globally in the client renderer. diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index 06475fc7..ccf98ff3 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -74,6 +74,7 @@ import { import packageJson from "../package.json"; import {isJetBrains2026_1Client} from "./JBUtils"; import {resolveTerminalOutputMode, type TerminalOutputMode} from "./TerminalOutputMode"; +import {stripEmptyReasoningComments} from "./ReasoningText"; import { createCodexMessagePhaseMeta, createAgentTextMessageChunk, @@ -1084,7 +1085,10 @@ export class CodexAcpServer { private createReasoningUpdates(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent[] { const parts = item.summary.length > 0 ? item.summary : item.content; const messageId = item.id; - return parts.map((text) => createAgentTextThoughtChunk(text, messageId)); + return parts + .map((text) => item.summary.length > 0 ? stripEmptyReasoningComments(text) : text) + .filter((text) => text.length > 0) + .map((text) => createAgentTextThoughtChunk(text, messageId)); } private createWebSearchUpdate( diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 13594fb0..0713c9f8 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -65,6 +65,7 @@ import { } from "./CodexToolCallMapper"; import { stripShellPrefix } from "./CommandUtils"; import {createTerminalOutputMeta, type TerminalOutputMode} from "./TerminalOutputMode"; +import {ReasoningSummaryFilter, stripEmptyReasoningComments} from "./ReasoningText"; import { createCodexMessagePhaseMeta, createAgentTextMessageChunk, @@ -83,6 +84,7 @@ export class CodexEventHandler { private readonly activeImageGenerationItems = new Set(); private readonly emittedImageViewItems = new Set(); private readonly seenReasoningDeltaItemIds = new Set(); + private readonly reasoningSummaryFilters = new Map(); private readonly terminalCommandIds = new Set(); private readonly terminalCommandOutputIds = new Set(); private proposedPlanMarkdown = ""; @@ -175,9 +177,9 @@ export class CodexEventHandler { case "thread/compacted": return this.createContextCompactedEvent(); case "item/reasoning/summaryTextDelta": - return this.createReasoningDeltaEvent(notification.params); + return this.createReasoningSummaryDeltaEvent(notification.params); case "item/reasoning/textDelta": - return this.createReasoningDeltaEvent(notification.params); + return this.createRawReasoningDeltaEvent(notification.params); case "item/reasoning/summaryPartAdded": return this.createReasoningSectionBreakEvent(notification.params); case "model/rerouted": @@ -404,16 +406,35 @@ export class CodexEventHandler { && left.tokenBudget === right.tokenBudget; } - private createReasoningDeltaEvent( - event: ReasoningSummaryTextDeltaNotification | ReasoningTextDeltaNotification - ): UpdateSessionEvent { + private createReasoningSummaryDeltaEvent(event: ReasoningSummaryTextDeltaNotification): UpdateSessionEvent | null { + this.seenReasoningDeltaItemIds.add(event.itemId); + let filter = this.reasoningSummaryFilters.get(event.itemId); + if (!filter) { + filter = new ReasoningSummaryFilter(); + this.reasoningSummaryFilters.set(event.itemId, filter); + } + const text = filter.push(event.delta); + return text.length > 0 ? this.createAgentThoughtEvent(text, event.itemId) : null; + } + + private createRawReasoningDeltaEvent(event: ReasoningTextDeltaNotification): UpdateSessionEvent { this.seenReasoningDeltaItemIds.add(event.itemId); return this.createAgentThoughtEvent(event.delta, event.itemId); } private createReasoningSectionBreakEvent(event: ReasoningSummaryPartAddedNotification): UpdateSessionEvent { this.seenReasoningDeltaItemIds.add(event.itemId); - return this.createAgentThoughtEvent("\n\n", event.itemId); + const trailingText = this.finishReasoningSummaryFilter(event.itemId); + return this.createAgentThoughtEvent(`${trailingText}\n\n`, event.itemId); + } + + private finishReasoningSummaryFilter(itemId: string): string { + const filter = this.reasoningSummaryFilters.get(itemId); + if (!filter) { + return ""; + } + this.reasoningSummaryFilters.delete(itemId); + return filter.finish(); } private createAgentThoughtEvent(text: string, messageId: string): UpdateSessionEvent { @@ -494,7 +515,10 @@ export class CodexEventHandler { return createImageGenerationUpdate(event.item, { terminalStatus: true }); case "reasoning": if (this.seenReasoningDeltaItemIds.delete(event.item.id)) { - return null; + const trailingText = this.finishReasoningSummaryFilter(event.item.id); + return trailingText.length > 0 + ? this.createAgentThoughtEvent(trailingText, event.item.id) + : null; } return this.createCompletedReasoningEvent(event.item); case "webSearch": @@ -526,7 +550,10 @@ export class CodexEventHandler { private createCompletedReasoningEvent(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent | null { const parts = item.summary.length > 0 ? item.summary : item.content; - const text = parts.filter(part => part.length > 0).join("\n\n"); + const text = parts + .map(part => item.summary.length > 0 ? stripEmptyReasoningComments(part) : part) + .filter(part => part.length > 0) + .join("\n\n"); if (text.length === 0) { return null; } diff --git a/src/ReasoningText.ts b/src/ReasoningText.ts new file mode 100644 index 00000000..c46c4a42 --- /dev/null +++ b/src/ReasoningText.ts @@ -0,0 +1,77 @@ +const COMPLETE_EMPTY_COMMENT = /^\s*$/u; +const PARTIAL_COMMENT_BODY = /^\s*(?:\*\/?\s*)?$/u; +const PARTIAL_COMMENT_CLOSE = /^\s*(?:\*\/\s*)?-{1,2}$/u; + +const isHorizontalWhitespace = (char: string): boolean => char === " " || char === "\t"; + +const canBecomeEmptyComment = (text: string): boolean => { + if (text === "<" || text === " { + output += text; + for (const char of text) { + if (char === "\n" || char === "\r") { + this.lineHasContent = false; + } else if (!isHorizontalWhitespace(char)) { + this.lineHasContent = true; + } + } + }; + + for (const char of delta) { + if (this.pending) { + this.pending += char; + if (canBecomeEmptyComment(this.pending)) { + continue; + } + emit(this.pending); + this.pending = ""; + continue; + } + + if (char === "<" && !this.lineHasContent) { + this.pending = char; + } else { + emit(char); + } + } + + return output; + } + + finish(): string { + const output = COMPLETE_EMPTY_COMMENT.test(this.pending) ? "" : this.pending; + this.pending = ""; + this.lineHasContent = false; + return output; + } +} + +export function stripEmptyReasoningComments(text: string): string { + const filter = new ReasoningSummaryFilter(); + return filter.push(text) + filter.finish(); +} diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index 0730455f..261d77e5 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -7,6 +7,7 @@ import type { CommandAction, Thread, ThreadItem } from "./app-server/v2"; import { createCommandActionEvent } from "./CodexToolCallMapper"; import { createTerminalOutputMeta, type TerminalOutputMode } from "./TerminalOutputMode"; import { createAgentMessageChunk, createCodexMessagePhaseMeta } from "./ContentChunks"; +import { stripEmptyReasoningComments } from "./ReasoningText"; type JsonRecord = Record; type AcpToolCallEvent = Extract; @@ -282,9 +283,14 @@ function createAgentReasoningEventUpdates(payload: JsonRecord): UpdateSessionEve return []; } + const sanitizedText = stripEmptyReasoningComments(text); + if (sanitizedText.length === 0) { + return []; + } + return [{ sessionUpdate: "agent_thought_chunk", - content: { type: "text", text }, + content: { type: "text", text: sanitizedText }, }]; } @@ -331,14 +337,18 @@ function contentBlocksFromResponseContent(content: unknown): ContentBlock[] { function createReasoningUpdates(item: JsonRecord): UpdateSessionEvent[] { const parts = textParts(item["summary"]); + const hasSummary = parts.length > 0; if (parts.length === 0) { parts.push(...textParts(item["content"])); } - return parts.map((text) => ({ - sessionUpdate: "agent_thought_chunk", - content: { type: "text", text }, - })); + return parts + .map((text) => hasSummary ? stripEmptyReasoningComments(text) : text) + .filter((text) => text.length > 0) + .map((text) => ({ + sessionUpdate: "agent_thought_chunk" as const, + content: { type: "text" as const, text }, + })); } function textParts(value: unknown): string[] { diff --git a/src/__tests__/CodexACPAgent/reasoning-events.test.ts b/src/__tests__/CodexACPAgent/reasoning-events.test.ts index 3d9ef367..2427c019 100644 --- a/src/__tests__/CodexACPAgent/reasoning-events.test.ts +++ b/src/__tests__/CodexACPAgent/reasoning-events.test.ts @@ -102,4 +102,57 @@ describe("CodexEventHandler - reasoning events", () => { "data/reasoning-completed-parts.json" ); }); + + it("filters a trailing empty comment split across reasoning summary deltas", async () => { + const notifications: ServerNotification[] = [ + { + method: "item/reasoning/summaryTextDelta", + params: { + threadId: sessionId, + turnId: "turn-1", + itemId: "reasoning-comment", + summaryIndex: 0, + delta: "**Checking files**\n\n", + }, + }, + { + method: "item/completed", + params: { + threadId: sessionId, + turnId: "turn-1", + completedAtMs: 0, + item: { + type: "reasoning", + id: "reasoning-comment", + summary: ["**Checking files**\n\n"], + content: [], + }, + }, + }, + ]; + + await setupPromptAndSendNotifications(mockFixture, sessionId, sessionState, notifications); + + expect(mockFixture.getAcpConnectionEvents([])).toEqual([{ + method: "sessionUpdate", + args: [{ + sessionId, + update: { + sessionUpdate: "agent_thought_chunk", + messageId: "reasoning-comment", + content: {type: "text", text: "**Checking files**\n\n"}, + }, + }], + }]); + }); }); diff --git a/src/__tests__/ReasoningText.test.ts b/src/__tests__/ReasoningText.test.ts new file mode 100644 index 00000000..b7bd6d2f --- /dev/null +++ b/src/__tests__/ReasoningText.test.ts @@ -0,0 +1,32 @@ +import { describe, expect, it } from "vitest"; +import { ReasoningSummaryFilter, stripEmptyReasoningComments } from "../ReasoningText"; + +describe("reasoning summary text", () => { + it("strips trailing empty comments, including the observed malformed variant", () => { + expect(stripEmptyReasoningComments("**Checking files**\n\n")).toBe( + "**Checking files**\n\n" + ); + expect(stripEmptyReasoningComments("**Checking files**\n\n \n")).toBe( + "**Checking files**\n\n" + ); + }); + + it("handles a marker split at every delta boundary", () => { + const text = "**Checking files**\n\n"; + for (let split = 0; split <= text.length; split += 1) { + const filter = new ReasoningSummaryFilter(); + const output = filter.push(text.slice(0, split)) + + filter.push(text.slice(split)) + + filter.finish(); + expect(output).toBe("**Checking files**\n\n"); + } + }); + + it("preserves comments that may be meaningful", () => { + expect(stripEmptyReasoningComments("Text ")).toBe("Text "); + expect(stripEmptyReasoningComments("")).toBe(""); + expect(stripEmptyReasoningComments("\nMore text")).toBe("\nMore text"); + expect(stripEmptyReasoningComments("```html\n\n```")) + .toBe("```html\n\n```"); + }); +}); From 6b2f019299c96913dfeff894afd916ced8175991 Mon Sep 17 00:00:00 2001 From: Leon Zhao Date: Sat, 11 Jul 2026 06:08:55 +0000 Subject: [PATCH 2/2] refactor: dedupe reasoning-part sanitization into one helper Collapse the three copy-pasted "pick summary-vs-content, strip empty comments only for summary, drop empties" pipelines (CodexAcpServer, CodexEventHandler, ResponseItemHistoryFallback) into a single sanitizeReasoningParts() helper in ReasoningText.ts, so the "strip summary only, never raw content" invariant lives in one place. Model: claude-opus-4-8[1m] Co-Authored-By: Claude Opus 4.8 (1M context) --- src/CodexAcpServer.ts | 7 ++----- src/CodexEventHandler.ts | 8 ++------ src/ReasoningText.ts | 13 +++++++++++++ src/ResponseItemHistoryFallback.ts | 12 ++---------- 4 files changed, 19 insertions(+), 21 deletions(-) diff --git a/src/CodexAcpServer.ts b/src/CodexAcpServer.ts index ccf98ff3..c846294f 100644 --- a/src/CodexAcpServer.ts +++ b/src/CodexAcpServer.ts @@ -74,7 +74,7 @@ import { import packageJson from "../package.json"; import {isJetBrains2026_1Client} from "./JBUtils"; import {resolveTerminalOutputMode, type TerminalOutputMode} from "./TerminalOutputMode"; -import {stripEmptyReasoningComments} from "./ReasoningText"; +import {sanitizeReasoningParts} from "./ReasoningText"; import { createCodexMessagePhaseMeta, createAgentTextMessageChunk, @@ -1083,11 +1083,8 @@ export class CodexAcpServer { } private createReasoningUpdates(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent[] { - const parts = item.summary.length > 0 ? item.summary : item.content; const messageId = item.id; - return parts - .map((text) => item.summary.length > 0 ? stripEmptyReasoningComments(text) : text) - .filter((text) => text.length > 0) + return sanitizeReasoningParts(item.summary, item.content) .map((text) => createAgentTextThoughtChunk(text, messageId)); } diff --git a/src/CodexEventHandler.ts b/src/CodexEventHandler.ts index 0713c9f8..9836aa56 100644 --- a/src/CodexEventHandler.ts +++ b/src/CodexEventHandler.ts @@ -65,7 +65,7 @@ import { } from "./CodexToolCallMapper"; import { stripShellPrefix } from "./CommandUtils"; import {createTerminalOutputMeta, type TerminalOutputMode} from "./TerminalOutputMode"; -import {ReasoningSummaryFilter, stripEmptyReasoningComments} from "./ReasoningText"; +import {ReasoningSummaryFilter, sanitizeReasoningParts} from "./ReasoningText"; import { createCodexMessagePhaseMeta, createAgentTextMessageChunk, @@ -549,11 +549,7 @@ export class CodexEventHandler { } private createCompletedReasoningEvent(item: ThreadItem & { type: "reasoning" }): UpdateSessionEvent | null { - const parts = item.summary.length > 0 ? item.summary : item.content; - const text = parts - .map(part => item.summary.length > 0 ? stripEmptyReasoningComments(part) : part) - .filter(part => part.length > 0) - .join("\n\n"); + const text = sanitizeReasoningParts(item.summary, item.content).join("\n\n"); if (text.length === 0) { return null; } diff --git a/src/ReasoningText.ts b/src/ReasoningText.ts index c46c4a42..b1e9c417 100644 --- a/src/ReasoningText.ts +++ b/src/ReasoningText.ts @@ -75,3 +75,16 @@ export function stripEmptyReasoningComments(text: string): string { const filter = new ReasoningSummaryFilter(); return filter.push(text) + filter.finish(); } + +/** + * Selects a reasoning item's displayable parts: summary when present, else raw content. Only the + * summary carries the model's empty-comment markers, so stripping is applied to summary parts only, + * never to raw content. Empty parts are dropped. + */ +export function sanitizeReasoningParts(summary: string[], content: string[]): string[] { + const useSummary = summary.length > 0; + const parts = useSummary ? summary : content; + return parts + .map((part) => useSummary ? stripEmptyReasoningComments(part) : part) + .filter((part) => part.length > 0); +} diff --git a/src/ResponseItemHistoryFallback.ts b/src/ResponseItemHistoryFallback.ts index 261d77e5..9cbb7de9 100644 --- a/src/ResponseItemHistoryFallback.ts +++ b/src/ResponseItemHistoryFallback.ts @@ -7,7 +7,7 @@ import type { CommandAction, Thread, ThreadItem } from "./app-server/v2"; import { createCommandActionEvent } from "./CodexToolCallMapper"; import { createTerminalOutputMeta, type TerminalOutputMode } from "./TerminalOutputMode"; import { createAgentMessageChunk, createCodexMessagePhaseMeta } from "./ContentChunks"; -import { stripEmptyReasoningComments } from "./ReasoningText"; +import { sanitizeReasoningParts, stripEmptyReasoningComments } from "./ReasoningText"; type JsonRecord = Record; type AcpToolCallEvent = Extract; @@ -336,15 +336,7 @@ function contentBlocksFromResponseContent(content: unknown): ContentBlock[] { } function createReasoningUpdates(item: JsonRecord): UpdateSessionEvent[] { - const parts = textParts(item["summary"]); - const hasSummary = parts.length > 0; - if (parts.length === 0) { - parts.push(...textParts(item["content"])); - } - - return parts - .map((text) => hasSummary ? stripEmptyReasoningComments(text) : text) - .filter((text) => text.length > 0) + return sanitizeReasoningParts(textParts(item["summary"]), textParts(item["content"])) .map((text) => ({ sessionUpdate: "agent_thought_chunk" as const, content: { type: "text" as const, text },