From 1913d354e17f972b8649013155ba70e62394c9a7 Mon Sep 17 00:00:00 2001 From: roboomp Date: Fri, 3 Jul 2026 20:54:35 +0000 Subject: [PATCH] fix(ai): route Bedrock dotted Claude profiles to Anthropic dialect and separate flattened bare demoted-thinking blocks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two defense-in-depth follow-ups to the Anthropic-dialect demotion fix flagged by the Codex reviewer on #4432: 1. Extend isClaudeModelId's regex from `(^|/)claude[-.]` to `(^|[/.])claude[-.]` so Bedrock cross-region inference profiles (us.anthropic.claude-…, eu.anthropic.claude-…, global.anthropic.claude-…, au.anthropic.claude-…) classify as Claude. parseAnthropicModel only enumerates opus/sonnet/fable/mythos, so a Haiku Bedrock profile whose kind isn't in the parser regex would otherwise slip through modelFamilyToken's fallback and fall through preferredDialect to XML, still emitting … on prior-turn demotion. 2. Join adjacent text blocks with \n (was "") when flattening assistant content in convertOpenAICompletionsMessages. Anthropic-dialect renderDemotedThinking returns bare prose with no self-terminator, so a demoted-reasoning text block followed by a visible-answer text block used to concatenate as "reasoningfinal answer". Streaming accumulates continuous prose into a single block, so multi-block content represents semantically distinct segments; the paragraph-break join is the right shape. Test coverage: identity-family adds dotted-prefix cases for isClaudeModelId and modelFamilyToken; transform-messages-thinking-dialect extends the Claude-target sweep with Bedrock profile ids; issue-3434/3528 repro tests updated to expect the newline-separated flatten shape. Refs #4430 --- packages/ai/CHANGELOG.md | 2 +- .../ai/src/providers/openai-completions.ts | 6 ++++- packages/ai/test/issue-3434-repro.test.ts | 8 +++---- packages/ai/test/issue-3528-repro.test.ts | 2 +- .../ai/test/openai-completions-compat.test.ts | 9 +++++++- ...ransform-messages-thinking-dialect.test.ts | 6 +++++ packages/catalog/src/identity/family.ts | 11 +++++++-- packages/catalog/test/identity-family.test.ts | 23 +++++++++++++++++++ 8 files changed, 57 insertions(+), 10 deletions(-) diff --git a/packages/ai/CHANGELOG.md b/packages/ai/CHANGELOG.md index dbbf0685b..fb173c7ad 100644 --- a/packages/ai/CHANGELOG.md +++ b/packages/ai/CHANGELOG.md @@ -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 `` tags that Anthropic's reasoning_extraction classifier flags as visible reasoning leakage ([#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 `` 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)). ## [16.3.4] - 2026-07-03 diff --git a/packages/ai/src/providers/openai-completions.ts b/packages/ai/src/providers/openai-completions.ts index 2776d05a4..a52fbd61c 100644 --- a/packages/ai/src/providers/openai-completions.ts +++ b/packages/ai/src/providers/openai-completions.ts @@ -1778,7 +1778,11 @@ 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. - assistantMsg.content = nonEmptyTextBlocks.map(b => b.text.toWellFormed()).join(""); + // 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"); } // Handle thinking blocks diff --git a/packages/ai/test/issue-3434-repro.test.ts b/packages/ai/test/issue-3434-repro.test.ts index 29c55130b..d6d16f8b9 100644 --- a/packages/ai/test/issue-3434-repro.test.ts +++ b/packages/ai/test/issue-3434-repro.test.ts @@ -194,7 +194,7 @@ describe("cross-API thinking-block preservation (#3433/#3434)", () => { if (!assistant) throw new Error("assistant message missing"); expect(assistant.reasoning_content).toBe(""); - expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}Done.`); + expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}\nDone.`); }); it("demotes thinking to canonical text when the target cannot replay it semantically", () => { @@ -211,7 +211,7 @@ describe("cross-API thinking-block preservation (#3433/#3434)", () => { if (!assistant) throw new Error("assistant message missing"); expect(assistant.reasoning_content).toBeUndefined(); - expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Explore the repo, then patch it.")}Done.`); + expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Explore the repo, then patch it.")}\nDone.`); }); it("demotes cross-API thinking for OpenCode reasoning targets with whenThinking schema", () => { @@ -233,7 +233,7 @@ describe("cross-API thinking-block preservation (#3433/#3434)", () => { if (!assistant) throw new Error("assistant message missing"); expect(assistant.reasoning_content).toBeUndefined(); - expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}Done.`); + expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}\nDone.`); }); it("demotes prior thinking to content when the OpenCode base compat runs with thinking off", () => { @@ -253,7 +253,7 @@ describe("cross-API thinking-block preservation (#3433/#3434)", () => { if (!assistant) throw new Error("assistant message missing"); expect(assistant.reasoning_content).toBeUndefined(); - expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}Done.`); + expect(assistant.content).toBe(`${renderDemotedThinking(target.id, "Read README and answer.")}\nDone.`); }); it("does not promote markup-healed same-model thinking into visible content", () => { diff --git a/packages/ai/test/issue-3528-repro.test.ts b/packages/ai/test/issue-3528-repro.test.ts index 77ebeae09..7cdbd5758 100644 --- a/packages/ai/test/issue-3528-repro.test.ts +++ b/packages/ai/test/issue-3528-repro.test.ts @@ -284,7 +284,7 @@ describe("llama.cpp warm-prefix preservation (#3528)", () => { const found = findAssistantMessage(wire) as Record | undefined; expect(found?.reasoning_content).toBeUndefined(); expect(found?.content).toBe( - `${renderDemotedThinking(target.id, "Cross-vendor reasoning chain that must survive the switch.")}Switched-in answer.`, + `${renderDemotedThinking(target.id, "Cross-vendor reasoning chain that must survive the switch.")}\nSwitched-in answer.`, ); expect("EvAnthropicOpaqueContinuationBlob==" in (found ?? {})).toBe(false); }); diff --git a/packages/ai/test/openai-completions-compat.test.ts b/packages/ai/test/openai-completions-compat.test.ts index 35bb27b00..3cd69f3de 100644 --- a/packages/ai/test/openai-completions-compat.test.ts +++ b/packages/ai/test/openai-completions-compat.test.ts @@ -233,7 +233,14 @@ describe("openai-completions compatibility", () => { throw new Error("assistant message missing"); } expect(typeof assistant.content).toBe("string"); - expect(assistant.content).toBe("hello world"); + // 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"); }); it("prepends thinking text to string assistant content when requiresThinkingAsText is set", () => { diff --git a/packages/ai/test/transform-messages-thinking-dialect.test.ts b/packages/ai/test/transform-messages-thinking-dialect.test.ts index b07a4b482..6b956912e 100644 --- a/packages/ai/test/transform-messages-thinking-dialect.test.ts +++ b/packages/ai/test/transform-messages-thinking-dialect.test.ts @@ -143,6 +143,12 @@ describe("transformMessages cross-provider thinking demotion → canonical diale { name: "Claude Haiku", id: "claude-haiku-4-5" }, { name: "Claude Fable", id: "claude-fable-5" }, { name: "Claude Mythos", id: "claude-mythos-5" }, + // Bedrock cross-region inference profiles. `parseAnthropicModel` + // doesn't enumerate `haiku`, so this exercises the isClaudeModelId + // dotted-prefix fallback specifically. + { name: "Claude Haiku (Bedrock US)", id: "us.anthropic.claude-haiku-4-5-20251001-v1:0" }, + { name: "Claude Haiku (Bedrock EU)", id: "eu.anthropic.claude-haiku-4-5-20251001-v1:0" }, + { name: "Claude Opus (Bedrock Global)", id: "global.anthropic.claude-opus-4-8" }, ] as const; for (const target of targets) { diff --git a/packages/catalog/src/identity/family.ts b/packages/catalog/src/identity/family.ts index 2043cef5b..f68f65779 100644 --- a/packages/catalog/src/identity/family.ts +++ b/packages/catalog/src/identity/family.ts @@ -41,9 +41,16 @@ export const isKimiK26ModelId = memo((modelId: string): boolean => { return /(^|\/)kimi-k2(?:\.6|p6)(?:[-:]|$)/i.test(modelId); }); -/** Claude ids in any namespace form (`claude-*`, `vendor/claude.x`). */ +/** + * Claude ids in any namespace form: bare (`claude-*`), path-namespaced + * (`anthropic/claude.x`), or dot-prefixed (`us.anthropic.claude-…`, + * `global.anthropic.claude-…`, `au.anthropic.claude-…` — Bedrock cross-region + * inference profiles). Necessary because {@link parseAnthropicModel} only + * classifies kinds enumerated in its regex, so any dotted profile whose kind + * (e.g. `haiku`) is not enumerated would otherwise slip past this fallback. + */ export const isClaudeModelId = memo((modelId: string): boolean => { - return /(^|\/)claude[-.]/i.test(modelId); + return /(^|[/.])claude[-.]/i.test(modelId); }); /** `anthropic/`-namespaced ids (aggregator catalogs like OpenRouter). */ diff --git a/packages/catalog/test/identity-family.test.ts b/packages/catalog/test/identity-family.test.ts index 7d8fd4095..bc62cf6cd 100644 --- a/packages/catalog/test/identity-family.test.ts +++ b/packages/catalog/test/identity-family.test.ts @@ -43,6 +43,21 @@ describe("isClaudeModelId", () => { expect(isClaudeModelId("anthropic/claude.3")).toBe(true); expect(isClaudeModelId("my-claudius")).toBe(false); }); + test("matches dotted Bedrock cross-region inference profile ids for Claude kinds not enumerated in parseAnthropicModel", () => { + // `parseAnthropicModel` only classifies opus/sonnet/fable/mythos, so a + // Haiku Bedrock profile (`us.anthropic.claude-haiku-…`) slips past its + // regex and MUST still classify as Claude via this fallback so + // `modelFamilyToken`/`preferredDialect` route it to the Anthropic + // dialect instead of falling through to XML. + expect(isClaudeModelId("us.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe(true); + expect(isClaudeModelId("eu.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe(true); + expect(isClaudeModelId("global.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe(true); + expect(isClaudeModelId("au.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe(true); + // Non-Claude names that happen to contain "claude" as a substring + // stay unmatched — no `.` / `/` / start delimiter before `claude-`. + expect(isClaudeModelId("subclaudian")).toBe(false); + expect(isClaudeModelId("claudius-5")).toBe(false); + }); }); describe("supportsAdaptiveThinkingDisplay", () => { @@ -211,6 +226,14 @@ describe("modelFamilyToken", () => { expect(modelFamilyToken("openrouter/anthropic/claude-opus-4-8")).toBe("anthropic"); }); + test("classifies Bedrock cross-region profile ids for Claude kinds not enumerated in parseAnthropicModel", () => { + // `parseAnthropicModel` doesn't know `haiku`, so this exercises the + // isClaudeModelId fallback specifically for dotted Bedrock profiles. + expect(modelFamilyToken("us.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe("anthropic"); + expect(modelFamilyToken("eu.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe("anthropic"); + expect(modelFamilyToken("global.anthropic.claude-haiku-4-5-20251001-v1:0")).toBe("anthropic"); + }); + test("classifies non-first-party families", () => { expect(modelFamilyToken("moonshotai/kimi-k2")).toBe("kimi"); expect(modelFamilyToken("qwen/qwen3-coder")).toBe("qwen");