fix: address PR review feedback for thinking support
- Add ThinkingCapable interface for provider capability detection - Warn when thinking_level is set but provider doesn't support it - Warn when temperature is cleared due to thinking enabled - Adjust budget values per Anthropic best practices (medium=16K, xhigh=64K) - Add budget clamp warning and 80% threshold warning - Add parseResponse thinking block tests - Add thinking_level field to config.example.json
This commit is contained in:
parent
1dc7ff753f
commit
6ed8f8cb64
5 changed files with 94 additions and 11 deletions
|
|
@ -6,7 +6,8 @@
|
||||||
"model_name": "gpt4",
|
"model_name": "gpt4",
|
||||||
"max_tokens": 8192,
|
"max_tokens": 8192,
|
||||||
"temperature": 0.7,
|
"temperature": 0.7,
|
||||||
"max_tool_iterations": 20
|
"max_tool_iterations": 20,
|
||||||
|
"thinking_level": ""
|
||||||
}
|
}
|
||||||
},
|
},
|
||||||
"model_list": [
|
"model_list": [
|
||||||
|
|
|
||||||
|
|
@ -779,7 +779,12 @@ func (al *AgentLoop) runLLMIteration(
|
||||||
// parseThinkingLevel guarantees ThinkingOff for empty/unknown values,
|
// parseThinkingLevel guarantees ThinkingOff for empty/unknown values,
|
||||||
// so checking != ThinkingOff is sufficient.
|
// so checking != ThinkingOff is sufficient.
|
||||||
if agent.ThinkingLevel != ThinkingOff {
|
if agent.ThinkingLevel != ThinkingOff {
|
||||||
|
if tc, ok := agent.Provider.(providers.ThinkingCapable); ok && tc.SupportsThinking() {
|
||||||
llmOpts["thinking_level"] = string(agent.ThinkingLevel)
|
llmOpts["thinking_level"] = string(agent.ThinkingLevel)
|
||||||
|
} else {
|
||||||
|
logger.WarnCF("agent", "thinking_level is set but current provider does not support it, ignoring",
|
||||||
|
map[string]any{"agent_id": agent.ID, "thinking_level": string(agent.ThinkingLevel)})
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
callLLM := func() (*providers.LLMResponse, error) {
|
callLLM := func() (*providers.LLMResponse, error) {
|
||||||
|
|
|
||||||
|
|
@ -31,6 +31,9 @@ type Provider struct {
|
||||||
baseURL string
|
baseURL string
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// SupportsThinking implements providers.ThinkingCapable.
|
||||||
|
func (p *Provider) SupportsThinking() bool { return true }
|
||||||
|
|
||||||
func NewProvider(token string) *Provider {
|
func NewProvider(token string) *Provider {
|
||||||
return NewProviderWithBaseURL(token, "")
|
return NewProviderWithBaseURL(token, "")
|
||||||
}
|
}
|
||||||
|
|
@ -202,6 +205,9 @@ func buildParams(
|
||||||
func applyThinkingConfig(params *anthropic.MessageNewParams, level string) {
|
func applyThinkingConfig(params *anthropic.MessageNewParams, level string) {
|
||||||
// Anthropic API rejects requests with temperature set alongside thinking.
|
// Anthropic API rejects requests with temperature set alongside thinking.
|
||||||
// Reset to zero value (omitted from JSON serialization).
|
// Reset to zero value (omitted from JSON serialization).
|
||||||
|
if params.Temperature.Valid() {
|
||||||
|
log.Printf("anthropic: temperature cleared because thinking is enabled (level=%s)", level)
|
||||||
|
}
|
||||||
params.Temperature = anthropic.MessageNewParams{}.Temperature
|
params.Temperature = anthropic.MessageNewParams{}.Temperature
|
||||||
|
|
||||||
if level == "adaptive" {
|
if level == "adaptive" {
|
||||||
|
|
@ -220,22 +226,34 @@ func applyThinkingConfig(params *anthropic.MessageNewParams, level string) {
|
||||||
|
|
||||||
// budget_tokens must be < max_tokens; clamp to respect user's max_tokens setting.
|
// budget_tokens must be < max_tokens; clamp to respect user's max_tokens setting.
|
||||||
if budget >= params.MaxTokens {
|
if budget >= params.MaxTokens {
|
||||||
|
log.Printf("anthropic: budget_tokens (%d) clamped to %d (max_tokens-1)", budget, params.MaxTokens-1)
|
||||||
budget = params.MaxTokens - 1
|
budget = params.MaxTokens - 1
|
||||||
|
} else if budget > params.MaxTokens*80/100 {
|
||||||
|
log.Printf("anthropic: thinking budget (%d) exceeds 80%% of max_tokens (%d), output may be truncated",
|
||||||
|
budget, params.MaxTokens)
|
||||||
}
|
}
|
||||||
params.Thinking = anthropic.ThinkingConfigParamOfEnabled(budget)
|
params.Thinking = anthropic.ThinkingConfigParamOfEnabled(budget)
|
||||||
}
|
}
|
||||||
|
|
||||||
// levelToBudget maps a thinking level string to budget_tokens for legacy models.
|
// levelToBudget maps a thinking level to budget_tokens.
|
||||||
|
// Values are based on Anthropic's recommendations and community best practices:
|
||||||
|
//
|
||||||
|
// low = 4,096 — simple reasoning, quick debugging (Claude Code "think")
|
||||||
|
// medium = 16,384 — Anthropic recommended sweet spot for most tasks
|
||||||
|
// high = 32,000 — complex architecture, deep analysis (diminishing returns above this)
|
||||||
|
// xhigh = 64,000 — extreme reasoning, research problems, benchmarks
|
||||||
|
//
|
||||||
|
// Note: For Claude 4.6+, prefer adaptive thinking over manual budget_tokens.
|
||||||
func levelToBudget(level string) int {
|
func levelToBudget(level string) int {
|
||||||
switch level {
|
switch level {
|
||||||
case "low":
|
case "low":
|
||||||
return 4096
|
return 4096
|
||||||
case "medium":
|
case "medium":
|
||||||
return 10000
|
return 16384
|
||||||
case "high":
|
case "high":
|
||||||
return 32000
|
return 32000
|
||||||
case "xhigh":
|
case "xhigh":
|
||||||
return 128000
|
return 64000
|
||||||
default:
|
default:
|
||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -1,6 +1,7 @@
|
||||||
package anthropicprovider
|
package anthropicprovider
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"encoding/json"
|
||||||
"testing"
|
"testing"
|
||||||
|
|
||||||
"github.com/anthropics/anthropic-sdk-go"
|
"github.com/anthropics/anthropic-sdk-go"
|
||||||
|
|
@ -33,9 +34,9 @@ func TestApplyThinkingConfig_BudgetLevels(t *testing.T) {
|
||||||
wantBudget int64
|
wantBudget int64
|
||||||
}{
|
}{
|
||||||
{"low", 4096},
|
{"low", 4096},
|
||||||
{"medium", 10000},
|
{"medium", 16384},
|
||||||
{"high", 32000},
|
{"high", 32000},
|
||||||
{"xhigh", 128000},
|
{"xhigh", 64000},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|
@ -100,9 +101,9 @@ func TestLevelToBudget(t *testing.T) {
|
||||||
want int
|
want int
|
||||||
}{
|
}{
|
||||||
{"low", "low", 4096},
|
{"low", "low", 4096},
|
||||||
{"medium", "medium", 10000},
|
{"medium", "medium", 16384},
|
||||||
{"high", "high", 32000},
|
{"high", "high", 32000},
|
||||||
{"xhigh", "xhigh", 128000},
|
{"xhigh", "xhigh", 64000},
|
||||||
{"off", "off", 0},
|
{"off", "off", 0},
|
||||||
{"empty", "", 0},
|
{"empty", "", 0},
|
||||||
}
|
}
|
||||||
|
|
@ -135,8 +136,59 @@ func TestBuildParams_ThinkingClearsTemperature(t *testing.T) {
|
||||||
if params.Thinking.OfEnabled == nil {
|
if params.Thinking.OfEnabled == nil {
|
||||||
t.Fatal("expected enabled thinking")
|
t.Fatal("expected enabled thinking")
|
||||||
}
|
}
|
||||||
if params.Thinking.OfEnabled.BudgetTokens != 10000 {
|
if params.Thinking.OfEnabled.BudgetTokens != 16384 {
|
||||||
t.Errorf("budget_tokens = %d, want 10000", params.Thinking.OfEnabled.BudgetTokens)
|
t.Errorf("budget_tokens = %d, want 16384", params.Thinking.OfEnabled.BudgetTokens)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// unmarshalBlocks constructs []ContentBlockUnion via JSON round-trip so that
|
||||||
|
// the internal JSON.raw field is populated (required by AsText/AsThinking).
|
||||||
|
func unmarshalBlocks(t *testing.T, jsonStr string) []anthropic.ContentBlockUnion {
|
||||||
|
t.Helper()
|
||||||
|
var blocks []anthropic.ContentBlockUnion
|
||||||
|
if err := json.Unmarshal([]byte(jsonStr), &blocks); err != nil {
|
||||||
|
t.Fatalf("unmarshalBlocks: %v", err)
|
||||||
|
}
|
||||||
|
return blocks
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseResponse_ThinkingBlock(t *testing.T) {
|
||||||
|
resp := &anthropic.Message{
|
||||||
|
Content: unmarshalBlocks(t, `[
|
||||||
|
{"type":"thinking","thinking":"Let me reason step by step...","signature":"sig"},
|
||||||
|
{"type":"text","text":"The answer is 42."}
|
||||||
|
]`),
|
||||||
|
StopReason: anthropic.StopReasonEndTurn,
|
||||||
|
}
|
||||||
|
|
||||||
|
result := parseResponse(resp)
|
||||||
|
|
||||||
|
if result.Reasoning != "Let me reason step by step..." {
|
||||||
|
t.Errorf("Reasoning = %q, want thinking content", result.Reasoning)
|
||||||
|
}
|
||||||
|
if result.Content != "The answer is 42." {
|
||||||
|
t.Errorf("Content = %q, want text content", result.Content)
|
||||||
|
}
|
||||||
|
if result.FinishReason != "stop" {
|
||||||
|
t.Errorf("FinishReason = %q, want stop", result.FinishReason)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
func TestParseResponse_NoThinkingBlock(t *testing.T) {
|
||||||
|
resp := &anthropic.Message{
|
||||||
|
Content: unmarshalBlocks(t, `[
|
||||||
|
{"type":"text","text":"Just a normal response."}
|
||||||
|
]`),
|
||||||
|
StopReason: anthropic.StopReasonEndTurn,
|
||||||
|
}
|
||||||
|
|
||||||
|
result := parseResponse(resp)
|
||||||
|
|
||||||
|
if result.Reasoning != "" {
|
||||||
|
t.Errorf("Reasoning = %q, want empty", result.Reasoning)
|
||||||
|
}
|
||||||
|
if result.Content != "Just a normal response." {
|
||||||
|
t.Errorf("Content = %q, want text content", result.Content)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -37,6 +37,13 @@ type StatefulProvider interface {
|
||||||
Close()
|
Close()
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ThinkingCapable is an optional interface for providers that support
|
||||||
|
// extended thinking (e.g. Anthropic). Used by the agent loop to warn
|
||||||
|
// when thinking_level is configured but the active provider cannot use it.
|
||||||
|
type ThinkingCapable interface {
|
||||||
|
SupportsThinking() bool
|
||||||
|
}
|
||||||
|
|
||||||
// FailoverReason classifies why an LLM request failed for fallback decisions.
|
// FailoverReason classifies why an LLM request failed for fallback decisions.
|
||||||
type FailoverReason string
|
type FailoverReason string
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Add table
Reference in a new issue