fix(ai): scope demoted-thinking separator to the demoted text block itself

Previous attempt joined every adjacent assistant text block with \n in convertOpenAICompletionsMessages, which changed conversation history for imported transcripts, bridge stitching, and streaming chunk splits — Codex flagged the regression on #4432. Move the paragraph terminator onto the demoted-thinking text block content produced by transform-messages line 490 instead, and revert the openai-completions flatten to .join(""). The terminator now targets the cross-API demotion boundary only: ordinary multi-block visible text stays byte-identical on flatten, and the bare Anthropic-dialect demoted reasoning still separates from a following visible answer via the trailing \n on the demoted block. The anthropic-messages wire path demotes at convertAnthropicMessages lines 3467/3489 (renderDemotedThinking direct) and ships text blocks as separate wire blocks — no flatten, no collision — so the terminator is not appended there, keeping the bare-prose invariant on the anthropic wire. Test assertions realigned: openai-completions flatten targets expect ${renderDemotedThinking(...)}\n; anthropic-messages wire targets keep bare prose. Both branches locked with expanded coverage. Refs #4430
2026-07-03 23:35:13 +00:00
parent 1913d354e1
commit 772da22d61
8 changed files with 41 additions and 24 deletions
@@ -4,7 +4,7 @@

 ### Fixed

- Fixed prior-turn reasoning demotion for Anthropic Claude models by generalizing the Fable-only bare-prose branch to the whole Anthropic dialect, so cross-model replays to Opus, Sonnet, Haiku, and Mythos no longer emit `<thinking>` tags that Anthropic's reasoning_extraction classifier flags as visible reasoning leakage. Also extended the Claude-id fallback classifier to Bedrock cross-region inference profiles (`us.anthropic.claude-…`, `eu.anthropic.claude-…`, etc.) so Haiku dotted profiles route to the Anthropic dialect, and separated demoted-thinking from the following visible answer in the openai-completions flatten path so bare Anthropic-dialect reasoning no longer runs into the reply text ([#4430](https://github.com/can1357/oh-my-pi/issues/4430)).
+- Fixed prior-turn reasoning demotion for Anthropic Claude models by generalizing the Fable-only bare-prose branch to the whole Anthropic dialect, so cross-model replays to Opus, Sonnet, Haiku, and Mythos no longer emit `<thinking>` tags that Anthropic's reasoning_extraction classifier flags as visible reasoning leakage. Also extended the Claude-id fallback classifier to Bedrock cross-region inference profiles (`us.anthropic.claude-…`, `eu.anthropic.claude-…`, etc.) so Haiku dotted profiles route to the Anthropic dialect, and appended a paragraph terminator to the demoted-thinking text block itself so bare Anthropic-dialect reasoning no longer runs into the following visible-answer block when the openai-completions convert path flattens adjacent text (without corrupting ordinary multi-block visible text from bridges, imported transcripts, or streaming chunk splits) ([#4430](https://github.com/can1357/oh-my-pi/issues/4430)).

 ## [16.3.4] - 2026-07-03

@@ -1778,11 +1778,14 @@ export function convertMessages(
 				// Always send assistant content as a plain string. Some OpenAI-compatible
 				// backends mirror array-of-text-block payloads back to the model literally,
 				// causing recursive nested content in subsequent turns.
-				// Separate distinct text blocks with a newline: for Anthropic-dialect
-				// targets `renderDemotedThinking` returns bare prose (no trailing
-				// delimiter), so a demoted-thinking block would otherwise run into the
-				// following visible-answer block ("reasoningfinal answer").
-				assistantMsg.content = nonEmptyTextBlocks.map(b => b.text.toWellFormed()).join("\n");
+				// Join without a separator so ordinary adjacent text blocks (bridge
+				// stitching, imported transcripts, streaming chunks) preserve their
+				// original byte sequence. The cross-model demotion path in
+				// transform-messages already appends its own paragraph terminator to
+				// the demoted-thinking text block content, so a bare Anthropic-dialect
+				// reasoning block still separates cleanly from the following visible
+				// answer without corrupting unrelated multi-block content.
+				assistantMsg.content = nonEmptyTextBlocks.map(b => b.text.toWellFormed()).join("");
 			}

 			// Handle thinking blocks
@@ -476,9 +476,18 @@ export function transformMessages<TApi extends Api>(
 					// TARGET model's own canonical thinking-block dialect (e.g. a ```thinking
 					// fence for Gemini) so it reads as reasoning rather than bare prose the
 					// model might mimic.
+					// Self-terminate the demoted text with a paragraph break so the
+					// bare Anthropic-dialect output (or any dialect's wrapped output
+					// whose closing tag isn't a natural word boundary) can't collide
+					// with the following visible-text block when a downstream consumer
+					// flattens adjacent text blocks (openai-completions convert). The
+					// terminator lives on the demoted block itself, so it targets the
+					// demoted-thinking boundary only — ordinary adjacent text blocks
+					// stitched from streaming / bridges / imported transcripts stay
+					// byte-identical on flatten.
 					return {
 						type: "text" as const,
-						text: renderDemotedThinking(model.id, sanitized.thinking),
+						text: `${renderDemotedThinking(model.id, sanitized.thinking)}\n`,
 					};
 				}

@@ -157,8 +157,12 @@ describe("Anthropic abandoned/aborted tool-use replay", () => {
 		expect(blocks.some(b => b.type === "thinking" && b.signature === "sig_done")).toBe(true);
 		expect(blocks.some(b => b.type === "thinking" && b.signature === "trunc")).toBe(false);
 		// Anthropic-dialect demotion of unsigned prior thinking is bare prose
-		// (the reasoning_extraction classifier flags `<thinking>` tags across the
-		// whole Claude family, not just Fable).
+		// (the reasoning_extraction classifier flags `<thinking>` tags across
+		// the whole Claude family, not just Fable). No trailing terminator on
+		// the anthropic-messages wire path: convertAnthropicMessages emits
+		// text blocks as separate wire blocks (no flatten), so the paragraph
+		// terminator lives only on the cross-API demotion in transform-messages
+		// where downstream openai-completions flattens adjacent text.
 		expect(blocks.some(b => b.type === "text" && b.text === "now decide")).toBe(true);
 		expect(blocks.some(b => b.type === "tool_use")).toBe(true);
 	});
@@ -645,7 +645,11 @@ describe("anthropic stream envelope handling", () => {
 		// Anthropic-dialect demotion of unsigned prior thinking is bare prose —
 		// the reasoning_extraction classifier flags `<thinking>` tags in prior
 		// turn history for the whole Claude family, so replaying the unwrapped
-		// reasoning as wrapped text would just re-introduce the same leak.
+		// reasoning as wrapped text would just re-introduce the same leak. No
+		// trailing terminator on the anthropic-messages wire path:
+		// convertAnthropicMessages emits text blocks as separate wire blocks
+		// (no flatten), so the paragraph terminator lives only on the
+		// cross-API demotion in transform-messages.
 		expect(replayAssistant?.content).toEqual([
 			{ type: "text", text: "Check logs before accepting container health." },
 		]);
@@ -356,7 +356,7 @@ describe("DeepSeek reasoning_content tool-call replay", () => {
 			const assistant = findOpenAICompletionAssistantWireMessage(messages);
 			expect(assistant).toBeDefined();
 			expect(assistant?.reasoning_content).toBe("");
-			expect(assistant?.content).toBe(renderDemotedThinking(model.id, "Need to preserve cross-api reasoning."));
+			expect(assistant?.content).toBe(`${renderDemotedThinking(model.id, "Need to preserve cross-api reasoning.")}\n`);
 		});
 		it("falls through to empty-string when thinking block has opaque signature and empty text", () => {
 			const model = deepseekModel({
@@ -233,14 +233,11 @@ describe("openai-completions compatibility", () => {
 			throw new Error("assistant message missing");
 		}
 		expect(typeof assistant.content).toBe("string");
-		// Distinct text blocks are joined with `\n` — see the flatten path in
-		// openai-completions.ts. Streaming accumulates chunks into a single
-		// block, so multi-block content represents semantically separate
-		// segments (a demoted-thinking block followed by the visible answer,
-		// or a text block flanking a tool call). Bare Anthropic-dialect
-		// demoted reasoning has no self-terminator, so the join site MUST
-		// insert one to keep the two segments from running together.
-		expect(assistant.content).toBe("hello\n world");
+		// Ordinary adjacent text blocks (bridge stitching, imported transcripts,
+		// streaming chunk splits) preserve their original byte sequence on
+		// flatten. The demoted-thinking separator lives on the demoted block
+		// itself (transform-messages), so it targets that boundary only.
+		expect(assistant.content).toBe("hello world");
 	});

 	it("prepends thinking text to string assistant content when requiresThinkingAsText is set", () => {
@@ -1370,7 +1367,7 @@ describe("kimi model detection via detectCompat", () => {
 		const payload = (await promise) as { messages: Array<Record<string, unknown>> };
 		const assistant = payload.messages.find(m => m.role === "assistant");
 		expect(assistant).toBeDefined();
-		expect(assistant?.content).toBe(renderDemotedThinking(model.id, "Need to preserve cross-api reasoning."));
+		expect(assistant?.content).toBe(`${renderDemotedThinking(model.id, "Need to preserve cross-api reasoning.")}\n`);
 		expect(assistant?.reasoning_content).toBe("");
 		expect(assistant?.reasoning).toBeUndefined();
 		expect(assistant?.reasoning_text).toBeUndefined();
@@ -100,7 +100,7 @@ describe("transformMessages cross-provider thinking demotion → canonical diale
 		expect(first?.type).toBe("text");
 		// Demoted reasoning is wrapped in Gemini's canonical thinking fence.
 		expect(first && first.type === "text" ? first.text : "").toBe(
-			getDialectDefinition("gemini").renderThinking(REASONING),
+			`${getDialectDefinition("gemini").renderThinking(REASONING)}\n`,
 		);
 		expect(first && first.type === "text" ? first.text : "").toContain("```thinking");
 		// The original reply text survives as its own block, after the fence.
@@ -120,7 +120,7 @@ describe("transformMessages cross-provider thinking demotion → canonical diale
 		const first = assistant.content[0];
 		expect(first?.type).toBe("text");
 		const text = first && first.type === "text" ? first.text : "";
-		expect(text).toBe(renderDemotedThinking("gpt-5", REASONING));
+		expect(text).toBe(`${renderDemotedThinking("gpt-5", REASONING)}\n`);
 		// No harmony chat-template control tokens leaked, and the unsafe native
 		// renderThinking output was explicitly NOT used.
 		expect(text).not.toContain("<|");
@@ -163,8 +163,8 @@ describe("transformMessages cross-provider thinking demotion → canonical diale
 			const first = assistant.content[0];
 			expect(first?.type).toBe("text");
 			const text = first && first.type === "text" ? first.text : "";
-			expect(text).toBe(REASONING);
-			expect(text).toBe(renderDemotedThinking(target.id, REASONING));
+			expect(text).toBe(`${REASONING}\n`);
+			expect(text).toBe(`${renderDemotedThinking(target.id, REASONING)}\n`);
 			expect(text).not.toContain("<thinking>");
 			expect(text).not.toContain("</thinking>");
 			expect(text).not.toContain("<think>");