Skip to content

Commit d9f5eaf

Browse files
committed
fix(middleware): repair tool_choice string mode + legacy flat shape for /v1/chat/completions
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 mudler#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 mudler#9508. Assisted-by: Claude:claude-sonnet-4-6 Signed-off-by: Adira Denis Muhando <dennisadira@gmail.com>
1 parent ebd9fcb commit d9f5eaf

2 files changed

Lines changed: 171 additions & 11 deletions

File tree

core/http/middleware/request.go

Lines changed: 25 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import (
1414
"github.com/mudler/LocalAI/core/schema"
1515
"github.com/mudler/LocalAI/core/services/galleryop"
1616
"github.com/mudler/LocalAI/core/templates"
17-
"github.com/mudler/LocalAI/pkg/functions"
1817
"github.com/mudler/LocalAI/pkg/model"
1918
"github.com/mudler/LocalAI/pkg/utils"
2019
"github.com/mudler/xlog"
@@ -225,7 +224,7 @@ func (re *RequestExtractor) SetOpenAIRequest(c echo.Context) error {
225224
input.Context = ctxWithCorrelationID
226225
input.Cancel = cancel
227226

228-
err := mergeOpenAIRequestAndModelConfig(cfg, input)
227+
err := MergeOpenAIRequestConfig(cfg, input)
229228
if err != nil {
230229
return err
231230
}
@@ -241,7 +240,7 @@ func (re *RequestExtractor) SetOpenAIRequest(c echo.Context) error {
241240
return nil
242241
}
243242

244-
func mergeOpenAIRequestAndModelConfig(config *config.ModelConfig, input *schema.OpenAIRequest) error {
243+
func MergeOpenAIRequestConfig(config *config.ModelConfig, input *schema.OpenAIRequest) error {
245244
if input.Echo {
246245
config.Echo = input.Echo
247246
}
@@ -320,17 +319,32 @@ func mergeOpenAIRequestAndModelConfig(config *config.ModelConfig, input *schema.
320319
}
321320

322321
if input.ToolsChoice != nil {
323-
var toolChoice functions.Tool
324-
325322
switch content := input.ToolsChoice.(type) {
326323
case string:
327-
_ = json.Unmarshal([]byte(content), &toolChoice)
324+
// "required" and "none" need explicit mode flags; "auto" is the
325+
// default and must remain a no-op to avoid polluting FunctionToCall().
326+
switch content {
327+
case "required", "none":
328+
input.FunctionCall = content
329+
}
330+
// "auto" — leave FunctionCall unset; model decides.
328331
case map[string]any:
329-
dat, _ := json.Marshal(content)
330-
_ = json.Unmarshal(dat, &toolChoice)
331-
}
332-
input.FunctionCall = map[string]any{
333-
"name": toolChoice.Function.Name,
332+
// Specific tool. OpenAI spec nests the function name under "function":
333+
// {"type":"function", "function":{"name":"..."}}
334+
// Legacy/Anthropic-compat form puts it at the top level:
335+
// {"type":"function", "name":"..."}
336+
if tcType, ok := content["type"].(string); ok && tcType == "function" {
337+
var name string
338+
if fn, ok := content["function"].(map[string]any); ok {
339+
name, _ = fn["name"].(string)
340+
}
341+
if name == "" {
342+
name, _ = content["name"].(string)
343+
}
344+
if name != "" {
345+
input.FunctionCall = map[string]any{"name": name}
346+
}
347+
}
334348
}
335349
}
336350

core/http/middleware/request_test.go

Lines changed: 146 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -306,3 +306,149 @@ var _ = Describe("MergeOpenResponsesConfig tool_choice parsing", func() {
306306
})
307307
})
308308
})
309+
310+
// ---------------------------------------------------------------------------
311+
// MergeOpenAIRequestConfig — tool_choice parsing (/v1/chat/completions)
312+
// ---------------------------------------------------------------------------
313+
//
314+
// Mirrors the MergeOpenResponsesConfig suite above but exercises the OpenAI
315+
// chat/completions path. The bug: the old code tried to json.Unmarshal the
316+
// string "required" into a functions.Tool (always fails), then unconditionally
317+
// set FunctionCall to {"name":""}, so "required" mode was silently dropped and
318+
// SetFunctionCallNameString("") was called instead of SetFunctionCallString("required").
319+
var _ = Describe("MergeOpenAIRequestConfig tool_choice parsing", func() {
320+
var cfg *config.ModelConfig
321+
322+
BeforeEach(func() {
323+
cfg = &config.ModelConfig{}
324+
})
325+
326+
Context("string tool_choice", func() {
327+
It("applies \"required\" mode", func() {
328+
req := &schema.OpenAIRequest{ToolsChoice: "required"}
329+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
330+
331+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
332+
Expect(cfg.ShouldUseFunctions()).To(BeTrue())
333+
})
334+
335+
It("applies \"none\" mode", func() {
336+
req := &schema.OpenAIRequest{ToolsChoice: "none"}
337+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
338+
339+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
340+
Expect(cfg.ShouldUseFunctions()).To(BeFalse())
341+
})
342+
343+
It("leaves config untouched for \"auto\"", func() {
344+
req := &schema.OpenAIRequest{ToolsChoice: "auto"}
345+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
346+
347+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
348+
Expect(cfg.FunctionToCall()).To(Equal(""))
349+
})
350+
})
351+
352+
Context("specific-function tool_choice (OpenAI spec shape)", func() {
353+
It("parses {type:function, function:{name:...}} and sets the specific-function name", func() {
354+
req := &schema.OpenAIRequest{
355+
ToolsChoice: map[string]any{
356+
"type": "function",
357+
"function": map[string]any{"name": "get_weather"},
358+
},
359+
}
360+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
361+
362+
Expect(cfg.ShouldCallSpecificFunction()).To(BeTrue())
363+
Expect(cfg.FunctionToCall()).To(Equal("get_weather"))
364+
})
365+
366+
It("prefers nested function.name over a stray top-level name", func() {
367+
req := &schema.OpenAIRequest{
368+
ToolsChoice: map[string]any{
369+
"type": "function",
370+
"function": map[string]any{"name": "correct_name"},
371+
"name": "legacy_name",
372+
},
373+
}
374+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
375+
376+
Expect(cfg.FunctionToCall()).To(Equal("correct_name"))
377+
})
378+
})
379+
380+
Context("specific-function tool_choice (legacy flat shape)", func() {
381+
It("parses {type:function, name:...} and sets the specific-function name", func() {
382+
req := &schema.OpenAIRequest{
383+
ToolsChoice: map[string]any{
384+
"type": "function",
385+
"name": "get_weather",
386+
},
387+
}
388+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
389+
390+
Expect(cfg.ShouldCallSpecificFunction()).To(BeTrue())
391+
Expect(cfg.FunctionToCall()).To(Equal("get_weather"))
392+
})
393+
})
394+
395+
Context("malformed tool_choice", func() {
396+
It("is a no-op when type is missing", func() {
397+
req := &schema.OpenAIRequest{
398+
ToolsChoice: map[string]any{
399+
"function": map[string]any{"name": "get_weather"},
400+
},
401+
}
402+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
403+
404+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
405+
})
406+
407+
It("is a no-op when type is not \"function\"", func() {
408+
req := &schema.OpenAIRequest{
409+
ToolsChoice: map[string]any{
410+
"type": "object",
411+
"function": map[string]any{"name": "get_weather"},
412+
},
413+
}
414+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
415+
416+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
417+
})
418+
419+
It("is a no-op when name is missing from both shapes", func() {
420+
req := &schema.OpenAIRequest{
421+
ToolsChoice: map[string]any{
422+
"type": "function",
423+
"function": map[string]any{},
424+
},
425+
}
426+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
427+
428+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
429+
Expect(cfg.FunctionToCall()).To(Equal(""))
430+
})
431+
432+
It("is a no-op when name is empty string", func() {
433+
req := &schema.OpenAIRequest{
434+
ToolsChoice: map[string]any{
435+
"type": "function",
436+
"function": map[string]any{"name": ""},
437+
},
438+
}
439+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
440+
441+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
442+
})
443+
})
444+
445+
Context("nil tool_choice", func() {
446+
It("is a no-op", func() {
447+
req := &schema.OpenAIRequest{ToolsChoice: nil}
448+
Expect(MergeOpenAIRequestConfig(cfg, req)).To(Succeed())
449+
450+
Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse())
451+
Expect(cfg.FunctionToCall()).To(Equal(""))
452+
})
453+
})
454+
})

0 commit comments

Comments
 (0)