From 3f4888cc720fdb93f7894cdb21f6c2610286b01a Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 25 Jul 2026 14:13:35 +0200 Subject: [PATCH 1/2] fix(llm): OpenAI provider compatibility for caching, temperature, and tool calls MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three request-shape bugs surfaced when running against the real OpenAI API (DeepSeek tolerates all three, which is why they went unnoticed): 1. Anthropic prompt-caching shape sent to every provider. With prompt_caching enabled, the loop moved the system prompt into the Anthropic-only top-level "system" field and added cache_control markers; OpenAI rejects this with 400 unknown_parameter. Markers are now only applied when the endpoint is Anthropic (new Client.IsAnthropic). OpenAI and DeepSeek cache automatically. 2. temperature:0 sent to reasoning models. odek defaults to temperature 0 for determinism, but OpenAI reasoning models (o1/o3/o4, gpt-5 series) only accept the default (1) and answer 400 unsupported_value. The field is now omitted for those models; behavior for all other providers/models is unchanged. 3. Function tools + reasoning_effort rejected by gpt-5.6 models. gpt-5.6-luna/sol reject tools combined with any reasoning_effort other than "none" — and their default effort is not "none", so omitting the field still 400s. The client now learns the constraint from the 400 (atomic flag), retries once with effort "none", and pins it for subsequent tool-bearing calls; tool-less calls keep the configured effort. Verified live against api.openai.com (gpt-4o-mini, gpt-5-nano, gpt-5.6-luna, gpt-5.6-sol — basic runs, tool round-trips, automatic prompt caching) and regression-checked against DeepSeek v4 Flash (native cache metrics and temperature:0 default intact). Regression tests: TestClient_IsAnthropic, TestCall_OmitsTemperatureForReasoningModels, TestCall_LearnsReasoningEffortNoneWithTools, TestEngine_PromptCaching_NonAnthropicSkipsMarkers. --- internal/llm/client.go | 72 ++++++++++++++++++-- internal/llm/client_test.go | 132 ++++++++++++++++++++++++++++++++++++ internal/loop/loop.go | 17 +++-- internal/loop/loop_test.go | 44 ++++++++++++ 4 files changed, 255 insertions(+), 10 deletions(-) diff --git a/internal/llm/client.go b/internal/llm/client.go index cb4f967..2aef322 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -10,6 +10,7 @@ import ( "net/http" "strconv" "strings" + "sync/atomic" "time" "github.com/BackendStack21/odek/internal/transport" @@ -25,6 +26,11 @@ type Client struct { MaxTokens int // max output tokens (0 = provider default) Temperature float64 // 0 = use provider default, <0 = omit from request http *http.Client + + // forceNoneEffort is learned at runtime: set when the provider rejects + // reasoning_effort combined with function tools (e.g. gpt-5.6-luna), + // so subsequent calls pin effort to "none" without a failed round-trip. + forceNoneEffort atomic.Bool } // maxResponseSize limits the LLM response body read to prevent DoS/OOM. @@ -54,8 +60,35 @@ func NewWithMaxTokens(baseURL, apiKey, model, thinking string, thinkingBudget in } } +// IsAnthropic reports whether the client's base URL targets the Anthropic +// API. Anthropic-specific request features (the top-level "system" field, +// cache_control markers) must only be sent when this is true — other +// providers reject them (OpenAI answers 400 unknown_parameter). +func (c *Client) IsAnthropic() bool { + return strings.Contains(c.BaseURL, "anthropic") +} + +// modelForbidsTemperature reports whether the model rejects an explicit +// temperature parameter. OpenAI reasoning models (o1/o3/o4 families and the +// gpt-5 series) only accept the default temperature (1); sending any other +// value — including odek's deterministic default 0 — returns a 400 +// "unsupported_value" error. Matching is model-name-based and +// provider-agnostic, since OpenAI-compatible proxies serving these model +// IDs enforce the same constraint. +func modelForbidsTemperature(model string) bool { + m := strings.ToLower(model) + for _, prefix := range []string{"o1", "o3", "o4", "gpt-5"} { + if strings.HasPrefix(m, prefix) { + return true + } + } + return false +} + // CacheControl marks a message or system block as cacheable by Anthropic. -// Providers that don't support it (OpenAI, DeepSeek) silently ignore the field. +// Only send it to Anthropic endpoints: OpenAI rejects Anthropic-style +// request shapes with a 400 (e.g. the top-level "system" field), so the +// loop only applies cache markers when Client.IsAnthropic reports true. type CacheControl struct { Type string `json:"type"` // "ephemeral" } @@ -154,7 +187,8 @@ var toolChoiceNone = "none" // // Returns the updated messages and a System field (populated if the system // message was moved out of the messages array for Anthropic compatibility). -// Providers that don't support prompt caching silently ignore these fields. +// Callers must only use this when the target provider is Anthropic +// (see Client.IsAnthropic) — OpenAI rejects the resulting request shape. func ApplyCacheMarkers(messages []Message) ([]Message, []SystemBlock) { var systemBlocks []SystemBlock annotated := make([]Message, 0, len(messages)) @@ -270,11 +304,11 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy body.Temperature = &one case "disabled": body.Thinking = &ThinkingConfig{Type: "disabled"} - if c.Temperature >= 0 { + if c.Temperature >= 0 && !modelForbidsTemperature(c.Model) { body.Temperature = &c.Temperature } default: - if c.Temperature >= 0 { + if c.Temperature >= 0 && !modelForbidsTemperature(c.Model) { body.Temperature = &c.Temperature } if c.Thinking == "low" || c.Thinking == "medium" || c.Thinking == "high" { @@ -282,18 +316,48 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy } } + // Some models (e.g. gpt-5.6-luna) reject function tools combined with + // any reasoning_effort other than "none" on /chat/completions — and + // their DEFAULT effort is not "none", so merely omitting the field is + // not enough. Once that constraint has been learned from a 400 (see + // below), pin effort to "none" whenever tools are present. + if c.forceNoneEffort.Load() && len(tools) > 0 { + body.ReasoningEffort = "none" + } + reqBytes, err := json.Marshal(body) if err != nil { return nil, fmt.Errorf("llm: marshal request: %w", err) } respBytes, err := c.postChatWithRetry(ctx, reqBytes) + if err != nil && len(tools) > 0 && !c.forceNoneEffort.Load() && reasoningEffortRejected(err) { + // Learn the constraint once, then every later call sends + // reasoning_effort "none" directly (no more failed round-trips). + c.forceNoneEffort.Store(true) + body.ReasoningEffort = "none" + reqBytes, mErr := json.Marshal(body) + if mErr != nil { + return nil, fmt.Errorf("llm: marshal request: %w", mErr) + } + respBytes, err = c.postChatWithRetry(ctx, reqBytes) + } if err != nil { return nil, err } return parseResponse(respBytes) } +// reasoningEffortRejected reports whether err is a 400 whose provider +// response names reasoning_effort as the offending parameter. +func reasoningEffortRejected(err error) bool { + if err == nil { + return false + } + msg := err.Error() + return strings.Contains(msg, "400") && strings.Contains(msg, `"reasoning_effort"`) +} + // postChatWithRetry POSTs reqBytes to /chat/completions and returns the raw 200 // response body, retrying transient network errors and retryable HTTP statuses // (429, 502, 503, 504) with exponential backoff. Shared by every chat call so diff --git a/internal/llm/client_test.go b/internal/llm/client_test.go index df13ade..6c6f27a 100644 --- a/internal/llm/client_test.go +++ b/internal/llm/client_test.go @@ -3,8 +3,11 @@ package llm import ( "context" "encoding/json" + "fmt" + "io" "net/http" "net/http/httptest" + "sync" "testing" "time" ) @@ -1050,3 +1053,132 @@ func TestParseResponse_AnthropicAndOpenAICache(t *testing.T) { t.Errorf("CachedTokens = %d, want 999", result.CachedTokens) } } + +func TestClient_IsAnthropic(t *testing.T) { + cases := []struct { + baseURL string + want bool + }{ + {"https://api.anthropic.com/v1", true}, + {"https://api.openai.com/v1", false}, + {"https://api.deepseek.com/v1", false}, + {"http://localhost:11434/v1", false}, + } + for _, tc := range cases { + c := New(tc.baseURL, "sk-test", "model", "", 0, 0) + if got := c.IsAnthropic(); got != tc.want { + t.Errorf("IsAnthropic(%q) = %v, want %v", tc.baseURL, got, tc.want) + } + } +} + +// OpenAI reasoning models (o1/o3/o4, gpt-5 family) reject any explicit +// temperature other than the default (1) with a 400. The client must omit +// the field for those models while still sending odek's deterministic +// default (0) to models that accept it. +func TestCall_OmitsTemperatureForReasoningModels(t *testing.T) { + cases := []struct { + model string + wantTempSent bool + }{ + {"gpt-5-nano", false}, + {"gpt-5", false}, + {"o3-mini", false}, + {"o1-preview", false}, + {"o4-mini", false}, + {"GPT-5-MINI", false}, // case-insensitive + {"gpt-4o-mini", true}, + {"deepseek-chat", true}, + {"claude-sonnet-4-5", true}, + } + + for _, tc := range cases { + t.Run(tc.model, func(t *testing.T) { + var captured []byte + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured, _ = io.ReadAll(r.Body) + fmt.Fprint(w, `{"choices":[{"message":{"content":"ok"}}]}`) + })) + defer server.Close() + + c := New(server.URL, "sk-test", tc.model, "", 0, 0) + c.Temperature = 0 // odek's deterministic default + if _, err := c.Call(context.Background(), []Message{{Role: "user", Content: "hi"}}, nil, nil); err != nil { + t.Fatalf("Call: %v", err) + } + + var body map[string]any + if err := json.Unmarshal(captured, &body); err != nil { + t.Fatalf("captured request is not JSON: %v", err) + } + _, sent := body["temperature"] + if sent != tc.wantTempSent { + t.Errorf("model %q: temperature sent = %v, want %v (body: %s)", tc.model, sent, tc.wantTempSent, captured) + } + }) + } +} + +// Models like gpt-5.6-luna reject function tools combined with any +// reasoning_effort other than "none" — and their default effort is not +// "none", so omitting the field still 400s. The client must learn the +// constraint from the 400, retry with reasoning_effort "none", and pin it +// for subsequent calls without further failed round-trips. +func TestCall_LearnsReasoningEffortNoneWithTools(t *testing.T) { + tools := []ToolDef{{ + Type: "function", + Function: FunctionDef{ + Name: "echo", + Description: "echoes input", + Parameters: map[string]any{"type": "object"}, + }, + }} + + var mu sync.Mutex + var efforts []string // recorded reasoning_effort per request ("" = absent) + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + body, _ := io.ReadAll(r.Body) + var parsed map[string]any + _ = json.Unmarshal(body, &parsed) + effort, _ := parsed["reasoning_effort"].(string) + mu.Lock() + efforts = append(efforts, effort) + mu.Unlock() + toolsArr, _ := parsed["tools"].([]any) + if len(toolsArr) > 0 && effort != "none" { + w.WriteHeader(http.StatusBadRequest) + fmt.Fprint(w, `{"error":{"message":"Function tools with reasoning_effort are not supported","type":"invalid_request_error","param":"reasoning_effort"}}`) + return + } + fmt.Fprint(w, `{"choices":[{"message":{"content":"ok"}}]}`) + })) + defer server.Close() + + c := New(server.URL, "sk-test", "gpt-5.6-luna", "high", 0, 0) + msgs := []Message{{Role: "user", Content: "hi"}} + + // First call: 400 (effort "high") → learned retry with "none" → success. + if _, err := c.Call(context.Background(), msgs, nil, tools); err != nil { + t.Fatalf("first Call: %v", err) + } + // Second call: must send "none" immediately, no failed attempt first. + if _, err := c.Call(context.Background(), msgs, nil, tools); err != nil { + t.Fatalf("second Call: %v", err) + } + // Calls without tools keep the configured effort. + if _, err := c.Call(context.Background(), msgs, nil, nil); err != nil { + t.Fatalf("tool-less Call: %v", err) + } + + mu.Lock() + defer mu.Unlock() + want := []string{"high", "none", "none", "high"} + if len(efforts) != len(want) { + t.Fatalf("requests = %v, want %v", efforts, want) + } + for i := range want { + if efforts[i] != want[i] { + t.Fatalf("requests = %v, want %v", efforts, want) + } + } +} diff --git a/internal/loop/loop.go b/internal/loop/loop.go index c29f630..2a53bc1 100644 --- a/internal/loop/loop.go +++ b/internal/loop/loop.go @@ -149,10 +149,12 @@ type Engine struct { // across turns — only the memory message changes each iteration. memMsgIdx int - // PromptCaching enables Anthropic/OpenAI/DeepSeek prompt caching markers. - // When enabled, the system prompt and first user message are annotated - // with cache_control markers, and the system prompt is moved to the - // dedicated "system" field for Anthropic compatibility. + // PromptCaching enables Anthropic prompt caching markers. When enabled + // and the LLM endpoint is Anthropic, the system prompt and first user + // message are annotated with cache_control markers, and the system + // prompt is moved to the dedicated "system" field. For non-Anthropic + // endpoints (OpenAI, DeepSeek) the markers are skipped entirely — those + // providers cache automatically or reject the Anthropic request shape. PromptCaching bool // MaxToolParallel controls how many tool calls run concurrently per @@ -818,10 +820,13 @@ func (e *Engine) runLoop(ctx context.Context, messages []llm.Message) (string, [ // THINK (timed) start := time.Now() - // Apply prompt caching markers when enabled + // Apply prompt caching markers when enabled — but only for Anthropic + // endpoints. OpenAI rejects the Anthropic request shape (top-level + // "system" field) with a 400, and DeepSeek caches automatically, + // so markers would be harmful or useless there. var systemBlocks []llm.SystemBlock callMsgs := messages - if e.PromptCaching { + if e.PromptCaching && e.client.IsAnthropic() { callMsgs, systemBlocks = llm.ApplyCacheMarkers(messages) } diff --git a/internal/loop/loop_test.go b/internal/loop/loop_test.go index f3dd77e..0d4e565 100644 --- a/internal/loop/loop_test.go +++ b/internal/loop/loop_test.go @@ -5,6 +5,7 @@ import ( "context" "encoding/json" "fmt" + "io" "net/http" "net/http/httptest" "sort" @@ -2519,3 +2520,46 @@ func TestClassifyToolCall_ShellStillWorks(t *testing.T) { t.Errorf("shell resource = %q, want command", resource) } } + +// Regression test: prompt caching markers are Anthropic-only. When +// PromptCaching is enabled against a non-Anthropic endpoint (e.g. OpenAI), +// the request must not contain the Anthropic-style top-level "system" field +// or cache_control markers — OpenAI rejects them with 400 unknown_parameter. +func TestEngine_PromptCaching_NonAnthropicSkipsMarkers(t *testing.T) { + var captured []byte + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + captured, _ = io.ReadAll(r.Body) + fmt.Fprint(w, `{"choices":[{"message":{"content":"done"}}]}`) + })) + defer server.Close() + + // server.URL (127.0.0.1) is not an Anthropic endpoint. + client := llm.New(server.URL, "sk-test", "test-model", "", 0, 0) + registry := tool.NewRegistry(nil) + engine := New(client, registry, 10, "You are a test agent.", nil, 0) + engine.PromptCaching = true + + if _, err := engine.Run(context.Background(), "hi"); err != nil { + t.Fatalf("Run() error: %v", err) + } + + var body map[string]any + if err := json.Unmarshal(captured, &body); err != nil { + t.Fatalf("captured request is not JSON: %v", err) + } + if _, ok := body["system"]; ok { + t.Errorf("non-Anthropic request must not contain top-level system field: %s", captured) + } + if strings.Contains(string(captured), "cache_control") { + t.Errorf("non-Anthropic request must not contain cache_control markers: %s", captured) + } + // The system prompt must still reach the model as a system message. + msgs, _ := body["messages"].([]any) + if len(msgs) == 0 { + t.Fatalf("no messages in request: %s", captured) + } + first, _ := msgs[0].(map[string]any) + if first["role"] != "system" { + t.Errorf("first message role = %v, want system", first["role"]) + } +} From b94f9e4e43a766d8d70649514917c4448eba2814 Mon Sep 17 00:00:00 2001 From: Rolando Santamaria Maso Date: Sat, 25 Jul 2026 14:22:46 +0200 Subject: [PATCH 2/2] fix(llm): map thinking enabled/disabled per provider instead of always sending the Anthropic object MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The thinking object ({"type":"enabled"/"disabled"}) was sent unconditionally; OpenAI rejects unknown top-level parameters with a 400, so thinking: disabled (and enabled) was broken against OpenAI — same family as the system-field bug fixed earlier in this branch. The shaping logic now lives in an extracted, directly testable buildCallParams method: - Anthropic/DeepSeek (new sendsThinkingObject gate): unchanged, thinking object sent as before. - OpenAI reasoning models (o1/o3/o4, gpt-5 series): enabled maps to reasoning_effort "high", disabled to "none" (verified accepted on the gpt-5 series); temperature stays omitted per the forbidlist. - Non-reasoning models (e.g. gpt-4o): no thinking capability exists, so both fields are omitted and temperature is honored as configured. The two existing thinking tests asserted the old unconditional behavior through an httptest server whose URL is never a recognized provider; they now exercise buildCallParams directly. New table test TestBuildCallParams_ThinkingMapping covers all provider/model/thinking combinations. --- internal/llm/client.go | 77 +++++++++++++++++++++-------- internal/llm/client_test.go | 99 ++++++++++++++++++++----------------- 2 files changed, 111 insertions(+), 65 deletions(-) diff --git a/internal/llm/client.go b/internal/llm/client.go index 2aef322..c277bf2 100644 --- a/internal/llm/client.go +++ b/internal/llm/client.go @@ -68,6 +68,15 @@ func (c *Client) IsAnthropic() bool { return strings.Contains(c.BaseURL, "anthropic") } +// sendsThinkingObject reports whether the provider accepts the +// Anthropic-style "thinking" request object. Anthropic requires it for +// extended thinking and DeepSeek supports it natively (deepseek-reasoner); +// other providers (OpenAI and compatibles) reject unknown top-level +// parameters, so thinking intent must be mapped to reasoning_effort instead. +func (c *Client) sendsThinkingObject() bool { + return c.IsAnthropic() || strings.Contains(c.BaseURL, "deepseek") +} + // modelForbidsTemperature reports whether the model rejects an explicit // temperature parameter. OpenAI reasoning models (o1/o3/o4 families and the // gpt-5 series) only accept the default temperature (1); sending any other @@ -272,12 +281,11 @@ func (c *Client) SimpleCall(ctx context.Context, systemPrompt, userPrompt string return raw.Choices[0].Message.Content, nil } -// Call sends a chat completion request and returns the result. -// systemBlocks is optional — pass nil for providers that don't support -// the separate System field (OpenAI, DeepSeek). When non-nil, the system -// prompt is sent in the "system" field instead of as a system message in -// the messages array (Anthropic format for prompt caching). -func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []SystemBlock, tools []ToolDef) (*CallResult, error) { +// buildCallParams assembles the request body for a Call, mapping the +// thinking configuration onto whatever shape the target provider accepts: +// the Anthropic-style "thinking" object for Anthropic/DeepSeek, +// reasoning_effort for OpenAI reasoning models, and nothing otherwise. +func (c *Client) buildCallParams(messages []Message, systemBlocks []SystemBlock, tools []ToolDef) CallParams { body := CallParams{ Model: c.Model, Messages: messages, @@ -289,21 +297,39 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy switch c.Thinking { case "enabled": - // Anthropic requires budget_tokens when enabling thinking. - // 5000 is a safe default: leaves ample room for the text response - // even on models with 8K max output (e.g. Claude Haiku). - // DeepSeek silently ignores the field. - budget := c.ThinkingBudget - if budget <= 0 { - budget = 5000 + if c.sendsThinkingObject() { + // Anthropic requires budget_tokens when enabling thinking. + // 5000 is a safe default: leaves ample room for the text response + // even on models with 8K max output (e.g. Claude Haiku). + // DeepSeek silently ignores the field. + budget := c.ThinkingBudget + if budget <= 0 { + budget = 5000 + } + body.Thinking = &ThinkingConfig{Type: "enabled", BudgetTokens: budget} + // Anthropic also requires temperature=1 when thinking is enabled. + // Force it regardless of the configured temperature to avoid a 400. + one := float64(1) + body.Temperature = &one + } else if modelForbidsTemperature(c.Model) { + // OpenAI reasoning models have no "thinking" object; the closest + // mapping for enabled extended thinking is maximum effort. + // Temperature stays omitted — only the provider default is accepted. + body.ReasoningEffort = "high" + } else if c.Temperature >= 0 { + // Non-reasoning models (e.g. gpt-4o) have no thinking to enable; + // just honor the configured temperature. + body.Temperature = &c.Temperature } - body.Thinking = &ThinkingConfig{Type: "enabled", BudgetTokens: budget} - // Anthropic also requires temperature=1 when thinking is enabled. - // Force it regardless of the configured temperature to avoid a 400. - one := float64(1) - body.Temperature = &one case "disabled": - body.Thinking = &ThinkingConfig{Type: "disabled"} + if c.sendsThinkingObject() { + body.Thinking = &ThinkingConfig{Type: "disabled"} + } else if modelForbidsTemperature(c.Model) { + // OpenAI reasoning models: disabling thinking maps to effort + // "none" (accepted on the gpt-5 series). Non-reasoning models + // have nothing to disable, so the field is omitted entirely. + body.ReasoningEffort = "none" + } if c.Temperature >= 0 && !modelForbidsTemperature(c.Model) { body.Temperature = &c.Temperature } @@ -320,11 +346,22 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy // any reasoning_effort other than "none" on /chat/completions — and // their DEFAULT effort is not "none", so merely omitting the field is // not enough. Once that constraint has been learned from a 400 (see - // below), pin effort to "none" whenever tools are present. + // Call), pin effort to "none" whenever tools are present. if c.forceNoneEffort.Load() && len(tools) > 0 { body.ReasoningEffort = "none" } + return body +} + +// Call sends a chat completion request and returns the result. +// systemBlocks is optional — pass nil for providers that don't support +// the separate System field (OpenAI, DeepSeek). When non-nil, the system +// prompt is sent in the "system" field instead of as a system message in +// the messages array (Anthropic format for prompt caching). +func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []SystemBlock, tools []ToolDef) (*CallResult, error) { + body := c.buildCallParams(messages, systemBlocks, tools) + reqBytes, err := json.Marshal(body) if err != nil { return nil, fmt.Errorf("llm: marshal request: %w", err) diff --git a/internal/llm/client_test.go b/internal/llm/client_test.go index 6c6f27a..de9a578 100644 --- a/internal/llm/client_test.go +++ b/internal/llm/client_test.go @@ -409,30 +409,11 @@ func TestClient_Call_HTTPError(t *testing.T) { } func TestClient_Call_WithThinking(t *testing.T) { - var receivedBody map[string]any - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewDecoder(r.Body).Decode(&receivedBody) - w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"choices":[{"message":{"content":"thinking response"}}]}`)) - })) - defer server.Close() - - c := New(server.URL, "sk-test", "deepseek-chat", "enabled", 0, 0) - result, err := c.Call(context.Background(), []Message{{Role: "user", Content: "think"}}, nil, nil) - if err != nil { - t.Fatalf("Call() error: %v", err) - } - if result.Content != "thinking response" { - t.Errorf("Content = %q", result.Content) - } - // Verify thinking was sent in the request - thinking, ok := receivedBody["thinking"] - if !ok { - t.Error("thinking field not sent in request") - } - thinkingMap, ok := thinking.(map[string]any) - if !ok || thinkingMap["type"] != "enabled" { - t.Errorf("thinking = %v, want {type: enabled}", thinking) + // DeepSeek supports the Anthropic-style thinking object natively. + c := New("https://api.deepseek.com/v1", "sk-test", "deepseek-chat", "enabled", 0, 0) + body := c.buildCallParams([]Message{{Role: "user", Content: "think"}}, nil, nil) + if body.Thinking == nil || body.Thinking.Type != "enabled" { + t.Errorf("thinking = %v, want {type: enabled}", body.Thinking) } } @@ -723,29 +704,14 @@ func TestClient_Call_FlashVsProThinkingContrast(t *testing.T) { }) t.Run("pro_thinking_enabled", func(t *testing.T) { - var body map[string]any - server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - json.NewDecoder(r.Body).Decode(&body) - w.Header().Set("Content-Type", "application/json") - w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`)) - })) - defer server.Close() - - c := New(server.URL, "sk-test", "deepseek-v4-pro", "enabled", 0, 0) - _, err := c.Call(context.Background(), []Message{{Role: "user", Content: "hi"}}, nil, nil) - if err != nil { - t.Fatal(err) - } - thinking, ok := body["thinking"] - if !ok { + // DeepSeek supports the Anthropic-style thinking object natively. + c := New("https://api.deepseek.com/v1", "sk-test", "deepseek-v4-pro", "enabled", 0, 0) + body := c.buildCallParams([]Message{{Role: "user", Content: "hi"}}, nil, nil) + if body.Thinking == nil { t.Fatal("Pro: thinking field should be present") } - thinkingMap, ok := thinking.(map[string]any) - if !ok { - t.Fatal("Pro: thinking should be an object") - } - if thinkingMap["type"] != "enabled" { - t.Errorf("Pro: thinking.type = %v, want 'enabled'", thinkingMap["type"]) + if body.Thinking.Type != "enabled" { + t.Errorf("Pro: thinking.type = %v, want 'enabled'", body.Thinking.Type) } }) } @@ -1182,3 +1148,46 @@ func TestCall_LearnsReasoningEffortNoneWithTools(t *testing.T) { } } } + +// The thinking configuration must be mapped onto the shape each provider +// accepts: the Anthropic-style "thinking" object for Anthropic/DeepSeek, +// reasoning_effort for OpenAI reasoning models, and nothing otherwise — +// OpenAI rejects unknown top-level parameters with a 400. +func TestBuildCallParams_ThinkingMapping(t *testing.T) { + cases := []struct { + name string + baseURL string + model string + thinking string + wantThinking string // "" = no thinking object + wantEffort string // "" = no reasoning_effort + }{ + {"deepseek enabled", "https://api.deepseek.com/v1", "deepseek-chat", "enabled", "enabled", ""}, + {"deepseek disabled", "https://api.deepseek.com/v1", "deepseek-chat", "disabled", "disabled", ""}, + {"anthropic enabled", "https://api.anthropic.com/v1", "claude-sonnet-4-5", "enabled", "enabled", ""}, + {"anthropic disabled", "https://api.anthropic.com/v1", "claude-sonnet-4-5", "disabled", "disabled", ""}, + {"openai reasoning disabled", "https://api.openai.com/v1", "gpt-5-nano", "disabled", "", "none"}, + {"openai reasoning enabled", "https://api.openai.com/v1", "gpt-5.6-luna", "enabled", "", "high"}, + {"openai reasoning effort passthrough", "https://api.openai.com/v1", "gpt-5-nano", "medium", "", "medium"}, + {"openai non-reasoning disabled", "https://api.openai.com/v1", "gpt-4o-mini", "disabled", "", ""}, + {"openai non-reasoning enabled", "https://api.openai.com/v1", "gpt-4o-mini", "enabled", "", ""}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + c := New(tc.baseURL, "sk-test", tc.model, tc.thinking, 0, 0) + body := c.buildCallParams([]Message{{Role: "user", Content: "hi"}}, nil, nil) + + gotThinking := "" + if body.Thinking != nil { + gotThinking = body.Thinking.Type + } + if gotThinking != tc.wantThinking { + t.Errorf("thinking object = %q, want %q", gotThinking, tc.wantThinking) + } + if body.ReasoningEffort != tc.wantEffort { + t.Errorf("reasoning_effort = %q, want %q", body.ReasoningEffort, tc.wantEffort) + } + }) + } +}