Skip to content

Commit b94f9e4

Browse files
committed
fix(llm): map thinking enabled/disabled per provider instead of always sending the Anthropic object
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.
1 parent 3f4888c commit b94f9e4

2 files changed

Lines changed: 111 additions & 65 deletions

File tree

internal/llm/client.go

Lines changed: 57 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,15 @@ func (c *Client) IsAnthropic() bool {
6868
return strings.Contains(c.BaseURL, "anthropic")
6969
}
7070

71+
// sendsThinkingObject reports whether the provider accepts the
72+
// Anthropic-style "thinking" request object. Anthropic requires it for
73+
// extended thinking and DeepSeek supports it natively (deepseek-reasoner);
74+
// other providers (OpenAI and compatibles) reject unknown top-level
75+
// parameters, so thinking intent must be mapped to reasoning_effort instead.
76+
func (c *Client) sendsThinkingObject() bool {
77+
return c.IsAnthropic() || strings.Contains(c.BaseURL, "deepseek")
78+
}
79+
7180
// modelForbidsTemperature reports whether the model rejects an explicit
7281
// temperature parameter. OpenAI reasoning models (o1/o3/o4 families and the
7382
// 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
272281
return raw.Choices[0].Message.Content, nil
273282
}
274283

275-
// Call sends a chat completion request and returns the result.
276-
// systemBlocks is optional — pass nil for providers that don't support
277-
// the separate System field (OpenAI, DeepSeek). When non-nil, the system
278-
// prompt is sent in the "system" field instead of as a system message in
279-
// the messages array (Anthropic format for prompt caching).
280-
func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []SystemBlock, tools []ToolDef) (*CallResult, error) {
284+
// buildCallParams assembles the request body for a Call, mapping the
285+
// thinking configuration onto whatever shape the target provider accepts:
286+
// the Anthropic-style "thinking" object for Anthropic/DeepSeek,
287+
// reasoning_effort for OpenAI reasoning models, and nothing otherwise.
288+
func (c *Client) buildCallParams(messages []Message, systemBlocks []SystemBlock, tools []ToolDef) CallParams {
281289
body := CallParams{
282290
Model: c.Model,
283291
Messages: messages,
@@ -289,21 +297,39 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy
289297

290298
switch c.Thinking {
291299
case "enabled":
292-
// Anthropic requires budget_tokens when enabling thinking.
293-
// 5000 is a safe default: leaves ample room for the text response
294-
// even on models with 8K max output (e.g. Claude Haiku).
295-
// DeepSeek silently ignores the field.
296-
budget := c.ThinkingBudget
297-
if budget <= 0 {
298-
budget = 5000
300+
if c.sendsThinkingObject() {
301+
// Anthropic requires budget_tokens when enabling thinking.
302+
// 5000 is a safe default: leaves ample room for the text response
303+
// even on models with 8K max output (e.g. Claude Haiku).
304+
// DeepSeek silently ignores the field.
305+
budget := c.ThinkingBudget
306+
if budget <= 0 {
307+
budget = 5000
308+
}
309+
body.Thinking = &ThinkingConfig{Type: "enabled", BudgetTokens: budget}
310+
// Anthropic also requires temperature=1 when thinking is enabled.
311+
// Force it regardless of the configured temperature to avoid a 400.
312+
one := float64(1)
313+
body.Temperature = &one
314+
} else if modelForbidsTemperature(c.Model) {
315+
// OpenAI reasoning models have no "thinking" object; the closest
316+
// mapping for enabled extended thinking is maximum effort.
317+
// Temperature stays omitted — only the provider default is accepted.
318+
body.ReasoningEffort = "high"
319+
} else if c.Temperature >= 0 {
320+
// Non-reasoning models (e.g. gpt-4o) have no thinking to enable;
321+
// just honor the configured temperature.
322+
body.Temperature = &c.Temperature
299323
}
300-
body.Thinking = &ThinkingConfig{Type: "enabled", BudgetTokens: budget}
301-
// Anthropic also requires temperature=1 when thinking is enabled.
302-
// Force it regardless of the configured temperature to avoid a 400.
303-
one := float64(1)
304-
body.Temperature = &one
305324
case "disabled":
306-
body.Thinking = &ThinkingConfig{Type: "disabled"}
325+
if c.sendsThinkingObject() {
326+
body.Thinking = &ThinkingConfig{Type: "disabled"}
327+
} else if modelForbidsTemperature(c.Model) {
328+
// OpenAI reasoning models: disabling thinking maps to effort
329+
// "none" (accepted on the gpt-5 series). Non-reasoning models
330+
// have nothing to disable, so the field is omitted entirely.
331+
body.ReasoningEffort = "none"
332+
}
307333
if c.Temperature >= 0 && !modelForbidsTemperature(c.Model) {
308334
body.Temperature = &c.Temperature
309335
}
@@ -320,11 +346,22 @@ func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []Sy
320346
// any reasoning_effort other than "none" on /chat/completions — and
321347
// their DEFAULT effort is not "none", so merely omitting the field is
322348
// not enough. Once that constraint has been learned from a 400 (see
323-
// below), pin effort to "none" whenever tools are present.
349+
// Call), pin effort to "none" whenever tools are present.
324350
if c.forceNoneEffort.Load() && len(tools) > 0 {
325351
body.ReasoningEffort = "none"
326352
}
327353

354+
return body
355+
}
356+
357+
// Call sends a chat completion request and returns the result.
358+
// systemBlocks is optional — pass nil for providers that don't support
359+
// the separate System field (OpenAI, DeepSeek). When non-nil, the system
360+
// prompt is sent in the "system" field instead of as a system message in
361+
// the messages array (Anthropic format for prompt caching).
362+
func (c *Client) Call(ctx context.Context, messages []Message, systemBlocks []SystemBlock, tools []ToolDef) (*CallResult, error) {
363+
body := c.buildCallParams(messages, systemBlocks, tools)
364+
328365
reqBytes, err := json.Marshal(body)
329366
if err != nil {
330367
return nil, fmt.Errorf("llm: marshal request: %w", err)

internal/llm/client_test.go

Lines changed: 54 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -409,30 +409,11 @@ func TestClient_Call_HTTPError(t *testing.T) {
409409
}
410410

411411
func TestClient_Call_WithThinking(t *testing.T) {
412-
var receivedBody map[string]any
413-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
414-
json.NewDecoder(r.Body).Decode(&receivedBody)
415-
w.Header().Set("Content-Type", "application/json")
416-
w.Write([]byte(`{"choices":[{"message":{"content":"thinking response"}}]}`))
417-
}))
418-
defer server.Close()
419-
420-
c := New(server.URL, "sk-test", "deepseek-chat", "enabled", 0, 0)
421-
result, err := c.Call(context.Background(), []Message{{Role: "user", Content: "think"}}, nil, nil)
422-
if err != nil {
423-
t.Fatalf("Call() error: %v", err)
424-
}
425-
if result.Content != "thinking response" {
426-
t.Errorf("Content = %q", result.Content)
427-
}
428-
// Verify thinking was sent in the request
429-
thinking, ok := receivedBody["thinking"]
430-
if !ok {
431-
t.Error("thinking field not sent in request")
432-
}
433-
thinkingMap, ok := thinking.(map[string]any)
434-
if !ok || thinkingMap["type"] != "enabled" {
435-
t.Errorf("thinking = %v, want {type: enabled}", thinking)
412+
// DeepSeek supports the Anthropic-style thinking object natively.
413+
c := New("https://api.deepseek.com/v1", "sk-test", "deepseek-chat", "enabled", 0, 0)
414+
body := c.buildCallParams([]Message{{Role: "user", Content: "think"}}, nil, nil)
415+
if body.Thinking == nil || body.Thinking.Type != "enabled" {
416+
t.Errorf("thinking = %v, want {type: enabled}", body.Thinking)
436417
}
437418
}
438419

@@ -723,29 +704,14 @@ func TestClient_Call_FlashVsProThinkingContrast(t *testing.T) {
723704
})
724705

725706
t.Run("pro_thinking_enabled", func(t *testing.T) {
726-
var body map[string]any
727-
server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
728-
json.NewDecoder(r.Body).Decode(&body)
729-
w.Header().Set("Content-Type", "application/json")
730-
w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`))
731-
}))
732-
defer server.Close()
733-
734-
c := New(server.URL, "sk-test", "deepseek-v4-pro", "enabled", 0, 0)
735-
_, err := c.Call(context.Background(), []Message{{Role: "user", Content: "hi"}}, nil, nil)
736-
if err != nil {
737-
t.Fatal(err)
738-
}
739-
thinking, ok := body["thinking"]
740-
if !ok {
707+
// DeepSeek supports the Anthropic-style thinking object natively.
708+
c := New("https://api.deepseek.com/v1", "sk-test", "deepseek-v4-pro", "enabled", 0, 0)
709+
body := c.buildCallParams([]Message{{Role: "user", Content: "hi"}}, nil, nil)
710+
if body.Thinking == nil {
741711
t.Fatal("Pro: thinking field should be present")
742712
}
743-
thinkingMap, ok := thinking.(map[string]any)
744-
if !ok {
745-
t.Fatal("Pro: thinking should be an object")
746-
}
747-
if thinkingMap["type"] != "enabled" {
748-
t.Errorf("Pro: thinking.type = %v, want 'enabled'", thinkingMap["type"])
713+
if body.Thinking.Type != "enabled" {
714+
t.Errorf("Pro: thinking.type = %v, want 'enabled'", body.Thinking.Type)
749715
}
750716
})
751717
}
@@ -1182,3 +1148,46 @@ func TestCall_LearnsReasoningEffortNoneWithTools(t *testing.T) {
11821148
}
11831149
}
11841150
}
1151+
1152+
// The thinking configuration must be mapped onto the shape each provider
1153+
// accepts: the Anthropic-style "thinking" object for Anthropic/DeepSeek,
1154+
// reasoning_effort for OpenAI reasoning models, and nothing otherwise —
1155+
// OpenAI rejects unknown top-level parameters with a 400.
1156+
func TestBuildCallParams_ThinkingMapping(t *testing.T) {
1157+
cases := []struct {
1158+
name string
1159+
baseURL string
1160+
model string
1161+
thinking string
1162+
wantThinking string // "" = no thinking object
1163+
wantEffort string // "" = no reasoning_effort
1164+
}{
1165+
{"deepseek enabled", "https://api.deepseek.com/v1", "deepseek-chat", "enabled", "enabled", ""},
1166+
{"deepseek disabled", "https://api.deepseek.com/v1", "deepseek-chat", "disabled", "disabled", ""},
1167+
{"anthropic enabled", "https://api.anthropic.com/v1", "claude-sonnet-4-5", "enabled", "enabled", ""},
1168+
{"anthropic disabled", "https://api.anthropic.com/v1", "claude-sonnet-4-5", "disabled", "disabled", ""},
1169+
{"openai reasoning disabled", "https://api.openai.com/v1", "gpt-5-nano", "disabled", "", "none"},
1170+
{"openai reasoning enabled", "https://api.openai.com/v1", "gpt-5.6-luna", "enabled", "", "high"},
1171+
{"openai reasoning effort passthrough", "https://api.openai.com/v1", "gpt-5-nano", "medium", "", "medium"},
1172+
{"openai non-reasoning disabled", "https://api.openai.com/v1", "gpt-4o-mini", "disabled", "", ""},
1173+
{"openai non-reasoning enabled", "https://api.openai.com/v1", "gpt-4o-mini", "enabled", "", ""},
1174+
}
1175+
1176+
for _, tc := range cases {
1177+
t.Run(tc.name, func(t *testing.T) {
1178+
c := New(tc.baseURL, "sk-test", tc.model, tc.thinking, 0, 0)
1179+
body := c.buildCallParams([]Message{{Role: "user", Content: "hi"}}, nil, nil)
1180+
1181+
gotThinking := ""
1182+
if body.Thinking != nil {
1183+
gotThinking = body.Thinking.Type
1184+
}
1185+
if gotThinking != tc.wantThinking {
1186+
t.Errorf("thinking object = %q, want %q", gotThinking, tc.wantThinking)
1187+
}
1188+
if body.ReasoningEffort != tc.wantEffort {
1189+
t.Errorf("reasoning_effort = %q, want %q", body.ReasoningEffort, tc.wantEffort)
1190+
}
1191+
})
1192+
}
1193+
}

0 commit comments

Comments
 (0)