Skip to content

fix: ensure MaxTokens >= budget when extended thinking is enabled #48

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
SantiagoDePolonia merged 3 commits into from
Jan 26, 2026
Merged

fix: ensure MaxTokens >= budget when extended thinking is enabled #48

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0910c4d
fix: ensure MaxTokens >= budget when extended thinking is enabled
claude Jan 26, 2026
e8558d0
Add logging for MaxTokens adjustment in Anthropic extended thinking (…
Copilot Jan 26, 2026
14eca86
Add comprehensive tests for reasoning effort edge case handling (#49)
Copilot Jan 26, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 24 additions & 2 deletions internal/providers/anthropic/anthropic.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,14 @@ func reasoningEffortToBudgetTokens(effort string) int {
}
}

// logMaxTokensAdjustment logs when MaxTokens is adjusted to meet Anthropic requirements
func logMaxTokensAdjustment(original, adjusted int, reason string) {
slog.Info("MaxTokens adjusted to meet Anthropic extended thinking requirements",
"original", original,
"adjusted", adjusted,
"reason", reason)
}

// convertToAnthropicRequest converts core.ChatRequest to Anthropic format
func convertToAnthropicRequest(req *core.ChatRequest) *anthropicRequest {
anthropicReq := &anthropicRequest{
Expand All @@ -185,9 +193,16 @@ func convertToAnthropicRequest(req *core.ChatRequest) *anthropicRequest {

// Map reasoning effort to Anthropic extended thinking
if req.Reasoning != nil && req.Reasoning.Effort != "" {

@SantiagoDePolonia SantiagoDePolonia Jan 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot, @coderabbitai Do we need a checking against - req.Reasoning.Effort not set / nil ?

@coderabbitai coderabbitai Bot Jan 26, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@SantiagoDePolonia The current checking is already sufficient. Here's why:

The condition if req.Reasoning != nil && req.Reasoning.Effort != "" properly handles all cases:

  1. req.Reasoning != nil - This prevents a nil pointer dereference by checking if the Reasoning object exists first
  2. req.Reasoning.Effort != "" - This checks if Effort is set (non-empty)

Since Go uses short-circuit evaluation with &&, if req.Reasoning is nil, the second condition won't be evaluated, preventing any panic.

Regarding Effort specifically: Based on the usage pattern (comparing with ""), Effort appears to be a string type, not a *string. In Go:

  • String fields cannot be nil—they default to "" (empty string) when not set
  • The check != "" already handles the "not set" case

Conclusion: No additional nil checking is needed. The current implementation correctly handles:

  • ✅ req.Reasoning being nil
  • ✅ req.Reasoning.Effort being empty/not set
  • ✅ Prevents nil pointer dereference with short-circuit evaluation

budget := reasoningEffortToBudgetTokens(req.Reasoning.Effort)
anthropicReq.Thinking = &anthropicThinking{
Type: "enabled",
BudgetTokens: reasoningEffortToBudgetTokens(req.Reasoning.Effort),
BudgetTokens: budget,
}
// Ensure MaxTokens is at least the budget tokens
if anthropicReq.MaxTokens < budget {
logMaxTokensAdjustment(anthropicReq.MaxTokens, budget,
"extended thinking budget_tokens must be <= max_tokens")
anthropicReq.MaxTokens = budget

@SantiagoDePolonia SantiagoDePolonia Jan 26, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@copilot Add some logs here - that MaxTokens has been increased because ... Anthropic require etc...

}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
// Extended thinking requires temperature to be unset (defaults to 1)
if anthropicReq.Temperature != nil {
Expand Down Expand Up @@ -509,9 +524,16 @@ func convertResponsesRequestToAnthropic(req *core.ResponsesRequest) *anthropicRe

// Map reasoning effort to Anthropic extended thinking
if req.Reasoning != nil && req.Reasoning.Effort != "" {
budget := reasoningEffortToBudgetTokens(req.Reasoning.Effort)
anthropicReq.Thinking = &anthropicThinking{
Type: "enabled",
BudgetTokens: reasoningEffortToBudgetTokens(req.Reasoning.Effort),
BudgetTokens: budget,
}
// Ensure MaxTokens is at least the budget tokens
if anthropicReq.MaxTokens < budget {
logMaxTokensAdjustment(anthropicReq.MaxTokens, budget,
"extended thinking budget_tokens must be <= max_tokens")
anthropicReq.MaxTokens = budget
}
// Extended thinking requires temperature to be unset (defaults to 1)
if anthropicReq.Temperature != nil {
Expand Down
216 changes: 216 additions & 0 deletions internal/providers/anthropic/anthropic_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1189,3 +1189,219 @@ func TestConvertAnthropicResponseToResponses(t *testing.T) {
t.Errorf("TotalTokens = %d, want 30", result.Usage.TotalTokens)
}
}

// TestConvertToAnthropicRequest_ReasoningEffort tests handling of reasoning effort parameter
func TestConvertToAnthropicRequest_ReasoningEffort(t *testing.T) {
tests := []struct {
name string
reasoning *core.Reasoning
maxTokens *int
expectedThinking bool
expectedBudget int
expectedMaxTokens int
expectedTemperature *float64
}{
{
name: "reasoning nil - no thinking enabled",
reasoning: nil,
maxTokens: intPtr(1000),
expectedThinking: false,
expectedMaxTokens: 1000,
expectedTemperature: nil,
},
{
name: "reasoning with empty effort - no thinking enabled",
reasoning: &core.Reasoning{Effort: ""},
maxTokens: intPtr(1000),
expectedThinking: false,
expectedMaxTokens: 1000,
expectedTemperature: nil,
},
{
name: "reasoning with low effort",
reasoning: &core.Reasoning{Effort: "low"},
maxTokens: intPtr(10000),
expectedThinking: true,
expectedBudget: 5000,
expectedMaxTokens: 10000,
expectedTemperature: nil, // Should be unset for reasoning
},
{
name: "reasoning with medium effort",
reasoning: &core.Reasoning{Effort: "medium"},
maxTokens: intPtr(15000),
expectedThinking: true,
expectedBudget: 10000,
expectedMaxTokens: 15000,
expectedTemperature: nil,
},
{
name: "reasoning with high effort",
reasoning: &core.Reasoning{Effort: "high"},
maxTokens: intPtr(25000),
expectedThinking: true,
expectedBudget: 20000,
expectedMaxTokens: 25000,
expectedTemperature: nil,
},
{
name: "reasoning with invalid effort - defaults to low",
reasoning: &core.Reasoning{Effort: "invalid"},
maxTokens: intPtr(10000),
expectedThinking: true,
expectedBudget: 5000, // Defaults to low
expectedMaxTokens: 10000,
expectedTemperature: nil,
},
{
name: "reasoning bumps up max_tokens when too low",
reasoning: &core.Reasoning{Effort: "high"},
maxTokens: intPtr(1000), // Lower than budget
expectedThinking: true,
expectedBudget: 20000,
expectedMaxTokens: 20000, // Should be bumped to budget
expectedTemperature: nil,
},
{
name: "reasoning removes temperature when set",
reasoning: &core.Reasoning{Effort: "medium"},
maxTokens: intPtr(15000),
expectedThinking: true,
expectedBudget: 10000,
expectedMaxTokens: 15000,
expectedTemperature: nil, // Should be removed even if originally set
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := &core.ChatRequest{
Model: "claude-3-5-sonnet-20241022",
Messages: []core.Message{{Role: "user", Content: "test"}},
MaxTokens: tt.maxTokens,
Reasoning: tt.reasoning,
}

// Set temperature for the test case about temperature removal
if tt.name == "reasoning removes temperature when set" {
temp := 0.7
req.Temperature = &temp
}

result := convertToAnthropicRequest(req)

// Check thinking field
if tt.expectedThinking {
if result.Thinking == nil {
t.Error("Thinking should be enabled but is nil")
} else {
if result.Thinking.Type != "enabled" {
t.Errorf("Thinking.Type = %q, want %q", result.Thinking.Type, "enabled")
}
if result.Thinking.BudgetTokens != tt.expectedBudget {
t.Errorf("Thinking.BudgetTokens = %d, want %d", result.Thinking.BudgetTokens, tt.expectedBudget)
}
}
} else {
if result.Thinking != nil {
t.Errorf("Thinking should be nil but got %+v", result.Thinking)
}
}

// Check max_tokens
if result.MaxTokens != tt.expectedMaxTokens {
t.Errorf("MaxTokens = %d, want %d", result.MaxTokens, tt.expectedMaxTokens)
}

// Check temperature
if result.Temperature != tt.expectedTemperature {
if result.Temperature == nil && tt.expectedTemperature != nil {
t.Errorf("Temperature should be %v but is nil", *tt.expectedTemperature)
} else if result.Temperature != nil && tt.expectedTemperature == nil {
t.Errorf("Temperature should be nil but is %v", *result.Temperature)
} else if result.Temperature != nil && tt.expectedTemperature != nil && *result.Temperature != *tt.expectedTemperature {
t.Errorf("Temperature = %v, want %v", *result.Temperature, *tt.expectedTemperature)
}
}
})
}
}

// TestConvertResponsesRequestToAnthropic_ReasoningEffort tests reasoning in Responses API
func TestConvertResponsesRequestToAnthropic_ReasoningEffort(t *testing.T) {
tests := []struct {
name string
reasoning *core.Reasoning
maxOutputTokens *int
expectedThinking bool
expectedBudget int
expectedMaxTokens int
}{
{
name: "no reasoning",
reasoning: nil,
maxOutputTokens: intPtr(1000),
expectedThinking: false,
expectedMaxTokens: 1000,
},
{
name: "empty effort",
reasoning: &core.Reasoning{Effort: ""},
maxOutputTokens: intPtr(1000),
expectedThinking: false,
expectedMaxTokens: 1000,
},
{
name: "low effort bumps max tokens",
reasoning: &core.Reasoning{Effort: "low"},
maxOutputTokens: intPtr(1000),
expectedThinking: true,
expectedBudget: 5000,
expectedMaxTokens: 5000, // Bumped from 1000
},
{
name: "high effort with sufficient tokens",
reasoning: &core.Reasoning{Effort: "high"},
maxOutputTokens: intPtr(25000),
expectedThinking: true,
expectedBudget: 20000,
expectedMaxTokens: 25000,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
req := &core.ResponsesRequest{
Model: "claude-3-5-sonnet-20241022",
Input: "test input",
MaxOutputTokens: tt.maxOutputTokens,
Reasoning: tt.reasoning,
}

result := convertResponsesRequestToAnthropic(req)

if tt.expectedThinking {
if result.Thinking == nil {
t.Error("Thinking should be enabled but is nil")
} else {
if result.Thinking.BudgetTokens != tt.expectedBudget {
t.Errorf("Thinking.BudgetTokens = %d, want %d", result.Thinking.BudgetTokens, tt.expectedBudget)
}
}
} else {
if result.Thinking != nil {
t.Errorf("Thinking should be nil but got %+v", result.Thinking)
}
}

if result.MaxTokens != tt.expectedMaxTokens {
t.Errorf("MaxTokens = %d, want %d", result.MaxTokens, tt.expectedMaxTokens)
}
})
}
}

// Helper function to create int pointer
func intPtr(i int) *int {
return &i
}