From f877942d97d45f2c0e09a9ee93d642ead4283196 Mon Sep 17 00:00:00 2001 From: walcz-de Date: Thu, 23 Apr 2026 18:30:05 +0200 Subject: [PATCH] fix(openresponses): parse OpenAI-spec nested tool_choice + use correct setter (#9509) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two bugs in MergeOpenResponsesConfig (/v1/responses + WebSocket, *not* /v1/chat/completions — that has a separate, working path via Tool unmarshal + SetFunctionCallNameString): 1. **Shape mismatch.** OpenAI's specific-function tool_choice nests the name under "function": {"type": "function", "function": {"name": "my_function"}} The legacy flat shape was: {"type": "function", "name": "my_function"} Only the flat shape was handled. OpenAI-compliant clients that reach /v1/responses (openai-python with the Responses API, Stainless-generated SDKs, …) silently failed to force the function. 2. **Wrong setter.** The code called SetFunctionCallString(name), which writes the mode field (functionCallString: "none"/"auto"/"required"). The specific-function name lives in a separate field (functionCallNameString), read by ShouldCallSpecificFunction and FunctionToCall. Net effect: a correctly-formed tool_choice never engaged grammar-based forcing. The fix preserves backward compatibility by accepting both shapes (nested preferred, flat as fallback) and routes to the correct setter. Note: The same "wrong setter" pattern appears at three other sites — anthropic/messages.go:883, openai/realtime_model.go:171, and openresponses/responses.go:776 — and /v1/chat/completions has its own issue parsing tool_choice="required" as a string (json.Unmarshal on a raw string fails silently). Those are filed as a tracking issue rather than bundled here to keep this PR focused. ## Test plan 9 new Ginkgo specs under "MergeOpenResponsesConfig tool_choice parsing": - string modes: "required" / "auto" / "none" - OpenAI-spec nested shape: {type:function, function:{name}} - Legacy Anthropic-compat flat shape: {type:function, name} - Shape-preference: nested wins over flat when both present - Malformed: missing type, wrong type, missing name, empty name, nil $ go test ./core/http/middleware/ -count=1 -run TestMiddleware Ran 28 of 28 Specs in 0.003 seconds -- PASS ## Repro (against /v1/responses) curl -N http://localai/v1/responses \ -H 'Content-Type: application/json' \ -d '{"model":"qwen3.6-35b-a3b-apex", "input":"Weather in Berlin?", "tools":[{"type":"function","name":"get_weather", "parameters":{"type":"object", "properties":{"city":{"type":"string"}}, "required":["city"]}}], "tool_choice":{"type":"function", "function":{"name":"get_weather"}}}' Before: grammar-based forcing silently inactive; model free-texts. After : grammar forces get_weather invocation; output contains tool_calls with function:{name:"get_weather", arguments:{...}}. --- core/http/middleware/request.go | 27 ++++- core/http/middleware/request_test.go | 156 +++++++++++++++++++++++++++ 2 files changed, 180 insertions(+), 3 deletions(-) diff --git a/core/http/middleware/request.go b/core/http/middleware/request.go index a6765fff5..1c0ee0ec7 100644 --- a/core/http/middleware/request.go +++ b/core/http/middleware/request.go @@ -614,10 +614,31 @@ func MergeOpenResponsesConfig(config *config.ModelConfig, input *schema.OpenResp } // "auto" is default - let model decide case map[string]any: - // Specific tool: {type:"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":"..."} + // The old code only handled the legacy shape AND used the wrong + // setter (SetFunctionCallString writes the mode field; the + // specific-function name lives in a separate field read by + // ShouldCallSpecificFunction / FunctionToCall). Net effect: a + // correctly-formed OpenAI tool_choice never engaged grammar-based + // forcing, the model got the tools but no selection hint, and + // streamed raw JSON as delta.content instead of delta.tool_calls. if tcType, ok := tc["type"].(string); ok && tcType == "function" { - if name, ok := tc["name"].(string); ok { - config.SetFunctionCallString(name) + var name string + if fn, ok := tc["function"].(map[string]any); ok { + if n, ok := fn["name"].(string); ok { + name = n + } + } + if name == "" { + if n, ok := tc["name"].(string); ok { + name = n + } + } + if name != "" { + config.SetFunctionCallNameString(name) } } } diff --git a/core/http/middleware/request_test.go b/core/http/middleware/request_test.go index da382dd41..0b1a04cf7 100644 --- a/core/http/middleware/request_test.go +++ b/core/http/middleware/request_test.go @@ -150,3 +150,159 @@ var _ = Describe("SetModelAndConfig middleware", func() { }) }) }) + +// --------------------------------------------------------------------------- +// MergeOpenResponsesConfig — tool_choice parsing +// --------------------------------------------------------------------------- +// +// The OpenAI chat/completions spec nests the function name under "function": +// {"type":"function", "function":{"name":"my_function"}} +// The legacy Anthropic-compat shape puts it at the top level: +// {"type":"function", "name":"my_function"} +// Both need to reach SetFunctionCallNameString (not SetFunctionCallString, +// which is the mode field "none"/"auto"/"required"). +// +// These specs assert both shapes populate the specific-function name and that +// downstream predicates (ShouldCallSpecificFunction, FunctionToCall) return +// the expected values so grammar-based forcing actually engages. +var _ = Describe("MergeOpenResponsesConfig tool_choice parsing", func() { + var cfg *config.ModelConfig + + BeforeEach(func() { + cfg = &config.ModelConfig{} + }) + + Context("string tool_choice", func() { + It("sets mode to required for tool_choice=\"required\"", func() { + req := &schema.OpenResponsesRequest{ToolChoice: "required"} + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + // "required" is a mode, not a specific function. + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + // ShouldUseFunctions must be true so tools are sent to the model. + Expect(cfg.ShouldUseFunctions()).To(BeTrue()) + }) + + It("leaves config untouched for tool_choice=\"auto\"", func() { + req := &schema.OpenResponsesRequest{ToolChoice: "auto"} + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.FunctionToCall()).To(Equal("")) + }) + + It("leaves config untouched for tool_choice=\"none\"", func() { + req := &schema.OpenResponsesRequest{ToolChoice: "none"} + Expect(MergeOpenResponsesConfig(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.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + // This is the key invariant the fix restores: a correctly-formed + // OpenAI tool_choice must result in ShouldCallSpecificFunction()=true. + Expect(cfg.ShouldCallSpecificFunction()).To(BeTrue()) + Expect(cfg.FunctionToCall()).To(Equal("get_weather")) + }) + + It("prefers the nested function.name over a stray top-level name", func() { + // Defense-in-depth: both shapes present, OpenAI spec wins. + req := &schema.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": "correct_name"}, + "name": "legacy_name", + }, + } + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.FunctionToCall()).To(Equal("correct_name")) + }) + }) + + Context("specific-function tool_choice (legacy Anthropic-compat shape)", func() { + It("parses {type:function, name:...} and sets the specific-function name", func() { + req := &schema.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "function", + "name": "get_weather", + }, + } + Expect(MergeOpenResponsesConfig(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.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + + It("is a no-op when type is not \"function\"", func() { + req := &schema.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "object", + "function": map[string]any{"name": "get_weather"}, + }, + } + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + + It("is a no-op when name is missing from both shapes", func() { + req := &schema.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "function", + "function": map[string]any{}, + }, + } + Expect(MergeOpenResponsesConfig(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.OpenResponsesRequest{ + ToolChoice: map[string]any{ + "type": "function", + "function": map[string]any{"name": ""}, + }, + } + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + }) + }) + + Context("nil tool_choice", func() { + It("is a no-op", func() { + req := &schema.OpenResponsesRequest{ToolChoice: nil} + Expect(MergeOpenResponsesConfig(cfg, req)).To(Succeed()) + + Expect(cfg.ShouldCallSpecificFunction()).To(BeFalse()) + Expect(cfg.FunctionToCall()).To(Equal("")) + }) + }) +})