From 0f25c31bd53b64dc3fcc8fce0bde954159a67a58 Mon Sep 17 00:00:00 2001 From: Daniel Hiltgen Date: Fri, 14 Aug 2026 11:30:27 -0700 Subject: [PATCH] qwen3.8: support developer instructions (#17749) * qwen3.8: support developer instructions Qwen3.8 does not define a developer role, while OpenAI-compatible coding agents commonly send developer instructions before user messages. Fold the leading system/developer instruction prefix into a single system turn before Qwen3.8 validation, preserving instruction precedence without changing Qwen3.5 or other renderer behavior. Add streaming tool-call integration coverage for the native Ollama, OpenAI Chat Completions, OpenAI Responses, and Anthropic Messages request shapes. Each case exercises prior assistant tool calls, tool results, follow-up rendering, and parsed tool-call output. Add Qwen3.8 to the release tools sweep. Removes an unnecessary unit test that should not have been included in the original 3.8 PR. * review comments --- integration/reg_fast_test.go | 1 + integration/reg_groups_test.go | 1 + integration/reg_release_test.go | 2 + integration/tools_routes_test.go | 453 +++++++++++++++++++++++++++++++ integration/tools_test.go | 1 + integration/utils_test.go | 2 +- model/renderers/qwen35.go | 46 +++- model/renderers/qwen38_test.go | 73 +++-- 8 files changed, 553 insertions(+), 26 deletions(-) create mode 100644 integration/tools_routes_test.go diff --git a/integration/reg_fast_test.go b/integration/reg_fast_test.go index 60486cfcb..a17c2f554 100644 --- a/integration/reg_fast_test.go +++ b/integration/reg_fast_test.go @@ -43,6 +43,7 @@ func init() { registerEmbeddingCases(testModels(fastEmbedModels)) registerVisionTextCases(testModels(fastVisionTextModels)) registerToolCases(testModels(fastToolsModels)) + registerToolRouteCases(testModels(fastToolsModels)) registerToolStressCases(testModels(fastToolsStressModels)) registerAudioTranscriptionCases(testModels(fastAudioModels)) } diff --git a/integration/reg_groups_test.go b/integration/reg_groups_test.go index 9b86dde4f..e799cde08 100644 --- a/integration/reg_groups_test.go +++ b/integration/reg_groups_test.go @@ -120,6 +120,7 @@ func TestConcurrency(t *testing.T) { func TestTools(t *testing.T) { runIntegrationGroup(t, "tools", + "tools-routes", "tools-stress", ) } diff --git a/integration/reg_release_test.go b/integration/reg_release_test.go index de8b5ea74..1f8a9bf25 100644 --- a/integration/reg_release_test.go +++ b/integration/reg_release_test.go @@ -53,6 +53,7 @@ var ( "gemma4", "gpt-oss:20b", "qwen3.6:27b", + "qwen3.8:27b", } releaseAudioModels = []string{ "nemotron3:33b", @@ -126,6 +127,7 @@ func init() { registerEmbeddingCases(testModels(releaseEmbedModels)) registerVisionTextCases(testModels(releaseVisionTextModels)) registerToolCases(testModels(releaseToolsModels)) + registerToolRouteCases(testModels(releaseToolsModels)) registerToolStressCases(testModels(releaseToolsModels)) registerVisionOCRDocumentCases(testModels(releaseVisionModels)) registerAudioTranscriptionCases(testModels(releaseAudioModels)) diff --git a/integration/tools_routes_test.go b/integration/tools_routes_test.go new file mode 100644 index 000000000..b19ac816b --- /dev/null +++ b/integration/tools_routes_test.go @@ -0,0 +1,453 @@ +//go:build integration + +package integration + +import ( + "bufio" + "bytes" + "context" + "encoding/json" + "io" + "net/http" + "os" + "runtime" + "strings" + "testing" + "time" + + "github.com/ollama/ollama/api" +) + +const ( + toolRouteName = "get_weather" + toolRouteLocation = "Boston" + toolRoutePreviousID = "call_previous" +) + +func registerToolRouteCases(models []string) { + registerModelIntegrationCases("tools-routes", models, runToolRoutesModel) +} + +func runToolRoutesModel(t *testing.T, model string) { + softTimeout, hardTimeout := getTimeouts(t) + if time.Since(started) > softTimeout { + t.Skip("skipping remaining tests to avoid excessive runtime") + } + ctx, cancel := context.WithTimeout(context.Background(), hardTimeout) + defer cancel() + + client, endpoint, cleanup := InitServerConnection(ctx, t) + t.Cleanup(func() { + cleanup() + assertNoUnexpectedRoleWarnings(t) + }) + + if v, ok := toolsMinVRAM[model]; ok { + skipUnderMinVRAM(t, v) + } + requireCapability(ctx, t, client, model, "tools") + + t.Run("ollama", func(t *testing.T) { + runOllamaToolRoute(t, ctx, client, model) + }) + t.Run("openai_chat_completions", func(t *testing.T) { + runOpenAIChatToolRoute(t, ctx, endpoint, model) + }) + t.Run("openai_responses", func(t *testing.T) { + runOpenAIResponsesToolRoute(t, ctx, endpoint, model) + }) + t.Run("anthropic_messages", func(t *testing.T) { + runAnthropicToolRoute(t, ctx, endpoint, model) + }) +} + +func assertNoUnexpectedRoleWarnings(t *testing.T) { + t.Helper() + if os.Getenv("OLLAMA_TEST_EXISTING") != "" || runtime.GOOS == "windows" { + return + } + + serverMutex.Lock() + defer serverMutex.Unlock() + if strings.Contains(serverLog.String(), "unexpected message role") { + t.Error("tool route request emitted an unexpected message role warning") + } +} + +func toolRouteTool() api.Tool { + return newTool(toolRouteName, "Get the current weather for a location", []string{"location"}, map[string]api.ToolProperty{ + "location": {Type: api.PropertyType{"string"}, Description: "The city name"}, + }) +} + +func toolRouteArguments(location string) api.ToolCallFunctionArguments { + arguments := api.NewToolCallFunctionArguments() + arguments.Set("location", location) + return arguments +} + +func runOllamaToolRoute(t *testing.T, ctx context.Context, client *api.Client, model string) { + t.Helper() + stream := true + req := api.ChatRequest{ + Model: model, + Messages: []api.Message{ + {Role: "system", Content: toolRouteSystemPrompt()}, + {Role: "user", Content: "Call get_weather for Paris."}, + { + Role: "assistant", + ToolCalls: []api.ToolCall{{ + ID: toolRoutePreviousID, + Function: api.ToolCallFunction{ + Name: toolRouteName, + Arguments: toolRouteArguments("Paris"), + }, + }}, + }, + {Role: "tool", Content: `{"condition":"sunny"}`, ToolName: toolRouteName, ToolCallID: toolRoutePreviousID}, + {Role: "user", Content: toolRouteUserPrompt()}, + }, + Tools: []api.Tool{toolRouteTool()}, + Stream: &stream, + Think: &api.ThinkValue{Value: false}, + Options: map[string]any{ + "temperature": 0, + "num_ctx": contextLength(16384), + "num_predict": 256, + }, + KeepAlive: &api.Duration{Duration: 10 * time.Second}, + } + + var calls []api.ToolCall + var content strings.Builder + if err := client.Chat(ctx, &req, func(response api.ChatResponse) error { + calls = append(calls, response.Message.ToolCalls...) + content.WriteString(response.Message.Content) + return nil + }); err != nil { + t.Fatalf("Ollama chat failed: %v", err) + } + checkNoLeakedTags(t, content.String()) + if len(calls) == 0 { + t.Fatalf("Ollama chat returned no tool call; content=%q", truncate(content.String(), 300)) + } + assertToolRouteCall(t, calls[len(calls)-1].Function.Name, calls[len(calls)-1].Function.Arguments.String()) +} + +func runOpenAIChatToolRoute(t *testing.T, ctx context.Context, endpoint, model string) { + t.Helper() + payload := map[string]any{ + "model": model, + "messages": []any{ + map[string]any{"role": "system", "content": "Tool protocol compatibility test."}, + map[string]any{"role": "developer", "content": toolRouteSystemPrompt()}, + map[string]any{"role": "user", "content": []any{map[string]any{"type": "text", "text": "Call get_weather for Paris."}}}, + map[string]any{ + "role": "assistant", + "content": nil, + "tool_calls": []any{map[string]any{ + "id": toolRoutePreviousID, + "type": "function", + "function": map[string]any{ + "name": toolRouteName, + "arguments": `{"location":"Paris"}`, + }, + }}, + }, + map[string]any{ + "role": "tool", + "tool_call_id": toolRoutePreviousID, + "content": []any{map[string]any{"type": "text", "text": `{"condition":"sunny"}`}}, + }, + map[string]any{"role": "user", "content": toolRouteUserPrompt()}, + }, + "tools": []any{openAIChatToolRouteDefinition()}, + "stream": true, + "temperature": 0, + "max_tokens": 256, + "reasoning_effort": "none", + } + + events := postToolRouteSSE(t, ctx, endpoint, "/v1/chat/completions", payload, nil) + var name, arguments string + for _, event := range events { + var chunk struct { + Choices []struct { + Delta struct { + ToolCalls []struct { + Function struct { + Name string `json:"name"` + Arguments string `json:"arguments"` + } `json:"function"` + } `json:"tool_calls"` + } `json:"delta"` + } `json:"choices"` + } + if err := json.Unmarshal(event.Data, &chunk); err != nil { + t.Fatalf("decode OpenAI Chat event: %v", err) + } + for _, choice := range chunk.Choices { + for _, call := range choice.Delta.ToolCalls { + name += call.Function.Name + arguments += call.Function.Arguments + } + } + } + assertToolRouteCall(t, name, arguments) +} + +func runOpenAIResponsesToolRoute(t *testing.T, ctx context.Context, endpoint, model string) { + t.Helper() + payload := map[string]any{ + "model": model, + "instructions": "Tool protocol compatibility test.", + "input": []any{ + map[string]any{"type": "message", "role": "developer", "content": toolRouteSystemPrompt()}, + map[string]any{"type": "message", "role": "user", "content": []any{map[string]any{"type": "input_text", "text": "Call get_weather for Paris."}}}, + map[string]any{ + "type": "function_call", + "call_id": toolRoutePreviousID, + "name": toolRouteName, + "arguments": `{"location":"Paris"}`, + }, + map[string]any{ + "type": "function_call_output", + "call_id": toolRoutePreviousID, + "output": []any{map[string]any{"type": "input_text", "text": `{"condition":"sunny"}`}}, + }, + map[string]any{"type": "message", "role": "user", "content": toolRouteUserPrompt()}, + }, + "tools": []any{map[string]any{ + "type": "function", + "name": toolRouteName, + "description": "Get the current weather for a location", + "strict": false, + "parameters": toolRouteParameters(), + }}, + "stream": true, + "temperature": 0, + "max_output_tokens": 256, + "reasoning": map[string]any{"effort": "none"}, + } + + events := postToolRouteSSE(t, ctx, endpoint, "/v1/responses", payload, nil) + var name, arguments string + for _, event := range events { + if event.Event != "response.output_item.done" { + continue + } + var data struct { + Item struct { + Type string `json:"type"` + Name string `json:"name"` + Arguments string `json:"arguments"` + } `json:"item"` + } + if err := json.Unmarshal(event.Data, &data); err != nil { + t.Fatalf("decode OpenAI Responses event: %v", err) + } + if data.Item.Type == "function_call" { + name = data.Item.Name + arguments = data.Item.Arguments + } + } + assertToolRouteCall(t, name, arguments) +} + +func runAnthropicToolRoute(t *testing.T, ctx context.Context, endpoint, model string) { + t.Helper() + payload := map[string]any{ + "model": model, + "max_tokens": 256, + "system": []any{ + map[string]any{"type": "text", "text": "Tool protocol compatibility test."}, + map[string]any{"type": "text", "text": toolRouteSystemPrompt(), "cache_control": map[string]any{"type": "ephemeral"}}, + }, + "messages": []any{ + map[string]any{"role": "user", "content": []any{map[string]any{"type": "text", "text": "Call get_weather for Paris."}}}, + map[string]any{"role": "assistant", "content": []any{map[string]any{ + "type": "tool_use", + "id": toolRoutePreviousID, + "name": toolRouteName, + "input": map[string]any{"location": "Paris"}, + }}}, + map[string]any{"role": "user", "content": []any{ + map[string]any{ + "type": "tool_result", + "tool_use_id": toolRoutePreviousID, + "content": []any{map[string]any{"type": "text", "text": `{"condition":"sunny"}`}}, + }, + map[string]any{"type": "text", "text": toolRouteUserPrompt()}, + }}, + }, + "tools": []any{map[string]any{ + "name": toolRouteName, + "description": "Get the current weather for a location", + "input_schema": toolRouteParameters(), + }}, + "stream": true, + "temperature": 0, + "thinking": map[string]any{"type": "disabled"}, + } + + events := postToolRouteSSE(t, ctx, endpoint, "/v1/messages", payload, map[string]string{ + "anthropic-version": "2023-06-01", + }) + var name, arguments string + var toolIndex int + var foundTool bool + for _, event := range events { + switch event.Event { + case "content_block_start": + var data struct { + Index int `json:"index"` + ContentBlock struct { + Type string `json:"type"` + Name string `json:"name"` + } `json:"content_block"` + } + if err := json.Unmarshal(event.Data, &data); err != nil { + t.Fatalf("decode Anthropic content start: %v", err) + } + if data.ContentBlock.Type == "tool_use" { + foundTool = true + toolIndex = data.Index + name = data.ContentBlock.Name + } + case "content_block_delta": + if !foundTool { + continue + } + var data struct { + Index int `json:"index"` + Delta struct { + Type string `json:"type"` + PartialJSON string `json:"partial_json"` + } `json:"delta"` + } + if err := json.Unmarshal(event.Data, &data); err != nil { + t.Fatalf("decode Anthropic content delta: %v", err) + } + if data.Index == toolIndex && data.Delta.Type == "input_json_delta" { + arguments += data.Delta.PartialJSON + } + } + } + assertToolRouteCall(t, name, arguments) +} + +func toolRouteSystemPrompt() string { + return "When the user asks for weather, call get_weather. Return only the tool call and do not answer in text." +} + +func toolRouteUserPrompt() string { + return "Now call get_weather with location exactly Boston. Return only the tool call." +} + +func openAIChatToolRouteDefinition() map[string]any { + return map[string]any{ + "type": "function", + "function": map[string]any{ + "name": toolRouteName, + "description": "Get the current weather for a location", + "parameters": toolRouteParameters(), + }, + } +} + +func toolRouteParameters() map[string]any { + return map[string]any{ + "type": "object", + "required": []string{"location"}, + "properties": map[string]any{"location": map[string]any{"type": "string"}}, + } +} + +type toolRouteSSEEvent struct { + Event string + Data json.RawMessage +} + +func postToolRouteSSE(t *testing.T, ctx context.Context, endpoint, path string, payload any, headers map[string]string) []toolRouteSSEEvent { + t.Helper() + body, err := json.Marshal(payload) + if err != nil { + t.Fatal(err) + } + + requestCtx, cancel := context.WithTimeout(ctx, 4*time.Minute) + defer cancel() + req, err := http.NewRequestWithContext(requestCtx, http.MethodPost, "http://"+endpoint+path, bytes.NewReader(body)) + if err != nil { + t.Fatal(err) + } + req.Header.Set("Content-Type", "application/json") + for name, value := range headers { + req.Header.Set(name, value) + } + + resp, err := http.DefaultClient.Do(req) + if err != nil { + t.Fatalf("POST %s failed: %v", path, err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusOK { + data, _ := io.ReadAll(io.LimitReader(resp.Body, 1<<20)) + t.Fatalf("POST %s returned %s: %s", path, resp.Status, data) + } + + var events []toolRouteSSEEvent + var eventName string + var eventData strings.Builder + flush := func() { + data := strings.TrimSpace(eventData.String()) + if data != "" && data != "[DONE]" { + events = append(events, toolRouteSSEEvent{Event: eventName, Data: json.RawMessage(data)}) + } + eventName = "" + eventData.Reset() + } + + scanner := bufio.NewScanner(resp.Body) + scanner.Buffer(make([]byte, 64<<10), 2<<20) + for scanner.Scan() { + line := scanner.Text() + if line == "" { + flush() + continue + } + if value, ok := strings.CutPrefix(line, "event:"); ok { + eventName = strings.TrimSpace(value) + continue + } + if value, ok := strings.CutPrefix(line, "data:"); ok { + if eventData.Len() > 0 { + eventData.WriteByte('\n') + } + eventData.WriteString(strings.TrimSpace(value)) + } + } + if err := scanner.Err(); err != nil { + t.Fatalf("read %s stream: %v", path, err) + } + flush() + if len(events) == 0 { + t.Fatalf("POST %s returned no SSE events", path) + } + return events +} + +func assertToolRouteCall(t *testing.T, name, arguments string) { + t.Helper() + if name != toolRouteName { + t.Fatalf("tool name = %q, want %q; arguments=%q", name, toolRouteName, arguments) + } + var values map[string]any + if err := json.Unmarshal([]byte(arguments), &values); err != nil { + t.Fatalf("tool arguments are not JSON: %q: %v", arguments, err) + } + location, _ := values["location"].(string) + if !strings.EqualFold(location, toolRouteLocation) { + t.Fatalf("tool location = %q, want %q; arguments=%s", location, toolRouteLocation, arguments) + } +} diff --git a/integration/tools_test.go b/integration/tools_test.go index 8fb4067e9..fa1cebe77 100644 --- a/integration/tools_test.go +++ b/integration/tools_test.go @@ -32,6 +32,7 @@ var toolsMinVRAM = map[string]uint64{ "nemotron3:33b": 32, "qwen3.5:2b": 4, "qwen3.6:27b": 20, + "qwen3.8:27b": 20, "qwen3-vl": 16, "gpt-oss:20b": 16, "gpt-oss:120b": 70, diff --git a/integration/utils_test.go b/integration/utils_test.go index 2d0fa2804..7ac356f22 100644 --- a/integration/utils_test.go +++ b/integration/utils_test.go @@ -308,7 +308,7 @@ func InitServerConnection(ctx context.Context, t *testing.T) (*api.Client, strin if t.Failed() || os.Getenv("OLLAMA_TEST_LOG_SERVER") != "" { slog.Warn("SERVER LOG FOLLOWS") - io.Copy(os.Stderr, &serverLog) + io.Copy(os.Stderr, bytes.NewReader(serverLog.Bytes())) slog.Warn("END OF SERVER") } slog.Info("cleanup complete", "failed", t.Failed()) diff --git a/model/renderers/qwen35.go b/model/renderers/qwen35.go index 4fb27f53f..8b9e57971 100644 --- a/model/renderers/qwen35.go +++ b/model/renderers/qwen35.go @@ -2,6 +2,7 @@ package renderers import ( "fmt" + "log/slog" "strings" "github.com/ollama/ollama/api" @@ -132,6 +133,39 @@ func qwen38ReasoningInstructions(think *api.ThinkValue) (string, error) { } } +// Qwen3.8 has no developer role. Fold leading developer instructions into its +// system turn so they retain precedence over user messages. +func normalizeQwen38Messages(messages []api.Message) ([]api.Message, error) { + var instructionCount int + var hasDeveloper bool + var instructions []string + for _, message := range messages { + if message.Role != "system" && message.Role != "developer" { + break + } + if len(message.Images) > 0 { + return nil, fmt.Errorf("%s message cannot contain images", message.Role) + } + instructionCount++ + hasDeveloper = hasDeveloper || message.Role == "developer" + if content := strings.TrimSpace(message.Content); content != "" { + instructions = append(instructions, content) + } + } + + if !hasDeveloper { + return messages, nil + } + + normalized := make([]api.Message, 0, len(messages)-instructionCount+1) + normalized = append(normalized, api.Message{ + Role: "system", + Content: strings.Join(instructions, "\n\n"), + }) + normalized = append(normalized, messages[instructionCount:]...) + return normalized, nil +} + func (r *Qwen35Renderer) validateMessages(messages []api.Message) error { if r.variant != qwen35Renderer38 { return nil @@ -166,8 +200,6 @@ func (r *Qwen35Renderer) validateMessages(messages []api.Message) error { return fmt.Errorf("system message must be at the beginning") } case "user", "assistant", "tool": - default: - return fmt.Errorf("unexpected message role %q", message.Role) } } @@ -175,6 +207,13 @@ func (r *Qwen35Renderer) validateMessages(messages []api.Message) error { } func (r *Qwen35Renderer) Render(messages []api.Message, tools []api.Tool, think *api.ThinkValue) (string, error) { + if r.variant == qwen35Renderer38 { + var err error + messages, err = normalizeQwen38Messages(messages) + if err != nil { + return "", err + } + } if err := r.validateMessages(messages); err != nil { return "", err } @@ -305,6 +344,9 @@ func (r *Qwen35Renderer) Render(messages []api.Message, tools []api.Tool, think if i == len(messages)-1 || messages[i+1].Role != "tool" { sb.WriteString(imEndTag + "\n") } + } else if r.variant == qwen35Renderer38 && message.Role != "system" { + slog.Warn("unexpected message role", "renderer", "qwen3.8", "role", message.Role) + sb.WriteString(imStartTag + message.Role + "\n" + content + imEndTag + "\n") } // prefill at the end diff --git a/model/renderers/qwen38_test.go b/model/renderers/qwen38_test.go index c5d7e4161..6e9b624aa 100644 --- a/model/renderers/qwen38_test.go +++ b/model/renderers/qwen38_test.go @@ -1,8 +1,6 @@ package renderers import ( - "crypto/sha256" - "encoding/hex" "encoding/json" "os" "os/exec" @@ -98,6 +96,7 @@ Reminder: - If there is no function call available, answer the question like normal with your current knowledge and do not tell the user about function calls <|im_end|> ` + developerToolHeader := strings.TrimSuffix(toolHeader, imEndTag+"\n") + "\n\nUse tools when requested." + imEndTag + "\n" tests := []struct { name string @@ -120,6 +119,41 @@ Hello<|im_end|> `, matchesTemplate: true, }, + { + name: "developer instruction maps to system", + messages: []api.Message{ + {Role: "developer", Content: "Use Go."}, + {Role: "user", Content: "Hello"}, + }, + want: `<|im_start|>system +` + qwen38RefXHigh + ` + +Use Go.<|im_end|> +<|im_start|>user +Hello<|im_end|> +<|im_start|>assistant + +`, + }, + { + name: "system and developer instructions merge", + messages: []api.Message{ + {Role: "system", Content: "Base policy."}, + {Role: "developer", Content: "Use Go."}, + {Role: "user", Content: "Hello"}, + }, + want: `<|im_start|>system +` + qwen38RefXHigh + ` + +Base policy. + +Use Go.<|im_end|> +<|im_start|>user +Hello<|im_end|> +<|im_start|>assistant + +`, + }, { name: "boolean true uses API medium effort", messages: []api.Message{ @@ -190,6 +224,20 @@ Next<|im_end|> `, matchesTemplate: true, }, + { + name: "developer instruction with tools", + messages: []api.Message{ + {Role: "developer", Content: "Use tools when requested."}, + {Role: "user", Content: "Check the weather."}, + }, + tools: weather, + think: think(true), + want: developerToolHeader + `<|im_start|>user +Check the weather.<|im_end|> +<|im_start|>assistant + +`, + }, { name: "tool call result and next generation prompt", messages: []api.Message{ @@ -332,14 +380,6 @@ func TestQwen38RendererRejectsInvalidTranscripts(t *testing.T) { }, wantErr: "system message cannot contain images", }, - { - name: "unexpected role", - messages: []api.Message{ - {Role: "user", Content: "Hello"}, - {Role: "developer", Content: "No"}, - }, - wantErr: `unexpected message role "developer"`, - }, } for _, tt := range tests { @@ -387,19 +427,6 @@ Partial` } } -func TestQwen38TemplateFixtureHash(t *testing.T) { - data, err := os.ReadFile(qwen38Template) - if err != nil { - t.Fatal(err) - } - // The publisher file has no trailing newline. The checked-in fixture adds - // the repository-standard final newline, which does not change Jinja output. - sum := sha256.Sum256([]byte(strings.TrimSuffix(string(data), "\n"))) - if got, want := hex.EncodeToString(sum[:]), "c3cf9e34abf4f9e36c2d72165aa9c132d3e2a725b6c2586aaa3a8af9d7a81041"; got != want { - t.Fatalf("template SHA-256 = %s, want %s", got, want) - } -} - func TestQwen38TokenizerMatchesReference(t *testing.T) { modelDir := os.Getenv("QWEN38_MODEL_DIR") if modelDir == "" {