diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index 53ec27919..676e64a2a 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -12,6 +12,7 @@ ### Changed - Send the `omp/` User-Agent on xAI chat (`xai` and `xai-oauth`) unless the request already set its own. +- Fixed Gemini thought summaries occasionally leaking a raw `` ```thinking `` / `` ``````thinking `` fence delimiter into the reasoning block, so it no longer shows up as fence spam in the thinking display or persisted transcripts ([#8719](https://github.com/can1357/oh-my-pi/issues/8719)). ## [17.3.5] - 2026-08-16 diff --git a/packages/ai/src/dialect/thinking-fence-strip.ts b/packages/ai/src/dialect/thinking-fence-strip.ts new file mode 100644 index 000000000..df7cd17f7 --- /dev/null +++ b/packages/ai/src/dialect/thinking-fence-strip.ts @@ -0,0 +1,92 @@ +/** + * Strip self-referential reasoning-fence delimiters that a provider leaks + * *inside* a structured thinking part. + * + * The visible-channel healers ({@link ../utils/leaked-thinking-stream}, + * {@link ./fenced-thinking}) split leaked ` ```thinking ` fences out of the + * *text* stream. They never run over parts a provider already flags as + * thinking, so when a model (observed on Gemini thought summaries — issue + * #8719) emits a bare ` ```thinking ` / ` ``````thinking ` opener line between + * summaries, that delimiter reaches display and persistence verbatim as fence + * spam inside the reasoning block. + * + * This stripper removes only a *standalone* reasoning-fence opener line — a line + * that is nothing but a run of ≥3 backticks immediately followed by the info + * string `thinking` or `reasoning`. Such a line is always redundant inside an + * already-structured thinking block and never carries content. Inline mentions + * (prose on the same line), language-tagged code fences (` ```rs `), and bare + * closers (` ``` `) are left untouched so legitimate fenced code inside the + * reasoning survives. + * + * Streaming-safe: deltas may split a line anywhere. A trailing partial line is + * held only while it remains a viable opener prefix; the moment it cannot be an + * opener it is flushed and the rest of the line passes through character-level. + * Correctness never depends on the prefix heuristic — every held line is + * classified strictly on its newline (or on {@link ThinkingFenceStripper.flush}) + * before it is dropped. + */ + +/** + * A complete standalone reasoning-fence opener: ≤3 lead spaces, ≥3 backticks, + * `thinking`/`reasoning`, optional trailing spaces, tolerating a trailing CR + * from a CRLF newline. + */ +const OPENER_LINE = /^ {0,3}`{3,}(?:thinking|reasoning)[ \t]*\r?$/i; + +/** Could `line` (a partial, newline-not-yet-seen) still grow into {@link OPENER_LINE}? */ +function couldBeOpenerPrefix(line: string): boolean { + // Tolerate a pending CR from a split CRLF. + const s = line.endsWith("\r") ? line.slice(0, -1) : line; + const m = /^ {0,3}(`*)([\s\S]*)$/.exec(s); + if (!m) return false; + const ticks = m[1]!.length; + const rest = m[2]!; + if (rest === "") return true; // still consuming leading spaces / backticks + if (ticks < 3) return false; // a non-backtick char appeared before 3 backticks: never a fence + const word = rest.replace(/[ \t]+$/, "").toLowerCase(); + return "thinking".startsWith(word) || "reasoning".startsWith(word); +} + +/** + * Stateful, line-oriented stripper for leaked reasoning-fence openers in one + * structured thinking block. One instance per thinking block; feed every + * thinking delta through {@link push} and drain the tail with {@link flush}. + */ +export class ThinkingFenceStripper { + /** Buffered content of the current line still being classified. */ + #carry = ""; + /** True once the current line is known not to be an opener; passes through until newline. */ + #passthrough = false; + + /** Consume one thinking delta; returns the sanitized text to emit (may be empty). */ + push(chunk: string): string { + let out = ""; + for (const ch of chunk) { + if (this.#passthrough) { + out += ch; + if (ch === "\n") this.#passthrough = false; + continue; + } + if (ch === "\n") { + if (!OPENER_LINE.test(this.#carry)) out += `${this.#carry}\n`; + this.#carry = ""; + continue; + } + this.#carry += ch; + if (!couldBeOpenerPrefix(this.#carry)) { + out += this.#carry; + this.#carry = ""; + this.#passthrough = true; + } + } + return out; + } + + /** Drain any held partial line at block end; returns text to emit (may be empty). */ + flush(): string { + const carry = this.#carry; + this.#carry = ""; + this.#passthrough = false; + return OPENER_LINE.test(carry) ? "" : carry; + } +} diff --git a/packages/ai/src/providers/google-shared.ts b/packages/ai/src/providers/google-shared.ts index 9af27d847..dd5cdcaca 100644 --- a/packages/ai/src/providers/google-shared.ts +++ b/packages/ai/src/providers/google-shared.ts @@ -6,6 +6,7 @@ import { scheduler } from "node:timers/promises"; import { calculateCost } from "@oh-my-pi/pi-catalog/models"; import { readSseJson } from "@oh-my-pi/pi-utils"; import { renderDemotedThinking } from "../dialect/demotion"; +import { ThinkingFenceStripper } from "../dialect/thinking-fence-strip"; import * as AIError from "../error"; import type { Api, @@ -617,11 +618,23 @@ export async function consumeGoogleStream(args: { const blocks = output.content; const blockIndex = () => blocks.length - 1; let currentBlock: TextContent | ThinkingContent | null = null; + // Heals a leaked reasoning-fence opener (```thinking / ``````thinking) that some + // Gemini thought summaries emit as a between-summary delimiter (#8719). One + // stripper per thinking block; created lazily on first thinking delta. + let thinkingStripper: ThinkingFenceStripper | null = null; let firstTokenSeen = false; let sawFinishReason = false; const flushCurrent = () => { if (!currentBlock) return; + if (currentBlock.type === "thinking" && thinkingStripper) { + const tail = thinkingStripper.flush(); + if (tail) { + currentBlock.thinking += tail; + stream.push({ type: "thinking_delta", contentIndex: blockIndex(), delta: tail, partial: output }); + } + } + thinkingStripper = null; pushBlockEndEvent(currentBlock, blockIndex(), output, stream); }; @@ -658,17 +671,21 @@ export async function consumeGoogleStream(args: { currentBlock = startTextOrThinkingBlock(isThinking, output, stream); } if (currentBlock.type === "thinking") { - currentBlock.thinking += part.text; + thinkingStripper ??= new ThinkingFenceStripper(); + const cleaned = thinkingStripper.push(part.text); + currentBlock.thinking += cleaned; currentBlock.thinkingSignature = retainThoughtSignature( currentBlock.thinkingSignature, part.thoughtSignature, ); - stream.push({ - type: "thinking_delta", - contentIndex: blockIndex(), - delta: part.text, - partial: output, - }); + if (cleaned) { + stream.push({ + type: "thinking_delta", + contentIndex: blockIndex(), + delta: cleaned, + partial: output, + }); + } } else { currentBlock.text += part.text; if (retainTextSignature) { diff --git a/packages/ai/test/google-thinking-fence-strip.test.ts b/packages/ai/test/google-thinking-fence-strip.test.ts new file mode 100644 index 000000000..e0f17696e --- /dev/null +++ b/packages/ai/test/google-thinking-fence-strip.test.ts @@ -0,0 +1,134 @@ +import { describe, expect, it } from "bun:test"; +import { ThinkingFenceStripper } from "@oh-my-pi/pi-ai/dialect/thinking-fence-strip"; +import { consumeGoogleStream } from "@oh-my-pi/pi-ai/providers/google-shared"; +import type { GenerateContentResponse, Part } from "@oh-my-pi/pi-ai/providers/google-types"; +import type { AssistantMessage, AssistantMessageEvent, Model } from "@oh-my-pi/pi-ai/types"; +import { AssistantMessageEventStream } from "@oh-my-pi/pi-ai/utils/event-stream"; +import { buildModel } from "@oh-my-pi/pi-catalog/build"; + +// Regression for #8719: Gemini thought summaries occasionally emit a bare +// ```thinking / ``````thinking opener line as a between-summary delimiter. +// It must never reach the persisted thinking text or the streamed thinking_delta. + +const feed = (stripper: ThinkingFenceStripper, chunks: string[]): string => + chunks.map(chunk => stripper.push(chunk)).join("") + stripper.flush(); + +describe("ThinkingFenceStripper", () => { + it("drops a standalone reasoning-fence opener line (any backtick run ≥3)", () => { + expect(feed(new ThinkingFenceStripper(), ["a\n```thinking\nb\n"])).toBe("a\nb\n"); + expect(feed(new ThinkingFenceStripper(), ["a\n``````thinking\nb\n"])).toBe("a\nb\n"); + expect(feed(new ThinkingFenceStripper(), ["```reasoning\nx"])).toBe("x"); + }); + + it("strips an opener even when the delimiter is split across deltas", () => { + expect(feed(new ThinkingFenceStripper(), ["intro\n``", "````thin", "king\nrest"])).toBe("intro\nrest"); + }); + + it("drops a trailing opener that never gets its newline (flush path)", () => { + expect(feed(new ThinkingFenceStripper(), ["done\n``````thinking"])).toBe("done\n"); + }); + + it("preserves language-tagged code fences and bare closers inside reasoning", () => { + const body = "look:\n```rs\nlet x = 1;\n```\ndone\n"; + expect(feed(new ThinkingFenceStripper(), [body])).toBe(body); + }); + + it("keeps inline mentions of the idiom (prose on the fence line)", () => { + const line = "I should emit a ```thinking block here.\n"; + expect(feed(new ThinkingFenceStripper(), [line])).toBe(line); + }); + + it("keeps indented content that only resembles a fence", () => { + // 4-space indent is a code line, not a fence; must survive verbatim. + expect(feed(new ThinkingFenceStripper(), [" ```thinking\n"])).toBe(" ```thinking\n"); + }); +}); + +const vertexModel: Model<"google-vertex"> = buildModel({ + id: "gemini-3.7-flash", + name: "Gemini 3.7 Flash (Vertex)", + api: "google-vertex", + provider: "google-vertex", + baseUrl: "", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200_000, + maxTokens: 32_000, +}); + +function emptyAssistant(): AssistantMessage { + return { + role: "assistant", + content: [], + api: "google-vertex", + provider: "google-vertex", + model: vertexModel.id, + usage: { + input: 0, + output: 0, + cacheRead: 0, + cacheWrite: 0, + totalTokens: 0, + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0, total: 0 }, + }, + stopReason: "stop", + timestamp: 0, + }; +} + +/** + * Drive `consumeGoogleStream` over fabricated chunks (each an array of parts) + * and return the persisted thinking text plus the concatenated thinking deltas. + */ +async function runThinking(chunks: Part[][]): Promise<{ thinking: string; streamed: string }> { + const output = emptyAssistant(); + const stream = new AssistantMessageEventStream(); + const streamed: string[] = []; + const collecting = (async () => { + for await (const event of stream as AsyncIterable) { + if (event.type === "thinking_delta") streamed.push(event.delta); + } + })(); + + async function* googleStream(): AsyncGenerator { + for (const parts of chunks) { + yield { candidates: [{ content: { parts } }] } as unknown as GenerateContentResponse; + } + yield { candidates: [{ finishReason: "STOP" }] } as unknown as GenerateContentResponse; + } + + await consumeGoogleStream({ googleStream: googleStream(), output, stream, model: vertexModel, options: undefined }); + stream.end(output); + await collecting; + + const block = output.content.find(b => b.type === "thinking"); + return { thinking: block?.thinking ?? "", streamed: streamed.join("") }; +} + +describe("consumeGoogleStream leaked thinking-fence delimiter (#8719)", () => { + it("heals a leaked ```thinking delimiter out of persistence and streaming", async () => { + const { thinking, streamed } = await runThinking([ + [{ text: "Investigating the return type.\n", thought: true }], + [{ text: "``````thinking\n**Investigating Adapter Host Logic**\n", thought: true }], + [{ text: "The host owns the loop.", thought: true }], + ]); + const expected = "Investigating the return type.\n**Investigating Adapter Host Logic**\nThe host owns the loop."; + expect(thinking).toBe(expected); + expect(thinking).not.toContain("```thinking"); + expect(streamed).toBe(expected); + }); + + it("leaves normal thought summaries untouched", async () => { + const clean = "Considered options A and B; picked B for latency."; + const { thinking, streamed } = await runThinking([[{ text: clean, thought: true }]]); + expect(thinking).toBe(clean); + expect(streamed).toBe(clean); + }); + + it("does not strip the same idiom from visible (non-thought) text", async () => { + const { thinking } = await runThinking([[{ text: "here:\n```thinking\nx\n", thought: false }]]); + // No thinking block at all — the text branch is untouched by the stripper. + expect(thinking).toBe(""); + }); +});