From d9f5eaf695359490d143361952863f7cf08b099a Mon Sep 17 00:00:00 2001 From: Adira Denis Muhando Date: Sun, 17 May 2026 10:30:56 +0300 Subject: [PATCH] fix(middleware): repair tool_choice string mode + legacy flat shape for /v1/chat/completions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The ToolsChoice block in mergeOpenAIRequestAndModelConfig had two bugs: 1. String mode ("required", "none"): json.Unmarshal([]byte("required"), &tool) always fails, leaving toolChoice zero-valued. The code then unconditionally set FunctionCall to {"name":""}, so SetFunctionCallNameString("") ran instead of SetFunctionCallString("required") — the mode was silently dropped. 2. Legacy flat tool_choice shape {"type":"function","name":"..."}: functions.Tool has no top-level Name field, so json.Unmarshal produced an empty Function.Name, and the specific-function name was never forwarded. Fix mirrors the approach from MergeOpenResponsesConfig (landed in #9509): - String case: pass "required"/"none" through as a string so the downstream FunctionCall switch routes to SetFunctionCallString; leave "auto" as a no-op. - Map case: walk the nested OpenAI shape first, fall back to the flat shape, only set FunctionCall when a non-empty name is found. Also exports mergeOpenAIRequestAndModelConfig as MergeOpenAIRequestConfig so it can be tested directly (same pattern as MergeOpenResponsesConfig). Adds 11 Ginkgo specs covering all modes, both map shapes, malformed inputs, and nil. Closes #9508. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Adira Denis Muhando --- core/http/middleware/request.go | 36 +++++-- core/http/middleware/request_test.go | 146 +++++++++++++++++++++++++++ 2 files changed, 171 insertions(+), 11 deletions(-) diff --git a/core/http/middleware/request.go b/core/http/middleware/request.go index 1c0ee0ec7cf1..fd58f74c7e4c 100644 --- a/core/http/middleware/request.go +++ b/core/http/middleware/request.go @@ -14,7 +14,6 @@ import ( "github.com/mudler/LocalAI/core/schema" "github.com/mudler/LocalAI/core/services/galleryop" "github.com/mudler/LocalAI/core/templates" - "github.com/mudler/LocalAI/pkg/functions" "github.com/mudler/LocalAI/pkg/model" "github.com/mudler/LocalAI/pkg/utils" "github.com/mudler/xlog" @@ -225,7 +224,7 @@ func (re *RequestExtractor) SetOpenAIRequest(c echo.Context) error { input.Context = ctxWithCorrelationID input.Cancel = cancel - err := mergeOpenAIRequestAndModelConfig(cfg, input) + err := MergeOpenAIRequestConfig(cfg, input) if err != nil { return err } @@ -241,7 +240,7 @@ func (re *RequestExtractor) SetOpenAIRequest(c echo.Context) error { return nil } -func mergeOpenAIRequestAndModelConfig(config *config.ModelConfig, input *schema.OpenAIRequest) error { +func MergeOpenAIRequestConfig(config *config.ModelConfig, input *schema.OpenAIRequest) error { if input.Echo { config.Echo = input.Echo } @@ -320,17 +319,32 @@ func mergeOpenAIRequestAndModelConfig(config *config.ModelConfig, input *schema. } if input.ToolsChoice != nil { - var toolChoice functions.Tool - switch content := input.ToolsChoice.(type) { case string: - _ = json.Unmarshal([]byte(content), &toolChoice) + // "required" and "none" need explicit mode flags; "auto" is the + // default and must remain a no-op to avoid polluting FunctionToCall(). + switch content { + case "required", "none": + input.FunctionCall = content + } + // "auto" — leave FunctionCall unset; model decides. case map[string]any: - dat, _ := json.Marshal(content) - _ = json.Unmarshal(dat, &toolChoice) - } - input.FunctionCall = map[string]any{ - "name": toolChoice.Function.Name, + // Specific tool. OpenAI spec nests the function name under "function": + // {"type":"function", "function":{"name":"..."}} + // Legacy/Anthropic-compat form puts it at the top level: + // {"type":"function", "name":"..."} + if tcType, ok := content["type"].(string); ok && tcType == "function" { + var name string + if fn, ok := content["function"].(map[string]any); ok { + name, _ = fn["name"].(string) + } + if name == "" { + name, _ = content["name"].(string) + } + if name != "" { + input.FunctionCall = map[string]any{"name": name} + } + } } } diff --git a/core/http/middleware/request_test.go b/core/http/middleware/request_test.go index 0b1a04cf70d2..e0a5637454dc 100644 --- a/core/http/middleware/request_test.go +++ b/core/http/middleware/request_test.go @@ -306,3 +306,149 @@ var _ = Describe("MergeOpenResponsesConfig tool_choice parsing", func() { }) }) }) + +// --------------------------------------------------------------------------- +// MergeOpenAIRequestConfig — tool_choice parsing (/v1/chat/completions) +// --------------------------------------------------------------------------- +// +// Mirrors the MergeOpenResponsesConfig suite above but exercises the OpenAI +// chat/completions path. The bug: the old code tried to json.Unmarshal the +// string "required" into a functions.Tool (always fails), then unconditionally +// set FunctionCall to {"name":""}, so "required" mode was silently dropped and +// SetFunctionCallNameString("") was called instead of SetFunctionCallString("required"). +var _ = Describe("MergeOpenAIRequestConfig tool_choice parsing", func() { + var cfg *config.ModelConfig + + BeforeEach(func() { + cfg = &config.ModelConfig{} + }) + + Context("string tool_choice", func() { + It("applies \"required\" mode", func() { + req := &schema.OpenAIRequest{ToolsChoice: "required"} + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.ShouldUseFunctions()).To(BeTrue()) + }) + + It("applies \"none\" mode", func() { + req := &schema.OpenAIRequest{ToolsChoice: "none"} + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.ShouldUseFunctions()).To(BeFalse()) + }) + + It("leaves config untouched for \"auto\"", func() { + req := &schema.OpenAIRequest{ToolsChoice: "auto"} + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.FunctionToCall()).To(Equal("")) + }) + }) + + Context("specific-function tool_choice (OpenAI spec shape)", func() { + It("parses {type:function, function:{name:...}} and sets the specific-function name", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeTrue()) + Expect(cfg.FunctionToCall()).To(Equal("get_weather")) + }) + + It("prefers nested function.name over a stray top-level name", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": "correct_name"}, + "name": "legacy_name", + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.FunctionToCall()).To(Equal("correct_name")) + }) + }) + + Context("specific-function tool_choice (legacy flat shape)", func() { + It("parses {type:function, name:...} and sets the specific-function name", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "function", + "name": "get_weather", + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeTrue()) + Expect(cfg.FunctionToCall()).To(Equal("get_weather")) + }) + }) + + Context("malformed tool_choice", func() { + It("is a no-op when type is missing", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + + It("is a no-op when type is not \"function\"", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "object", + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + + It("is a no-op when name is missing from both shapes", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "function", + "function": map[string]any{}, + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.FunctionToCall()).To(Equal("")) + }) + + It("is a no-op when name is empty string", func() { + req := &schema.OpenAIRequest{ + ToolsChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": ""}, + }, + } + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + }) + + Context("nil tool_choice", func() { + It("is a no-op", func() { + req := &schema.OpenAIRequest{ToolsChoice: nil} + Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.FunctionToCall()).To(Equal("")) + }) + }) +})