From 50ab63de3d3cfc2cdefd18659e284b7c79083840 Mon Sep 17 00:00:00 2001 From: kael-odin Date: Wed, 8 Jul 2026 09:12:03 +0800 Subject: [PATCH] fix(anthropic): apply thinking config on override path, symmetric with openai MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AnthropicAdapter.Stream built thinking config (buildAnthropicThinkingConfig) and wrote it into body only inside the `if len(body)==0` normal-construction block. The RequestBodyOverride branch skipped it entirely — a disabled effort on the override path left whatever thinking config the override body carried, violating user intent and diverging from openai.go, where applyOpenAIThinkingDisable runs unconditionally after both branches. Also, on the normal path, disabled only wrote thinking:{type:disabled} but left a stale output_config (set by a prior adaptive turn or by AnthropicExtraParams) in place — an explicitly disabled request could still carry output_config.effort=high, a contradictory payload. Fix: - Extract applyAnthropicThinkingConfig(body, req), called unconditionally after the override/normal block, mirroring openai.go:1860. - disabled: force thinking:{type:"disabled"}, delete output_config, set thinking_disabled_provider_param=thinking.type knob. - adaptive (AnthropicThinkingEffort non-empty): write thinking:{type:adaptive,display:summarized} + output_config. - empty effort: no-op. - buildAnthropicThinkingConfig retained for stable-message-count / message normalization signals inside the normal block. Tests: anthropic_thinking_disable_test.go covers all three branches, alias normalization (off→disabled), disabled overriding existing adaptive config, and the override-path symmetry scenario. --- internal/backend/agent/model/anthropic.go | 39 ++++- .../model/anthropic_thinking_disable_test.go | 141 ++++++++++++++++++ 2 files changed, 174 insertions(+), 6 deletions(-) create mode 100644 internal/backend/agent/model/anthropic_thinking_disable_test.go diff --git a/internal/backend/agent/model/anthropic.go b/internal/backend/agent/model/anthropic.go index 7906539..44906fc 100644 --- a/internal/backend/agent/model/anthropic.go +++ b/internal/backend/agent/model/anthropic.go @@ -261,18 +261,17 @@ func (adapter *AnthropicAdapter) Stream(ctx context.Context, req StreamRequest, body["tools"] = tools } body["system"] = anthropicProviderSystemBlocks(systemParts) - if thinkingConfig != nil { - body["thinking"] = thinkingConfig - if normalizeRuntimeThinkingEffort(req.ThinkingEffort) != "disabled" { - body["output_config"] = buildAnthropicOutputConfig(req) - } - } frontier := buildAnthropicCacheFrontier(body, stableMessageCount) req.RequestKnobs = annotateAnthropicRequestKnobs(req.RequestKnobs, body, frontier) body = cloneRequestBodyOverride(body) applyAnthropicCacheBreakpoints(body, frontier.BreakpointPositions) frontier.BreakpointCount = len(frontier.BreakpointPositions) } + // applyAnthropicThinkingConfig 在 override 块之外无条件调用，确保 RequestBodyOverride + // 路径与正常构造路径行为一致：disabled 时强制 thinking:{type:disabled} 并清理冲突字段， + // 非 disabled 时按 AnthropicThinkingEffort 写 adaptive 配置。与 openai.go 的 + // applyOpenAIThinkingDisable 对称——后者也是无条件在两条路径之后调用。 + applyAnthropicThinkingConfig(body, req) if err := ApplyAnthropicExtraParams(body, req.AnthropicExtraParamsEnabled, req.AnthropicExtraParamsJSON); err != nil { finishedAt = time.Now().UTC() recordLLMSummaryArtifact(req, buildLLMSummaryPayload(req, "anthropic", modelID, startedAt, time.Time{}, finishedAt, "", 0, 0, 0, 0, err)) @@ -1367,6 +1366,34 @@ func buildAnthropicThinkingConfig(req StreamRequest) map[string]any { } } +// applyAnthropicThinkingConfig 在请求体构造完成后（含 RequestBodyOverride 路径）无条件调用， +// 与 openai.go 的 applyOpenAIThinkingDisable 对称。它把 thinking 配置写入 body 并在 disabled +// 时清理与之冲突的字段，确保两条构造路径行为一致： +// - disabled: 强制 thinking:{type:"disabled"}，删除 output_config / 残留 thinking adaptive 配置， +// 记录 thinking_disabled_provider_param=thinking.type knob +// - adaptive: 按 AnthropicThinkingEffort 写 thinking:{type:adaptive,display:summarized} + output_config +// +// 在 override 路径下，上层若已在 override body 里塞了 thinking/output_config，disabled 时会被正确覆盖。 +func applyAnthropicThinkingConfig(body map[string]any, req StreamRequest) { + if len(body) == 0 { + return + } + if normalizeRuntimeThinkingEffort(req.ThinkingEffort) != "disabled" { + if strings.TrimSpace(req.AnthropicThinkingEffort) == "" { + return + } + body["thinking"] = map[string]any{ + "type": "adaptive", + "display": "summarized", + } + body["output_config"] = buildAnthropicOutputConfig(req) + return + } + body["thinking"] = map[string]any{"type": "disabled"} + delete(body, "output_config") + setRequestKnob(req, "thinking_disabled_provider_param", "thinking.type") +} + func buildAnthropicOutputConfig(req StreamRequest) map[string]any { return map[string]any{ "effort": anthropicThinkingEffort(req), diff --git a/internal/backend/agent/model/anthropic_thinking_disable_test.go b/internal/backend/agent/model/anthropic_thinking_disable_test.go new file mode 100644 index 0000000..e2517d9 --- /dev/null +++ b/internal/backend/agent/model/anthropic_thinking_disable_test.go @@ -0,0 +1,141 @@ +package modeladapter + +import ( + "testing" +) + +// TestApplyAnthropicThinkingConfig covers the three effort branches on a body +// built by the normal construction path, plus the override path symmetry that +// was the original bug (thinking config was only applied inside the +// `if len(body)==0` block, so RequestBodyOverride skipped it entirely). +func TestApplyAnthropicThinkingConfig(t *testing.T) { + tests := []struct { + name string + body map[string]any + thinkingEffort string + adaptiveEffort string + wantThinking any + wantOutputCfg bool // expect output_config key present + }{ + { + name: "disabled_writes_disabled_and_drops_output_config", + body: map[string]any{"model": "m", "output_config": map[string]any{"effort": "high"}}, + thinkingEffort: "disabled", + wantThinking: map[string]any{"type": "disabled"}, + wantOutputCfg: false, + }, + { + name: "adaptive_writes_adaptive_and_output_config", + body: map[string]any{"model": "m"}, + adaptiveEffort: "high", + wantThinking: map[string]any{"type": "adaptive", "display": "summarized"}, + wantOutputCfg: true, + }, + { + name: "empty_effort_no_op", + body: map[string]any{"model": "m"}, + wantThinking: nil, + wantOutputCfg: false, + }, + { + name: "disabled_overrides_existing_adaptive_thinking", + body: map[string]any{"thinking": map[string]any{"type": "adaptive", "display": "summarized"}, "output_config": map[string]any{"effort": "high"}}, + thinkingEffort: "disabled", + wantThinking: map[string]any{"type": "disabled"}, + wantOutputCfg: false, + }, + { + name: "disabled_normalizes_aliases", + body: map[string]any{"model": "m"}, + thinkingEffort: "off", + wantThinking: map[string]any{"type": "disabled"}, + wantOutputCfg: false, + }, + } + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + req := StreamRequest{ + ThinkingEffort: tc.thinkingEffort, + AnthropicThinkingEffort: tc.adaptiveEffort, + RequestKnobs: map[string]any{}, + } + applyAnthropicThinkingConfig(tc.body, req) + + gotThinking, hasThinking := tc.body["thinking"] + if tc.wantThinking == nil { + if hasThinking { + t.Fatalf("expected no thinking key, got %v", gotThinking) + } + } else { + if !hasThinking { + t.Fatalf("expected thinking=%v, key absent", tc.wantThinking) + } + if !mapEqual(gotThinking, tc.wantThinking) { + t.Fatalf("thinking mismatch\nwant: %v\ngot: %v", tc.wantThinking, gotThinking) + } + } + + _, hasOutputCfg := tc.body["output_config"] + if hasOutputCfg != tc.wantOutputCfg { + t.Fatalf("output_config presence=%v, want %v", hasOutputCfg, tc.wantOutputCfg) + } + + if tc.thinkingEffort == "disabled" || tc.thinkingEffort == "off" { + gotKnob := req.RequestKnobs["thinking_disabled_provider_param"] + if gotKnob != "thinking.type" { + t.Fatalf("knob thinking_disabled_provider_param=%v, want thinking.type", gotKnob) + } + } + }) + } +} + +// TestApplyAnthropicThinkingConfigOverridePath simulates the RequestBodyOverride +// branch: the adapter receives a body that did NOT go through the normal +// construction (no tools/messages/system written by the adapter). Before the +// fix, thinking config was skipped entirely on this path. Now +// applyAnthropicThinkingConfig runs unconditionally and disables thinking. +func TestApplyAnthropicThinkingConfigOverridePath(t *testing.T) { + overrideBody := map[string]any{ + "model": "claude-x", + "messages": []any{map[string]any{"role": "user", "content": "hi"}}, + "thinking": map[string]any{"type": "adaptive", "display": "summarized"}, + "output_config": map[string]any{"effort": "high"}, + "stream": true, + } + req := StreamRequest{ + ThinkingEffort: "disabled", + RequestKnobs: map[string]any{}, + } + + applyAnthropicThinkingConfig(overrideBody, req) + + gotThinking := overrideBody["thinking"] + if !mapEqual(gotThinking, map[string]any{"type": "disabled"}) { + t.Fatalf("override path: thinking not forced to disabled, got %v", gotThinking) + } + if _, hasOutputCfg := overrideBody["output_config"]; hasOutputCfg { + t.Fatalf("override path: output_config should be dropped on disabled, still present") + } + if gotKnob := req.RequestKnobs["thinking_disabled_provider_param"]; gotKnob != "thinking.type" { + t.Fatalf("override path: knob=%v, want thinking.type", gotKnob) + } +} + +func mapEqual(a, b any) bool { + ma, okA := a.(map[string]any) + mb, okB := b.(map[string]any) + if !okA || !okB { + return a == b + } + if len(ma) != len(mb) { + return false + } + for k, va := range ma { + vb, ok := mb[k] + if !ok || !mapEqual(va, vb) { + return false + } + } + return true +}