From a44dbf2ba49118534f7a5746a6cae9d9a8dd3ef5 Mon Sep 17 00:00:00 2001 From: hoangvinh14 Date: Mon, 16 Mar 2026 07:43:08 +0700 Subject: [PATCH] feat(providers/dashscope): add Qwen 3.5 series support with per-model thinking capability (#215) * feat(providers/dashscope): guard enable_thinking injection by per-model capability check Introduces ModelThinkingCapable interface and ModelSupportsThinking field on ChatRequest so DashScope can skip thinking-param injection for models that do not support it (e.g. qwen3-plus, qwen3-turbo), preventing \"model not supported\" API errors. - types.go: add ModelThinkingCapable interface + ModelSupportsThinking *bool on ChatRequest - dashscope.go: add dashscopeThinkingModels whitelist + ModelSupportsThinking(); honour pre-computed hint - loop.go: detect ModelThinkingCapable and set hint on ChatRequest before LLM call - provider_models.go: add qwen3.5-plus / qwen3.5-turbo to DashScope model list - dashscope_test.go: full test suite for whitelist, injection, hint override, budget mapping * Fix review code. --------- Co-authored-by: Nguyen Gia Hoang Vinh --- internal/agent/loop.go | 7 ++ internal/http/provider_models.go | 5 +- internal/providers/dashscope.go | 66 +++++++++- internal/providers/dashscope_test.go | 178 +++++++++++++++++++++++++++ internal/providers/types.go | 13 ++ 5 files changed, 262 insertions(+), 7 deletions(-) create mode 100644 internal/providers/dashscope_test.go diff --git a/internal/agent/loop.go b/internal/agent/loop.go index 7c3151a8..b05113aa 100644 --- a/internal/agent/loop.go +++ b/internal/agent/loop.go @@ -479,6 +479,13 @@ func (l *Loop) runLoop(ctx context.Context, req RunRequest) (*RunResult, error) if l.thinkingLevel != "" && l.thinkingLevel != "off" { if tc, ok := l.provider.(providers.ThinkingCapable); ok && tc.SupportsThinking() { chatReq.Options[providers.OptThinkingLevel] = l.thinkingLevel + // For providers with per-model thinking capability (e.g. DashScope), + // pre-compute whether this specific model supports thinking and pass it + // along so the provider doesn't need to re-derive it. + if mc, ok := l.provider.(providers.ModelThinkingCapable); ok { + supports := mc.ModelSupportsThinking(l.model) + chatReq.ModelSupportsThinking = &supports + } } else { slog.Debug("thinking_level ignored: provider does not support thinking", "provider", l.provider.Name(), "level", l.thinkingLevel) diff --git a/internal/http/provider_models.go b/internal/http/provider_models.go index be5682c6..139013e1 100644 --- a/internal/http/provider_models.go +++ b/internal/http/provider_models.go @@ -215,7 +215,10 @@ func minimaxModels() []ModelInfo { // DashScope does not expose a standard /v1/models endpoint. func dashScopeModels() []ModelInfo { return []ModelInfo{ - // Chat / text + // Qwen3.5 series — Text Generation + Deep Thinking + Visual Understanding + {ID: "qwen3.5-plus", Name: "Qwen 3.5 Plus"}, + {ID: "qwen3.5-turbo", Name: "Qwen 3.5 Turbo"}, + // Qwen3 hosted series — Text + Thinking {ID: "qwen3-max", Name: "Qwen 3 Max"}, {ID: "qwen3-plus", Name: "Qwen 3 Plus"}, {ID: "qwen3-turbo", Name: "Qwen 3 Turbo"}, diff --git a/internal/providers/dashscope.go b/internal/providers/dashscope.go index f9e00568..dc1126eb 100644 --- a/internal/providers/dashscope.go +++ b/internal/providers/dashscope.go @@ -11,6 +11,23 @@ const ( dashscopeDefaultModel = "qwen3-max" ) +// dashscopeThinkingModels lists DashScope models that accept the +// enable_thinking / thinking_budget parameters (Qwen3 open-weight and Qwen3.5 series). +// Models NOT in this set (e.g. qwen3-plus, qwen3-turbo) will silently +// skip thinking injection to avoid API "model not supported" errors. +var dashscopeThinkingModels = map[string]bool{ + // Qwen3.5 series — thinking + vision + "qwen3.5-plus": true, + "qwen3.5-turbo": true, + // Qwen3 hosted + "qwen3-max": true, + // Qwen3 open-weight (available as hosted inference) + "qwen3-235b-a22b": true, + "qwen3-32b": true, + "qwen3-14b": true, + "qwen3-8b": true, +} + // DashScopeProvider wraps OpenAIProvider to handle DashScope-specific behaviors. // Critical: DashScope does NOT support tools + streaming simultaneously. // When tools are present, ChatStream falls back to non-streaming Chat(). @@ -33,12 +50,32 @@ func NewDashScopeProvider(apiKey, apiBase, defaultModel string) *DashScopeProvid func (p *DashScopeProvider) Name() string { return "dashscope" } func (p *DashScopeProvider) SupportsThinking() bool { return true } -// ChatStream handles DashScope's limitation: tools + streaming cannot coexist. -// When tools are present, falls back to non-streaming Chat() and synthesizes -// chunk callbacks for the caller. -func (p *DashScopeProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk func(StreamChunk)) (*ChatResponse, error) { - // Map thinking_level to DashScope-specific params before passing to OpenAI base - if level, ok := req.Options[OptThinkingLevel].(string); ok && level != "" && level != "off" { +// ModelSupportsThinking implements ModelThinkingCapable. +// Returns true only for models that accept enable_thinking / thinking_budget. +func (p *DashScopeProvider) ModelSupportsThinking(model string) bool { + return dashscopeThinkingModels[p.resolveModel(model)] +} + +// applyThinkingGuard maps thinking_level to DashScope-specific params +// (enable_thinking / thinking_budget) only when the model supports it. +// Returns the (possibly mutated) request. Shared by Chat and ChatStream. +func (p *DashScopeProvider) applyThinkingGuard(req ChatRequest) ChatRequest { + level, ok := req.Options[OptThinkingLevel].(string) + if !ok || level == "" || level == "off" { + return req + } + + // Determine if this model supports thinking: + // 1. Prefer the pre-computed hint from the agent loop (ModelSupportsThinking field). + // 2. Fall back to local map lookup. + var supportsThinking bool + if req.ModelSupportsThinking != nil { + supportsThinking = *req.ModelSupportsThinking + } else { + supportsThinking = p.ModelSupportsThinking(req.Model) + } + + if supportsThinking { // Clone Options to avoid mutating caller's map opts := make(map[string]any, len(req.Options)+2) maps.Copy(opts, req.Options) @@ -46,8 +83,25 @@ func (p *DashScopeProvider) ChatStream(ctx context.Context, req ChatRequest, onC opts[OptThinkingBudget] = dashscopeThinkingBudget(level) delete(opts, OptThinkingLevel) // don't pass generic key to OpenAI buildRequestBody req.Options = opts + } else { + slog.Debug("dashscope: model does not support thinking, skipping enable_thinking", + "model", p.resolveModel(req.Model), "requested_level", level) } + return req +} + +// Chat overrides OpenAIProvider.Chat to apply the per-model thinking guard. +func (p *DashScopeProvider) Chat(ctx context.Context, req ChatRequest) (*ChatResponse, error) { + return p.OpenAIProvider.Chat(ctx, p.applyThinkingGuard(req)) +} + +// ChatStream handles DashScope's limitation: tools + streaming cannot coexist. +// When tools are present, falls back to non-streaming Chat() and synthesizes +// chunk callbacks for the caller. +func (p *DashScopeProvider) ChatStream(ctx context.Context, req ChatRequest, onChunk func(StreamChunk)) (*ChatResponse, error) { + req = p.applyThinkingGuard(req) + if len(req.Tools) > 0 { slog.Debug("dashscope: tools present, falling back to non-streaming Chat") resp, err := p.Chat(ctx, req) diff --git a/internal/providers/dashscope_test.go b/internal/providers/dashscope_test.go new file mode 100644 index 00000000..1c0fb34b --- /dev/null +++ b/internal/providers/dashscope_test.go @@ -0,0 +1,178 @@ +package providers + +import ( + "context" + "encoding/json" + "fmt" + "net/http" + "net/http/httptest" + "testing" +) + +// newDashScopeTestServer sets up a mock SSE server for ChatStream calls and +// returns both the server and a pointer that will hold the last captured request body. +func newDashScopeTestServer(t *testing.T) (*httptest.Server, *map[string]any) { + t.Helper() + captured := &map[string]any{} + server := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + if err := json.NewDecoder(r.Body).Decode(captured); err != nil { + t.Errorf("failed to decode request body: %v", err) + } + // stream=false path (Chat): return JSON + if v, _ := (*captured)["stream"].(bool); !v { + w.Header().Set("Content-Type", "application/json") + fmt.Fprint(w, `{"choices":[{"message":{"role":"assistant","content":"ok"},"finish_reason":"stop"}]}`) + return + } + // stream=true path (ChatStream): return SSE + w.Header().Set("Content-Type", "text/event-stream") + fmt.Fprint(w, "data: {\"choices\":[{\"delta\":{\"content\":\"ok\"},\"finish_reason\":\"stop\"}]}\n\n") + fmt.Fprint(w, "data: [DONE]\n\n") + })) + t.Cleanup(server.Close) + return server, captured +} + +// callDashScopeStream sends req through ChatStream and returns the captured body. +func callDashScopeStream(t *testing.T, req ChatRequest) map[string]any { + t.Helper() + server, captured := newDashScopeTestServer(t) + p := NewDashScopeProvider("test-key", server.URL, "") + p.retryConfig.Attempts = 1 + p.ChatStream(context.Background(), req, nil) //nolint:errcheck + return *captured +} + +// TestDashScopeModelSupportsThinking verifies the whitelist is correct. +func TestDashScopeModelSupportsThinking(t *testing.T) { + p := NewDashScopeProvider("key", "", "") + + tests := []struct { + model string + want bool + }{ + {"qwen3.5-plus", true}, + {"qwen3.5-turbo", true}, + {"qwen3-max", true}, + {"qwen3-235b-a22b", true}, + {"qwen3-32b", true}, + {"qwen3-14b", true}, + {"qwen3-8b", true}, + // Models that do NOT support thinking: + {"qwen3-plus", false}, + {"qwen3-turbo", false}, + {"qwen2-72b-instruct", false}, + } + + for _, tt := range tests { + got := p.ModelSupportsThinking(tt.model) + if got != tt.want { + t.Errorf("ModelSupportsThinking(%q) = %v, want %v", tt.model, got, tt.want) + } + } +} + +// TestDashScopeThinkingInjected_WhenModelSupports verifies enable_thinking IS sent +// for a model on the whitelist (qwen3.5-plus) when thinking_level is set. +// ChatStream is used because that's the path where thinking params are injected. +func TestDashScopeThinkingInjected_WhenModelSupports(t *testing.T) { + body := callDashScopeStream(t, ChatRequest{ + Model: "qwen3.5-plus", + Messages: []Message{{Role: "user", Content: "hi"}}, + Options: map[string]any{OptThinkingLevel: "medium"}, + }) + + if body["enable_thinking"] != true { + t.Errorf("enable_thinking = %v, want true for qwen3.5-plus with thinking_level=medium", body["enable_thinking"]) + } + budget, _ := body["thinking_budget"].(float64) + if budget <= 0 { + t.Errorf("thinking_budget = %v, want > 0", body["thinking_budget"]) + } + if _, hasThinkingLevel := body["thinking_level"]; hasThinkingLevel { + t.Error("thinking_level should be removed from body (converted to enable_thinking)") + } +} + +// TestDashScopeThinkingNotInjected_WhenModelDoesNotSupport verifies enable_thinking +// is NOT sent for qwen3-plus (not on whitelist), even when thinking_level is set. +// This is the root cause fix: previously this caused "model 'qwen3-plus' is not supported". +func TestDashScopeThinkingNotInjected_WhenModelDoesNotSupport(t *testing.T) { + body := callDashScopeStream(t, ChatRequest{ + Model: "qwen3-plus", + Messages: []Message{{Role: "user", Content: "hi"}}, + Options: map[string]any{OptThinkingLevel: "medium"}, + }) + + if _, has := body["enable_thinking"]; has { + t.Errorf("enable_thinking should NOT be sent for qwen3-plus, got: %v", body["enable_thinking"]) + } + if _, has := body["thinking_budget"]; has { + t.Errorf("thinking_budget should NOT be sent for qwen3-plus, got: %v", body["thinking_budget"]) + } +} + +// TestDashScopeThinkingNotInjected_ViaHint verifies that the ModelSupportsThinking=false +// hint suppresses injection even for a whitelisted model. +func TestDashScopeThinkingNotInjected_ViaHint(t *testing.T) { + falseVal := false + body := callDashScopeStream(t, ChatRequest{ + Model: "qwen3.5-plus", // on whitelist, but hint overrides + Messages: []Message{{Role: "user", Content: "hi"}}, + Options: map[string]any{OptThinkingLevel: "high"}, + ModelSupportsThinking: &falseVal, + }) + + if _, has := body["enable_thinking"]; has { + t.Error("enable_thinking should NOT be sent when ModelSupportsThinking=false hint is set") + } +} + +// TestDashScopeThinkingInjected_ViaHint verifies that a true hint on a +// non-whitelisted model still triggers injection (explicit caller override). +func TestDashScopeThinkingInjected_ViaHint(t *testing.T) { + trueVal := true + body := callDashScopeStream(t, ChatRequest{ + Model: "qwen3-plus", // not on whitelist, but hint explicitly allows + Messages: []Message{{Role: "user", Content: "hi"}}, + Options: map[string]any{OptThinkingLevel: "low"}, + ModelSupportsThinking: &trueVal, + }) + + if body["enable_thinking"] != true { + t.Errorf("enable_thinking = %v, want true when ModelSupportsThinking=true hint is set", body["enable_thinking"]) + } +} + +// TestDashScopeNoThinkingLevel verifies that without thinking_level, +// enable_thinking is never injected regardless of model. +func TestDashScopeNoThinkingLevel(t *testing.T) { + body := callDashScopeStream(t, ChatRequest{ + Model: "qwen3-max", + Messages: []Message{{Role: "user", Content: "hi"}}, + Options: map[string]any{OptMaxTokens: 1}, + }) + + if _, has := body["enable_thinking"]; has { + t.Error("enable_thinking should NOT be sent without thinking_level") + } +} + +// TestDashScopeThinkingBudgetValues verifies thinking_budget level mapping. +func TestDashScopeThinkingBudgetValues(t *testing.T) { + tests := []struct { + level string + want int + }{ + {"low", 4096}, + {"medium", 16384}, + {"high", 32768}, + {"unknown", 16384}, // default + } + for _, tt := range tests { + got := dashscopeThinkingBudget(tt.level) + if got != tt.want { + t.Errorf("dashscopeThinkingBudget(%q) = %d, want %d", tt.level, got, tt.want) + } + } +} diff --git a/internal/providers/types.go b/internal/providers/types.go index 42d59b56..c2dcc443 100644 --- a/internal/providers/types.go +++ b/internal/providers/types.go @@ -43,12 +43,25 @@ type ThinkingCapable interface { SupportsThinking() bool } +// ModelThinkingCapable is optionally implemented by providers where thinking +// support varies per model (e.g. DashScope: qwen3-max supports it, qwen3-plus does not). +// This allows the agent loop to pass per-model capability hints to the provider. +type ModelThinkingCapable interface { + ModelSupportsThinking(model string) bool +} + // ChatRequest contains the input for a Chat/ChatStream call. type ChatRequest struct { Messages []Message `json:"messages"` Tools []ToolDefinition `json:"tools,omitempty"` Model string `json:"model,omitempty"` Options map[string]any `json:"options,omitempty"` + + // ModelSupportsThinking is set by the agent loop when the provider implements + // ModelThinkingCapable. Providers use this to decide whether to inject + // enable_thinking / thinking_budget parameters for the requested model. + // nil = unknown (assume capable, for backward compatibility). + ModelSupportsThinking *bool `json:"-"` } // ChatResponse is the result from an LLM call.