Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
11 changes: 10 additions & 1 deletion internal/llm/client.go
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,9 @@ type ClientConfig struct {
Timeout time.Duration // Request timeout
ExtraBody map[string]any // Vendor-specific fields merged into every request body
ExtraHeaders map[string]string // Extra HTTP headers sent with every request
// LegacyMaxTokens sends the legacy `max_tokens` request field instead of
// `max_completion_tokens` (required by Gemini's OpenAI-compatible endpoint).
LegacyMaxTokens bool
}

// --- Factory ---
Expand All @@ -201,6 +204,8 @@ func NewLLMClient(ep ResolvedEndpoint) LLMClient {
Timeout: ep.Timeout,
ExtraBody: ep.ExtraBody,
ExtraHeaders: ep.ExtraHeaders,

LegacyMaxTokens: ep.LegacyMaxTokens,
}
if ep.Protocol == "anthropic" {
return NewAnthropicClient(cfg)
Expand Down Expand Up @@ -400,7 +405,11 @@ func (c *OpenAIClient) buildOpenAIParams(model string, req ChatRequest) openai.C
params.Tools = tools
}
if req.MaxTokens > 0 {
params.MaxCompletionTokens = openai.Int(int64(req.MaxTokens))
if c.cfg.LegacyMaxTokens {
params.MaxTokens = openai.Int(int64(req.MaxTokens))
} else {
params.MaxCompletionTokens = openai.Int(int64(req.MaxTokens))
}
}
if req.Temperature != nil {
params.Temperature = openai.Float(*req.Temperature)
Expand Down
86 changes: 86 additions & 0 deletions internal/llm/client_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,92 @@ func TestNewAnthropicClient_URLNormalization(t *testing.T) {
}
}

func TestBuildOpenAIParams_LegacyMaxTokens(t *testing.T) {
tests := []struct {
name string
legacyMaxTokens bool
maxTokens int
}{
{name: "legacy sends max_tokens", legacyMaxTokens: true, maxTokens: 1000},
{name: "modern sends max_completion_tokens", legacyMaxTokens: false, maxTokens: 1000},
// MaxTokens==0 must leave BOTH fields unset (gates the `req.MaxTokens > 0` guard),
// regardless of the legacy flag.
{name: "legacy with zero max_tokens sets neither", legacyMaxTokens: true, maxTokens: 0},
{name: "modern with zero max_tokens sets neither", legacyMaxTokens: false, maxTokens: 0},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := NewOpenAIClient(ClientConfig{
URL: "https://api.example.com/v1",
LegacyMaxTokens: tt.legacyMaxTokens,
})

params := client.buildOpenAIParams("some-model", ChatRequest{
Messages: []Message{{Role: "user", Content: "hi"}},
MaxTokens: tt.maxTokens,
})

if tt.maxTokens == 0 {
if params.MaxTokens.Valid() {
t.Errorf("MaxTokens = %+v, want unset when MaxTokens==0", params.MaxTokens.Value)
}
if params.MaxCompletionTokens.Valid() {
t.Errorf("MaxCompletionTokens = %+v, want unset when MaxTokens==0", params.MaxCompletionTokens.Value)
}
return
}

if tt.legacyMaxTokens {
if !params.MaxTokens.Valid() || params.MaxTokens.Value != int64(tt.maxTokens) {
t.Errorf("MaxTokens = %+v (valid=%v), want %d", params.MaxTokens.Value, params.MaxTokens.Valid(), tt.maxTokens)
}
if params.MaxCompletionTokens.Valid() {
t.Errorf("MaxCompletionTokens = %+v, want unset", params.MaxCompletionTokens.Value)
}
} else {
if !params.MaxCompletionTokens.Valid() || params.MaxCompletionTokens.Value != int64(tt.maxTokens) {
t.Errorf("MaxCompletionTokens = %+v (valid=%v), want %d", params.MaxCompletionTokens.Value, params.MaxCompletionTokens.Valid(), tt.maxTokens)
}
if params.MaxTokens.Valid() {
t.Errorf("MaxTokens = %+v, want unset", params.MaxTokens.Value)
}
}
})
}
}

// TestNewLLMClient_LegacyMaxTokensForwarded guards the ResolvedEndpoint -> ClientConfig
// hop: a dropped assignment in NewLLMClient would silently send max_completion_tokens
// to a Gemini endpoint (400). client_test.go is in-package, so we can read cfg directly.
func TestNewLLMClient_LegacyMaxTokensForwarded(t *testing.T) {
tests := []struct {
name string
want bool
}{
{name: "legacy true forwarded", want: true},
{name: "legacy false forwarded", want: false},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
client := NewLLMClient(ResolvedEndpoint{
URL: "https://api.example.com/v1",
Token: "test-token",
Model: "test-model",
Protocol: "openai",
LegacyMaxTokens: tt.want,
})
oc, ok := client.(*OpenAIClient)
if !ok {
t.Fatalf("expected *OpenAIClient, got %T", client)
}
if oc.cfg.LegacyMaxTokens != tt.want {
t.Errorf("OpenAIClient cfg.LegacyMaxTokens = %v, want %v", oc.cfg.LegacyMaxTokens, tt.want)
}
})
}
}

func TestBuildAnthropicParams_CacheControl(t *testing.T) {
client := NewAnthropicClient(ClientConfig{URL: "https://api.anthropic.com"})

Expand Down
19 changes: 19 additions & 0 deletions internal/llm/providers.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ type Provider struct {
AuthHeader string // Anthropic-only; empty for OpenAI-compatible
EnvVar string // environment variable name for API key fallback
Models []string

// LegacyMaxTokens forces the legacy `max_tokens` request field instead of
// `max_completion_tokens` for OpenAI-compatible endpoints that require it.
LegacyMaxTokens bool
}

var registry = []Provider{
Expand Down Expand Up @@ -109,6 +113,21 @@ var registry = []Provider{
"deepseek-v4-flash",
},
},
{
Name: "gemini",
DisplayName: "Google Gemini API",
Protocol: "openai",
BaseURL: "https://generativelanguage.googleapis.com/v1beta/openai",
EnvVar: "GEMINI_API_KEY",
LegacyMaxTokens: true,
Models: []string{
// Gemini 3.x -preview models 400 on OCR's multi-turn tool-calling review
// path (thought_signature must be echoed back, which the OpenAI-compat client
// does not yet do). Ship only the GA 2.5 models, verified end-to-end.
"gemini-2.5-flash",
"gemini-2.5-pro",
},
},
{
Name: "tencent-tokenhub",
DisplayName: "Tencent TokenHub API",
Expand Down
33 changes: 32 additions & 1 deletion internal/llm/providers_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ func TestListProviders_Order(t *testing.T) {
if len(providers) < 3 {
t.Fatalf("expected at least 3 providers, got %d", len(providers))
}
expected := []string{"anthropic", "baidu-qianfan", "dashscope", "dashscope-tokenplan", "deepseek", "hy-tokenplan", "kimi", "mimo", "minimax", "openai", "tencent-tokenhub", "volcengine", "z-ai", "z-ai-coding"}
expected := []string{"anthropic", "baidu-qianfan", "dashscope", "dashscope-tokenplan", "deepseek", "gemini", "hy-tokenplan", "kimi", "mimo", "minimax", "openai", "tencent-tokenhub", "volcengine", "z-ai", "z-ai-coding"}
if len(providers) != len(expected) {
t.Fatalf("expected %d providers, got %d", len(expected), len(providers))
}
Expand Down Expand Up @@ -126,3 +126,34 @@ func TestLookupProvider_OpenAIDetails(t *testing.T) {
t.Errorf("AuthHeader = %q, want empty", p.AuthHeader)
}
}

func TestLookupProvider_GeminiDetails(t *testing.T) {
p, ok := LookupProvider("gemini")
if !ok {
t.Fatal("gemini not found")
}
if p.Protocol != "openai" {
t.Errorf("Protocol = %q, want %q", p.Protocol, "openai")
}
if p.BaseURL != "https://generativelanguage.googleapis.com/v1beta/openai" {
t.Errorf("BaseURL = %q, want %q", p.BaseURL, "https://generativelanguage.googleapis.com/v1beta/openai")
}
if p.EnvVar != "GEMINI_API_KEY" {
t.Errorf("EnvVar = %q, want %q", p.EnvVar, "GEMINI_API_KEY")
}
if p.AuthHeader != "" {
t.Errorf("AuthHeader = %q, want empty", p.AuthHeader)
}
if !p.LegacyMaxTokens {
t.Errorf("LegacyMaxTokens = %v, want true", p.LegacyMaxTokens)
}
expected := []string{"gemini-2.5-flash", "gemini-2.5-pro"}
if len(p.Models) != len(expected) {
t.Fatalf("expected %d models, got %d", len(expected), len(p.Models))
}
for i, model := range expected {
if p.Models[i] != model {
t.Errorf("Models[%d] = %q, want %q", i, p.Models[i], model)
}
}
}
24 changes: 15 additions & 9 deletions internal/llm/resolver.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,9 @@ type ResolvedEndpoint struct {
Source string // human-readable config source label
ExtraBody map[string]any // vendor-specific request body fields
ExtraHeaders map[string]string // extra HTTP headers for the LLM request
// LegacyMaxTokens sends the legacy `max_tokens` request field instead of
// `max_completion_tokens`. Set from a provider preset (e.g. Gemini).
LegacyMaxTokens bool
// Timeout is the per-request HTTP timeout; 0 means use the client default (5 min).
// Only config file (llm/provider sections) and OCR_LLM_TIMEOUT env var can set this.
// tryCCEnv and tryShellRC always leave it at 0 since those sources have no timeout
Expand Down Expand Up @@ -271,11 +274,13 @@ func tryProviderConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,

var url, protocol, authHeader, model string
var extraBody map[string]any
var legacyMaxTokens bool

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Currently, LegacyMaxTokens is only set when resolving a preset provider (where isPreset is true). However, custom providers configured via custom_providers or the legacy llm block might also connect to OpenAI-compatible endpoints that require the legacy max_tokens parameter instead of max_completion_tokens. Consider adding a LegacyMaxTokens field (perhaps as a *bool to distinguish unset from false) to providerEntryConfig and llmFileConfig in a future update so that custom providers can also opt into using legacy max tokens if needed.


if isPreset {
url = preset.BaseURL
protocol = preset.Protocol
authHeader = preset.AuthHeader
legacyMaxTokens = preset.LegacyMaxTokens
if entry.URL != "" {
url = entry.URL
}
Expand Down Expand Up @@ -360,15 +365,16 @@ func tryProviderConfig(cfg configFile, modelOverride string) (ResolvedEndpoint,
}

return ResolvedEndpoint{
URL: url,
Token: apiKey,
Model: model,
Protocol: protocol,
AuthHeader: authHeader,
Source: "provider:" + cfg.Provider,
ExtraBody: extraBody,
ExtraHeaders: extraHeaders,
Timeout: timeout,
URL: url,
Token: apiKey,
Model: model,
Protocol: protocol,
AuthHeader: authHeader,
Source: "provider:" + cfg.Provider,
ExtraBody: extraBody,
ExtraHeaders: extraHeaders,
Timeout: timeout,
LegacyMaxTokens: legacyMaxTokens,
}, true, nil
}

Expand Down
68 changes: 68 additions & 0 deletions internal/llm/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -334,6 +334,74 @@ func TestResolveEndpoint_ProviderOpenAI(t *testing.T) {
}
}

// TestResolveEndpoint_ProviderLegacyMaxTokens guards the preset -> ResolvedEndpoint hop
// through the same public path real usage takes: resolving provider:gemini must yield
// LegacyMaxTokens=true, while presets/custom providers that don't opt in stay false.
// A dropped assignment here would keep all other tests green while Gemini users silently
// send max_completion_tokens and get 400s.
func TestResolveEndpoint_ProviderLegacyMaxTokens(t *testing.T) {
tests := []struct {
name string
cfg configFile
want bool
}{
{
name: "gemini preset resolves with legacy max_tokens",
cfg: configFile{
Provider: "gemini",
Providers: map[string]providerEntryConfig{
"gemini": {APIKey: "gemini-key", Model: "gemini-2.5-flash"},
},
},
want: true,
},
{
name: "anthropic preset does not set legacy max_tokens",
cfg: configFile{
Provider: "anthropic",
Providers: map[string]providerEntryConfig{
"anthropic": {APIKey: "sk-ant-test", Model: "claude-sonnet-4-6"},
},
},
want: false,
},
{
name: "custom provider does not set legacy max_tokens",
cfg: configFile{
Provider: "my-gateway",
CustomProviders: map[string]providerEntryConfig{
"my-gateway": {
APIKey: "token",
URL: "https://gateway.internal.com/v1",
Protocol: "openai",
Model: "llama-3-70b",
},
},
},
want: false,
},
}

for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
clearAllEnv(t)
data, _ := json.Marshal(tt.cfg)
cfgPath := filepath.Join(t.TempDir(), "config.json")
if err := os.WriteFile(cfgPath, data, 0644); err != nil {
t.Fatalf("write config: %v", err)
}

ep, err := ResolveEndpoint(cfgPath)
if err != nil {
t.Fatalf("unexpected error: %v", err)
}
if ep.LegacyMaxTokens != tt.want {
t.Errorf("LegacyMaxTokens = %v, want %v", ep.LegacyMaxTokens, tt.want)
}
})
}
}

func TestResolveEndpoint_ProviderModelOverride(t *testing.T) {
clearAllEnv(t)

Expand Down
1 change: 1 addition & 0 deletions pages/src/content/docs/en/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ environment variable.
| `dashscope-tokenplan` | openai | `https://token-plan.cn-beijing.maas.aliyuncs.com/compatible-mode/v1` | `DASHSCOPE_TOKENPLAN_KEY` |
| `volcengine` | openai | `https://ark.cn-beijing.volces.com/api/v3` | `ARK_API_KEY` |
| `deepseek` | openai | `https://api.deepseek.com` | `DEEPSEEK_API_KEY` |
| `gemini` | openai | `https://generativelanguage.googleapis.com/v1beta/openai` | `GEMINI_API_KEY` |
| `tencent-tokenhub` | openai | `https://tokenhub.tencentmaas.com/v1` | `TENCENT_TOKENHUB_API_KEY` |
| `hy-tokenplan` | openai | `https://api.lkeap.cloud.tencent.com/plan/v3` | `TENCENT_HUNYUAN_TOKENPLAN_KEY` |
| `kimi` | openai | `https://api.moonshot.cn/v1` | `MOONSHOT_API_KEY` |
Expand Down
1 change: 1 addition & 0 deletions pages/src/content/docs/ja/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ ocr config set providers.anthropic.api_key sk-ant-xxxxxxxxxx
| `dashscope-tokenplan` | openai | `https://token-plan.cn-beijing.maas.aliyuncs.com/compatible-mode/v1` | `DASHSCOPE_TOKENPLAN_KEY` |
| `volcengine` | openai | `https://ark.cn-beijing.volces.com/api/v3` | `ARK_API_KEY` |
| `deepseek` | openai | `https://api.deepseek.com` | `DEEPSEEK_API_KEY` |
| `gemini` | openai | `https://generativelanguage.googleapis.com/v1beta/openai` | `GEMINI_API_KEY` |
| `tencent-tokenhub` | openai | `https://tokenhub.tencentmaas.com/v1` | `TENCENT_TOKENHUB_API_KEY` |
| `hy-tokenplan` | openai | `https://api.lkeap.cloud.tencent.com/plan/v3` | `TENCENT_HUNYUAN_TOKENPLAN_KEY` |
| `kimi` | openai | `https://api.moonshot.cn/v1` | `MOONSHOT_API_KEY` |
Expand Down
1 change: 1 addition & 0 deletions pages/src/content/docs/zh/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ ocr config set providers.anthropic.api_key sk-ant-xxxxxxxxxx
| `dashscope-tokenplan` | openai | `https://token-plan.cn-beijing.maas.aliyuncs.com/compatible-mode/v1` | `DASHSCOPE_TOKENPLAN_KEY` |
| `volcengine` | openai | `https://ark.cn-beijing.volces.com/api/v3` | `ARK_API_KEY` |
| `deepseek` | openai | `https://api.deepseek.com` | `DEEPSEEK_API_KEY` |
| `gemini` | openai | `https://generativelanguage.googleapis.com/v1beta/openai` | `GEMINI_API_KEY` |
| `tencent-tokenhub` | openai | `https://tokenhub.tencentmaas.com/v1` | `TENCENT_TOKENHUB_API_KEY` |
| `hy-tokenplan` | openai | `https://api.lkeap.cloud.tencent.com/plan/v3` | `TENCENT_HUNYUAN_TOKENPLAN_KEY` |
| `kimi` | openai | `https://api.moonshot.cn/v1` | `MOONSHOT_API_KEY` |
Expand Down