diff --git a/familiar-gateway/internal/classifier/effort.go b/familiar-gateway/internal/classifier/effort.go index ecdf056..b3b303c 100644 --- a/familiar-gateway/internal/classifier/effort.go +++ b/familiar-gateway/internal/classifier/effort.go @@ -14,6 +14,14 @@ package classifier type ThinkingBudget struct { Enabled bool TokenBudget int + // Effort is the backend's native thinking-depth selector (Qwen 3.8: + // low/medium/xhigh). Empty leaves it unset so the server default + // applies. Unlike TokenBudget — which only grants headroom and is + // deliberately NOT allowed to bound the trusted path (see the comment + // at the thinkingHeadroom assignment in pipeline.go) — this tells the + // model to think less, which is safe: it cannot truncate a thought + // mid-stream, it just produces a shorter one. + Effort string } // MemoryBudget is the resolved retrieval shape for one MemoryDepth. @@ -52,9 +60,13 @@ func DefaultResolver() *EffortResolver { return &EffortResolver{ Thinking: map[ThinkingLevel]ThinkingBudget{ ThinkingOff: {Enabled: false}, - ThinkingLow: {Enabled: true, TokenBudget: 500}, - ThinkingMedium: {Enabled: true, TokenBudget: 2000}, - ThinkingHigh: {Enabled: true, TokenBudget: 8000}, + ThinkingLow: {Enabled: true, TokenBudget: 500, Effort: "low"}, + ThinkingMedium: {Enabled: true, TokenBudget: 2000, Effort: "medium"}, + // High maps to medium, not xhigh, on purpose. The level is a small + // model's one-shot guess, and xhigh is a large jump in latency for + // a guess to be spending. Operators can raise it per-level via + // [effort.thinking.high] effort = "xhigh". + ThinkingHigh: {Enabled: true, TokenBudget: 8000, Effort: "medium"}, }, Memory: map[MemoryDepth]MemoryBudget{ MemoryNone: {Skip: true}, diff --git a/familiar-gateway/internal/classifier/from_config.go b/familiar-gateway/internal/classifier/from_config.go index a7212b5..b339a93 100644 --- a/familiar-gateway/internal/classifier/from_config.go +++ b/familiar-gateway/internal/classifier/from_config.go @@ -33,6 +33,9 @@ func ResolverFromConfig(cfg config.EffortConfig) *EffortResolver { if src.TokenBudget > 0 { merged.TokenBudget = src.TokenBudget } + if src.Effort != "" { + merged.Effort = src.Effort + } r.Thinking[level] = merged } diff --git a/familiar-gateway/internal/config/config.go b/familiar-gateway/internal/config/config.go index 59e610b..f2799fc 100644 --- a/familiar-gateway/internal/config/config.go +++ b/familiar-gateway/internal/config/config.go @@ -113,6 +113,10 @@ type EffortThinkingConfig struct { type EffortThinkingLevel struct { Enabled *bool `toml:"enabled"` // nil = use level-default TokenBudget int `toml:"token_budget"` + // Effort overrides the backend's native thinking depth for this level + // (e.g. "low", "medium", "xhigh" on Qwen 3.8). Empty = use the + // level-default; set it to "" explicitly nowhere — omit the key instead. + Effort string `toml:"effort"` } // EffortMemoryDepthConfig — retrieval knobs per MemoryDepth. diff --git a/familiar-gateway/internal/llm/openai.go b/familiar-gateway/internal/llm/openai.go index 1c337ea..7b125bd 100644 --- a/familiar-gateway/internal/llm/openai.go +++ b/familiar-gateway/internal/llm/openai.go @@ -12,6 +12,26 @@ import ( "time" ) +// thinkingKwargs builds the chat_template_kwargs payload. +// +// reasoning_effort MUST travel inside chat_template_kwargs, not as a +// top-level request field. Measured against rune (Qwen 3.8 on llama.cpp) with +// one fixed prompt: as a top-level field, low/medium/xhigh all produced ~420 +// characters of reasoning — silently ignored. Inside chat_template_kwargs the +// same prompt gave 430 chars at low and 991 at xhigh. The server's +// --reasoning-effort launch flag sets the default; this overrides it per +// request. +// +// An empty Effort omits the key entirely, so the server default still applies +// and backends without an effort dial are unaffected. +func thinkingKwargs(req CompletionRequest) map[string]any { + kw := map[string]any{"enable_thinking": req.EnableThinking} + if req.EnableThinking && req.ReasoningEffort != "" { + kw["reasoning_effort"] = req.ReasoningEffort + } + return kw +} + // OpenAIProvider implements Provider for OpenAI-compatible endpoints // (llama-server, Ollama, vLLM, etc.). type OpenAIProvider struct { @@ -322,7 +342,7 @@ func (p *OpenAIProvider) Complete(ctx context.Context, req CompletionRequest) (* // to Qwen3.5-122B, hardcoding true forced thinking on everywhere, // which broke tier 3's "fast structured output, no thinking" // contract. Respect the request field now. See BUGS.md Bug 2. - ChatTemplateKwargs: map[string]any{"enable_thinking": req.EnableThinking}, + ChatTemplateKwargs: thinkingKwargs(req), } if req.ToolChoice != "" { body.ToolChoice = req.ToolChoice @@ -417,7 +437,7 @@ func (p *OpenAIProvider) CompleteStream(ctx context.Context, req CompletionReque Tools: tools, // See Complete() for why this respects req.EnableThinking rather // than forcing true. Same rationale applies for streaming. - ChatTemplateKwargs: map[string]any{"enable_thinking": req.EnableThinking}, + ChatTemplateKwargs: thinkingKwargs(req), } if req.ToolChoice != "" { body.ToolChoice = req.ToolChoice diff --git a/familiar-gateway/internal/llm/openai_test.go b/familiar-gateway/internal/llm/openai_test.go index b655cbf..fccc180 100644 --- a/familiar-gateway/internal/llm/openai_test.go +++ b/familiar-gateway/internal/llm/openai_test.go @@ -476,3 +476,80 @@ func TestOpenAIProviderCompleteStream_UsageWinsOverPredictedN(t *testing.T) { t.Errorf("DecodeMs = %v, want 4000", resp.DecodeMs) } } + +// TestOpenAIComplete_ReasoningEffort pins the TRANSPORT of the effort knob, +// which is the part that fails invisibly. +// +// Measured against rune (Qwen 3.8 on llama.cpp): a TOP-LEVEL "reasoning_effort" +// request field is silently ignored — low/medium/xhigh all produced ~420 chars +// of reasoning on one fixed prompt. Inside chat_template_kwargs the same prompt +// gave 430 chars at low and 991 at xhigh. A change moving this back to a +// top-level field would break nothing at runtime; it would just quietly stop +// working. Hence this test. +// +// Also pins the guard: effort only rides along when thinking is on, so a +// backend without an effort dial (MLX/Gemma on the tier-4 path) keeps +// receiving exactly what it received before. Verified live that MLX returns +// HTTP 200 and ignores the key rather than erroring. +func TestOpenAIComplete_ReasoningEffort(t *testing.T) { + capture := func(t *testing.T, req CompletionRequest) map[string]any { + t.Helper() + var body struct { + ChatTemplateKwargs map[string]any `json:"chat_template_kwargs"` + ReasoningEffort string `json:"reasoning_effort"` + } + srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + _ = json.NewDecoder(r.Body).Decode(&body) + _, _ = w.Write([]byte(`{"choices":[{"message":{"content":"ok"}}]}`)) + })) + defer srv.Close() + + p := NewOpenAIProvider("test", srv.URL, "") + if _, err := p.Complete(context.Background(), req); err != nil { + t.Fatalf("Complete: %v", err) + } + if body.ReasoningEffort != "" { + t.Errorf("reasoning_effort must NOT be a top-level field (the server ignores it there), got %q", + body.ReasoningEffort) + } + return body.ChatTemplateKwargs + } + + t.Run("forwarded inside chat_template_kwargs when thinking is on", func(t *testing.T) { + kw := capture(t, CompletionRequest{ + Model: "m", + Messages: []Message{{Role: "user", Content: "hi"}}, + EnableThinking: true, + ReasoningEffort: "low", + }) + if got, ok := kw["reasoning_effort"]; !ok || got != "low" { + t.Errorf("expected chat_template_kwargs.reasoning_effort=low, got %+v", kw) + } + }) + + t.Run("omitted when thinking is off", func(t *testing.T) { + kw := capture(t, CompletionRequest{ + Model: "m", + Messages: []Message{{Role: "user", Content: "hi"}}, + EnableThinking: false, + ReasoningEffort: "xhigh", + }) + if _, ok := kw["reasoning_effort"]; ok { + t.Errorf("reasoning_effort must not be sent when thinking is off, got %+v", kw) + } + }) + + t.Run("omitted when unset so the server default applies", func(t *testing.T) { + kw := capture(t, CompletionRequest{ + Model: "m", + Messages: []Message{{Role: "user", Content: "hi"}}, + EnableThinking: true, + }) + if _, ok := kw["reasoning_effort"]; ok { + t.Errorf("reasoning_effort must be omitted when unset, got %+v", kw) + } + if got, ok := kw["enable_thinking"]; !ok || got != true { + t.Errorf("enable_thinking must still be forwarded, got %+v", kw) + } + }) +} diff --git a/familiar-gateway/internal/llm/provider.go b/familiar-gateway/internal/llm/provider.go index 82a1a78..f3dd2a0 100644 --- a/familiar-gateway/internal/llm/provider.go +++ b/familiar-gateway/internal/llm/provider.go @@ -61,6 +61,16 @@ type CompletionRequest struct { MaxTokens int Stream bool EnableThinking bool + // ReasoningEffort selects the model's native thinking depth when the + // backend supports it (Qwen 3.8 exposes low/medium/xhigh). Empty means + // "do not send it" — the server's launch default then applies, which is + // what happened for every turn before this existed. + // + // This is the knob EnableThinking/MaxThinkingTokens could not provide. + // MaxThinkingTokens only ever widened max_tokens headroom; it never told + // the model to think less, so a trivial question still reasoned at + // whatever depth the server was started with. + ReasoningEffort string // MaxThinkingTokens is a soft budget for the model's reasoning tokens. // Zero means "no explicit budget" — providers pick their own default. // Providers that don't expose a thinking budget ignore this field. diff --git a/familiar-gateway/internal/pipeline/pipeline.go b/familiar-gateway/internal/pipeline/pipeline.go index c089ca9..cc2a3fe 100644 --- a/familiar-gateway/internal/pipeline/pipeline.go +++ b/familiar-gateway/internal/pipeline/pipeline.go @@ -2502,7 +2502,11 @@ func (p *Pipeline) buildLLMRequest(messages []llm.Message, route *routeResult, i // headroom we really granted rather than the level's nominal // budget, for whenever a provider does gain a separate knob. MaxThinkingTokens: thinkingHeadroom, - OnReasoningChunk: onReasoningChunk, + // The classifier's level now actually reaches the model. Previously + // it only widened max_tokens headroom, so every turn reasoned at the + // server's launch default regardless of how trivial the question was. + ReasoningEffort: thinkingBudget.Effort, + OnReasoningChunk: onReasoningChunk, } if overrides != nil { req.Temperature = overrides.Temperature