From 3b23ef3d4b37b5ee4c94a4a2eba1f6b59b49a478 Mon Sep 17 00:00:00 2001 From: Nils Reeh Date: Sat, 18 Apr 2026 00:47:50 +0200 Subject: [PATCH] fix(pi-ai): wire thinking:{type} field and extend adaptive-thinking model coverage (#4392) Co-Authored-By: Claude Sonnet 4.6 (cherry picked from commit 503e79070d198254661febad35a267ead487b7e1) --- .../src/providers/amazon-bedrock.test.ts | 172 ++++++++++++++++++ .../pi-ai/src/providers/amazon-bedrock.ts | 25 ++- .../pi-ai/src/providers/anthropic-shared.ts | 6 +- .../claude-code-cli/stream-adapter.ts | 56 +++++- .../tests/stream-adapter.test.ts | 54 ++++++ 5 files changed, 301 insertions(+), 12 deletions(-) create mode 100644 packages/pi-ai/src/providers/amazon-bedrock.test.ts diff --git a/packages/pi-ai/src/providers/amazon-bedrock.test.ts b/packages/pi-ai/src/providers/amazon-bedrock.test.ts new file mode 100644 index 000000000..d00a49cdd --- /dev/null +++ b/packages/pi-ai/src/providers/amazon-bedrock.test.ts @@ -0,0 +1,172 @@ +/** + * TDD Red Phase — Bug #4392 / Pre-existing Bug #4352 + * + * `supportsAdaptiveThinking()` in amazon-bedrock.ts is missing opus-4-7, + * sonnet-4-7, and haiku-4-5. These tests FAIL until the bug is fixed. + * + * Related: #4392 (opus-4-7 adaptive thinking not recognised on Bedrock) + * #4352 (pre-existing: only opus-4-6 / sonnet-4-6 whitelisted) + */ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +import { + supportsAdaptiveThinking, + mapThinkingLevelToEffort, + buildAdditionalModelRequestFields, + type BedrockOptions, +} from "./amazon-bedrock.js"; + +import type { Model } from "../types.js"; + +// --------------------------------------------------------------------------- +// Helpers +// --------------------------------------------------------------------------- + +function makeModel(id: string): Model<"bedrock-converse-stream"> { + return { + id, + name: id, + api: "bedrock-converse-stream", + provider: "amazon-bedrock" as any, + baseUrl: "", + reasoning: true, + input: ["text"], + cost: { input: 0, output: 0, cacheRead: 0, cacheWrite: 0 }, + contextWindow: 200000, + maxTokens: 32000, + }; +} + +// --------------------------------------------------------------------------- +// supportsAdaptiveThinking — RED tests (#4392 / #4352) +// --------------------------------------------------------------------------- + +describe("supportsAdaptiveThinking — Bug #4392 / #4352: missing models", () => { + // These two already pass (regression guard): + it("returns true for opus-4-6 (hyphen, Bedrock ARN style)", () => { + assert.ok(supportsAdaptiveThinking("anthropic.claude-opus-4-6-20250514-v1:0")); + }); + + it("returns true for sonnet-4-6 (hyphen)", () => { + assert.ok(supportsAdaptiveThinking("anthropic.claude-sonnet-4-6-20250514-v1:0")); + }); + + // --- RED: the following FAIL because opus-4-7 / sonnet-4-7 / haiku-4-5 are missing --- + + it("[#4392] returns true for opus-4-7 (hyphen, Bedrock ARN style)", () => { + // FAILS: supportsAdaptiveThinking does not include 'opus-4-7' + assert.ok( + supportsAdaptiveThinking("anthropic.claude-opus-4-7-20250514-v1:0"), + "opus-4-7 should support adaptive thinking (bug #4392)", + ); + }); + + it("[#4392] returns true for opus-4-7 (dot separator)", () => { + // FAILS: supportsAdaptiveThinking does not include 'opus-4.7' + assert.ok( + supportsAdaptiveThinking("anthropic.claude-opus-4.7-20250514-v1:0"), + "opus-4.7 (dot) should support adaptive thinking (bug #4392)", + ); + }); + + it("[#4352] returns true for sonnet-4-7 (hyphen)", () => { + // FAILS: supportsAdaptiveThinking does not include 'sonnet-4-7' + assert.ok( + supportsAdaptiveThinking("anthropic.claude-sonnet-4-7-20250514-v1:0"), + "sonnet-4-7 should support adaptive thinking (bug #4352)", + ); + }); + + it("[#4352] returns true for haiku-4-5 (hyphen)", () => { + // FAILS: supportsAdaptiveThinking does not include 'haiku-4-5' + assert.ok( + supportsAdaptiveThinking("anthropic.claude-haiku-4-5-20250514-v1:0"), + "haiku-4-5 should support adaptive thinking (bug #4352)", + ); + }); +}); + +// --------------------------------------------------------------------------- +// buildAdditionalModelRequestFields — adaptive thinking output for opus-4-7 +// Tests go through the public API surface to validate end-to-end behaviour. +// --------------------------------------------------------------------------- + +describe("buildAdditionalModelRequestFields — Bug #4392: opus-4-7 must use adaptive thinking", () => { + const options: BedrockOptions = { reasoning: "high" }; + + it("[#4392] opus-4-7 Bedrock ARN → thinking.type === 'adaptive' (not budget_tokens)", () => { + const model = makeModel("anthropic.claude-opus-4-7-20250514-v1:0"); + const fields = buildAdditionalModelRequestFields(model, options); + // FAILS: because supportsAdaptiveThinking returns false for opus-4-7, + // the function returns { thinking: { type: "enabled", budget_tokens: ... } } + assert.equal( + fields?.thinking?.type, + "adaptive", + "opus-4-7 should produce thinking.type='adaptive', not budget_tokens", + ); + }); + + it("[#4392] opus-4-7 dot separator → thinking.type === 'adaptive'", () => { + const model = makeModel("anthropic.claude-opus-4.7-20250514-v1:0"); + const fields = buildAdditionalModelRequestFields(model, options); + assert.equal( + fields?.thinking?.type, + "adaptive", + "opus-4.7 (dot) should produce thinking.type='adaptive'", + ); + }); + + it("[#4352] sonnet-4-7 → thinking.type === 'adaptive'", () => { + const model = makeModel("anthropic.claude-sonnet-4-7-20250514-v1:0"); + const fields = buildAdditionalModelRequestFields(model, options); + assert.equal( + fields?.thinking?.type, + "adaptive", + "sonnet-4-7 should produce thinking.type='adaptive'", + ); + }); + + it("[#4352] haiku-4-5 → thinking.type === 'adaptive'", () => { + const model = makeModel("anthropic.claude-haiku-4-5-20250514-v1:0"); + const fields = buildAdditionalModelRequestFields(model, options); + assert.equal( + fields?.thinking?.type, + "adaptive", + "haiku-4-5 should produce thinking.type='adaptive'", + ); + }); +}); + +// --------------------------------------------------------------------------- +// mapThinkingLevelToEffort — RED test for xhigh on opus-4-7 +// The Bedrock version returns "max" (dead code path at line 411), whereas +// the correct value is "xhigh" (as implemented in anthropic-shared.ts). +// --------------------------------------------------------------------------- + +describe("mapThinkingLevelToEffort — Bug #4392: opus-4-7 xhigh should return 'xhigh' not 'max'", () => { + it("[#4392] maps xhigh → 'xhigh' for opus-4-7 (native xhigh support)", () => { + // FAILS: current code returns "max" for opus-4-7 at line 411, + // and in any case this code path is unreachable because + // supportsAdaptiveThinking returns false for opus-4-7. + // After the fix, supportsAdaptiveThinking will return true AND + // mapThinkingLevelToEffort must return "xhigh" (not "max"). + const result = mapThinkingLevelToEffort("xhigh", "anthropic.claude-opus-4-7-20250514-v1:0"); + assert.equal( + result, + "xhigh", + "opus-4-7 supports native xhigh effort — must not be clamped to 'max'", + ); + }); + + it("[#4392] maps xhigh → 'max' for opus-4-6 (no native xhigh, clamped)", () => { + // This already passes — regression guard. + const result = mapThinkingLevelToEffort("xhigh", "anthropic.claude-opus-4-6-20250514-v1:0"); + assert.equal(result, "max"); + }); + + it("maps high → 'high' for opus-4-7 (not affected by bug)", () => { + const result = mapThinkingLevelToEffort("high", "anthropic.claude-opus-4-7-20250514-v1:0"); + assert.equal(result, "high"); + }); +}); diff --git a/packages/pi-ai/src/providers/amazon-bedrock.ts b/packages/pi-ai/src/providers/amazon-bedrock.ts index dee0c363e..78f076e68 100644 --- a/packages/pi-ai/src/providers/amazon-bedrock.ts +++ b/packages/pi-ai/src/providers/amazon-bedrock.ts @@ -383,21 +383,29 @@ function handleContentBlockStop( } /** - * Check if the model supports adaptive thinking (Opus 4.6 and Sonnet 4.6). + * Check if the model supports adaptive thinking (Opus 4.6/4.7, Sonnet 4.6/4.7, Haiku 4.5). + * @internal exported for testing only */ -function supportsAdaptiveThinking(modelId: string): boolean { +export function supportsAdaptiveThinking(modelId: string): boolean { return ( modelId.includes("opus-4-6") || modelId.includes("opus-4.6") || + modelId.includes("opus-4-7") || + modelId.includes("opus-4.7") || modelId.includes("sonnet-4-6") || - modelId.includes("sonnet-4.6") + modelId.includes("sonnet-4.6") || + modelId.includes("sonnet-4-7") || + modelId.includes("sonnet-4.7") || + modelId.includes("haiku-4-5") || + modelId.includes("haiku-4.5") ); } -function mapThinkingLevelToEffort( +/** @internal exported for testing only */ +export function mapThinkingLevelToEffort( level: SimpleStreamOptions["reasoning"], modelId: string, -): "low" | "medium" | "high" | "max" { +): "low" | "medium" | "high" | "xhigh" | "max" { switch (level) { case "minimal": case "low": @@ -407,7 +415,9 @@ function mapThinkingLevelToEffort( case "high": return "high"; case "xhigh": - return modelId.includes("opus-4-6") || modelId.includes("opus-4.6") ? "max" : "high"; + if (modelId.includes("opus-4-7") || modelId.includes("opus-4.7")) return "xhigh"; + if (modelId.includes("opus-4-6") || modelId.includes("opus-4.6")) return "max"; + return "high"; default: return "high"; } @@ -688,7 +698,8 @@ function mapStopReason(reason: string | undefined): StopReason { } } -function buildAdditionalModelRequestFields( +/** @internal exported for testing only */ +export function buildAdditionalModelRequestFields( model: Model<"bedrock-converse-stream">, options: BedrockOptions, ): Record | undefined { diff --git a/packages/pi-ai/src/providers/anthropic-shared.ts b/packages/pi-ai/src/providers/anthropic-shared.ts index 567609147..187ebedf6 100644 --- a/packages/pi-ai/src/providers/anthropic-shared.ts +++ b/packages/pi-ai/src/providers/anthropic-shared.ts @@ -153,7 +153,11 @@ export function supportsAdaptiveThinking(modelId: string): boolean { modelId.includes("opus-4-6") || modelId.includes("opus-4.6") || modelId.includes("sonnet-4-6") || - modelId.includes("sonnet-4.6") + modelId.includes("sonnet-4.6") || + modelId.includes("sonnet-4-7") || + modelId.includes("sonnet-4.7") || + modelId.includes("haiku-4-5") || + modelId.includes("haiku-4.5") ); } diff --git a/src/resources/extensions/claude-code-cli/stream-adapter.ts b/src/resources/extensions/claude-code-cli/stream-adapter.ts index a22b33aff..211902f8b 100644 --- a/src/resources/extensions/claude-code-cli/stream-adapter.ts +++ b/src/resources/extensions/claude-code-cli/stream-adapter.ts @@ -18,7 +18,7 @@ import type { ToolCall, } from "@singularity-forge/pi-ai"; import type { ExtensionUIContext } from "@singularity-forge/pi-coding-agent"; -import { EventStream, mapThinkingLevelToEffort, supportsAdaptiveThinking } from "@singularity-forge/pi-ai"; +import { EventStream } from "@singularity-forge/pi-ai"; import { execSync } from "node:child_process"; import { PartialMessageBuilder, ZERO_USAGE, mapUsage } from "./partial-builder.js"; import { buildWorkflowMcpServers } from "../sf/workflow-mcp.js"; @@ -679,6 +679,42 @@ export async function resolveClaudePermissionMode( return "bypassPermissions"; } +// NOTE: These helpers intentionally mirror @singularity-forge/pi-ai anthropic-shared +// behavior so this extension remains typecheck-stable even when the published +// @singularity-forge/pi-ai barrel lags behind monorepo source exports. +function modelSupportsAdaptiveThinking(modelId: string): boolean { + return ( + modelId.includes("opus-4-6") + || modelId.includes("opus-4.6") + || modelId.includes("opus-4-7") + || modelId.includes("opus-4.7") + || modelId.includes("sonnet-4-6") + || modelId.includes("sonnet-4.6") + || modelId.includes("sonnet-4-7") + || modelId.includes("sonnet-4.7") + || modelId.includes("haiku-4-5") + || modelId.includes("haiku-4.5") + ); +} + +function mapThinkingLevelToAnthropicEffort(level: ThinkingLevel | undefined, modelId: string): "low" | "medium" | "high" | "xhigh" | "max" { + switch (level) { + case "minimal": + case "low": + return "low"; + case "medium": + return "medium"; + case "high": + return "high"; + case "xhigh": + if (modelId.includes("opus-4-7") || modelId.includes("opus-4.7")) return "xhigh"; + if (modelId.includes("opus-4-6") || modelId.includes("opus-4.6")) return "max"; + return "high"; + default: + return "high"; + } +} + /** * Build the options object passed to the Claude Agent SDK's `query()` call. * @@ -715,10 +751,21 @@ export function buildSdkOptions( "Bash(pwd)", ...(mcpServers ? Object.keys(mcpServers).map((serverName) => `mcp__${serverName}__*`) : []), ]; + const supportsAdaptive = modelSupportsAdaptiveThinking(modelId); const effort = - reasoning && supportsAdaptiveThinking(modelId) - ? mapThinkingLevelToEffort(reasoning, modelId) + reasoning && supportsAdaptive + ? mapThinkingLevelToAnthropicEffort(reasoning, modelId) : undefined; + + // Bug B: SDK requires thinking:{type:"adaptive"} alongside effort for adaptive thinking to activate. + // Bug C: SDK requires thinking:{type:"disabled"} to actually stop adaptive thinking when reasoning is off; + // omitting the field leaves the SDK in its adaptive default (or persisted session state). + const thinkingConfig = supportsAdaptive + ? effort + ? { thinking: { type: "adaptive" } } + : { thinking: { type: "disabled" } } + : undefined; + return { pathToClaudeCodeExecutable: getClaudePath(), model: modelId, @@ -732,7 +779,8 @@ export function buildSdkOptions( disallowedTools, ...(allowedTools.length > 0 ? { allowedTools } : {}), ...(mcpServers ? { mcpServers } : {}), - betas: modelId.includes("sonnet") ? ["context-1m-2025-08-07"] : [], + betas: (modelId.includes("sonnet") || modelId.includes("opus-4-7") || modelId.includes("opus-4.7")) ? ["context-1m-2025-08-07"] : [], + ...(thinkingConfig ?? {}), ...(effort ? { effort } : {}), ...sdkExtraOptions, }; diff --git a/src/resources/extensions/claude-code-cli/tests/stream-adapter.test.ts b/src/resources/extensions/claude-code-cli/tests/stream-adapter.test.ts index b9943d78b..a702b559d 100644 --- a/src/resources/extensions/claude-code-cli/tests/stream-adapter.test.ts +++ b/src/resources/extensions/claude-code-cli/tests/stream-adapter.test.ts @@ -451,6 +451,60 @@ describe("stream-adapter — session persistence (#2859)", () => { assert.equal("effort" in options, false); }); + // --- Bug fixes #4392: thinking field & model coverage --- + + test("buildSdkOptions sets thinking disabled when reasoning is undefined on adaptive model (#4392)", () => { + // Bug C: thinkingLevel="off" means reasoning===undefined; SDK needs thinking:{type:"disabled"} + const options = buildSdkOptions("claude-sonnet-4-6", "test", undefined, {}); + assert.deepEqual( + (options as any).thinking, + { type: "disabled" }, + "thinking must be {type:'disabled'} when reasoning is undefined so SDK stops adaptive thinking", + ); + }); + + test("buildSdkOptions omits effort when reasoning is undefined (thinking disabled) (#4392)", () => { + // Bug C corollary: no effort when thinking is off + const options = buildSdkOptions("claude-sonnet-4-6", "test", undefined, {}); + assert.equal("effort" in options, false, "effort must not be set when reasoning is undefined"); + }); + + test("buildSdkOptions sets thinking adaptive when reasoning is provided (#4392)", () => { + // Bug B: when effort is set, thinking:{type:"adaptive"} must also be present + const options = buildSdkOptions("claude-opus-4-6", "test", undefined, { reasoning: "high" }); + assert.deepEqual( + (options as any).thinking, + { type: "adaptive" }, + "thinking must be {type:'adaptive'} alongside effort when reasoning is set", + ); + }); + + test("buildSdkOptions includes both effort and thinking.type=adaptive when reasoning is set (#4392)", () => { + // Bug B: both fields must be present together + const options = buildSdkOptions("claude-opus-4-6", "test", undefined, { reasoning: "high" }); + assert.equal(options.effort, "high", "effort must be set"); + assert.deepEqual((options as any).thinking, { type: "adaptive" }, "thinking must be adaptive"); + }); + + test("buildSdkOptions maps reasoning to effort for sonnet-4-7 (modelSupportsAdaptiveThinking #4392)", () => { + // Bug D: sonnet-4-7 was missing from modelSupportsAdaptiveThinking + const options = buildSdkOptions("claude-sonnet-4-7", "test", undefined, { reasoning: "high" }); + assert.equal(options.effort, "high", "sonnet-4-7 must support adaptive thinking and map effort"); + }); + + test("buildSdkOptions maps reasoning to effort for haiku-4-5 (modelSupportsAdaptiveThinking #4392)", () => { + // Bug D: haiku-4-5 was missing from modelSupportsAdaptiveThinking + const options = buildSdkOptions("claude-haiku-4-5", "test", undefined, { reasoning: "high" }); + assert.equal(options.effort, "high", "haiku-4-5 must support adaptive thinking and map effort"); + }); + + test("buildSdkOptions does not set thinking field for non-adaptive model when reasoning is undefined (#4392)", () => { + // Non-adaptive models (e.g. claude-sonnet-4-20250514) don't use the thinking API at all; + // no thinking field should be set when reasoning is undefined + const options = buildSdkOptions("claude-sonnet-4-20250514", "test", undefined, {}); + assert.equal("thinking" in options, false, "non-adaptive models must not receive a thinking field"); + }); + test("buildSdkOptions includes workflow MCP server config when env is set", () => { const prev = { SF_WORKFLOW_MCP_COMMAND: process.env.SF_WORKFLOW_MCP_COMMAND,