From 2e3d5b79adf4c0e0e6f06913bb6060792cad4c1c Mon Sep 17 00:00:00 2001 From: JianFeeeee Date: Mon, 31 Aug 2026 10:23:35 +0800 Subject: [PATCH] fix(adapters): stop dropping non-streaming tool calls (agent loops died on turn 2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four adapters handled tool_calls in transform_stream_chunk but lost them in transform_response, so any NON-streaming tool-using conversation broke on its second request: the client received finish_reason:"tool_calls" with no tool_calls payload, replayed an assistant message whose function name/arguments were empty, and the upstream rejected the next turn with 400 invalid tool_call function, function/name/arguments cannot be empty The production audit trail shows 46 such failures on sensenova alone. - sensenova.lua: forward message.tool_calls, decoding the arguments JSON string into an object as the unified shape expects. - gemini.lua: collect functionCall parts from candidates[].content.parts. Also correct finish_reason, since Gemini reports "STOP" even when it emitted a function call and clients keyed on it treat that as a finished answer. - ollama.lua: the field was initialized to an empty table and never filled; fill it and likewise correct done_reason "stop" -> "tool_calls". trae is a different failure with the same symptom: trae-local-api's OpenAI endpoint (/v1/chat/completions, src/server.js:353) never reads the request's `tools` array — only its Anthropic endpoint does — so the relayed model is never told the tool schema and instead PRINTS a {...} block into content, leaving message.tool_calls null and finish_reason "stop". An OpenAI client sees an ordinary completion and its agent loop ends mid-conversation. trae.lua now recovers the structured call from that text, strips the block from user-visible content, and corrects finish_reason. Both tag spellings (/, the latter is what the same codebase's Anthropic prompt asks for) and all three argument key names (arguments/params/input) are accepted. This is a defensive fallback: fixing the upstream shim to honour `tools` remains the real fix, since the model still guesses parameter names. Tests: TestNonStreamToolCallsPreserved covers all ten OpenAI-shaped adapters, TestGeminiNonStreamToolCalls and TestOllamaNonStreamToolCalls cover their native shapes, TestTraeTextToolCallRecovery covers both tag spellings, prose around the block, and asserts a plain text answer never gains tool_calls. Verified end-to-end against mock upstreams reproducing each shape: a full two-round agent loop (tool call -> tool result -> final answer) now completes for both the structured and the text-emitted variants. --- cmd/gui/package-lock.json | 4 +- cmd/gui/package.json | 2 +- internal/lua/adapters/gemini.lua | 26 ++++ internal/lua/adapters/ollama.lua | 28 +++- internal/lua/adapters/sensenova.lua | 28 ++++ internal/lua/adapters/trae.lua | 102 ++++++++++++- internal/lua/vm_test.go | 226 ++++++++++++++++++++++++++++ 7 files changed, 408 insertions(+), 8 deletions(-) diff --git a/cmd/gui/package-lock.json b/cmd/gui/package-lock.json index 0682296..8eb9d01 100644 --- a/cmd/gui/package-lock.json +++ b/cmd/gui/package-lock.json @@ -1,12 +1,12 @@ { "name": "modelrouter-gui", - "version": "1.4.1", + "version": "1.4.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "modelrouter-gui", - "version": "1.4.1", + "version": "1.4.2", "license": "Proprietary", "devDependencies": { "electron": "^33.0.0", diff --git a/cmd/gui/package.json b/cmd/gui/package.json index afe4f37..b5f71ae 100644 --- a/cmd/gui/package.json +++ b/cmd/gui/package.json @@ -1,6 +1,6 @@ { "name": "modelrouter-gui", - "version": "1.4.1", + "version": "1.4.2", "description": "ModelRouter Desktop — embedded ModelRouter core with tray", "author": "ModelRouter", "main": "main.js", diff --git a/internal/lua/adapters/gemini.lua b/internal/lua/adapters/gemini.lua index 168d920..96fbf36 100644 --- a/internal/lua/adapters/gemini.lua +++ b/internal/lua/adapters/gemini.lua @@ -81,16 +81,42 @@ function adapter.transform_response(raw_body) if resp.candidates and #resp.candidates > 0 then local cand = resp.candidates[1] + local tools = {} if cand.content and cand.content.parts then for _, part in ipairs(cand.content.parts) do if part.text then unified.content = unified.content .. part.text + elseif part.functionCall then + -- Non-streaming tool calls used to be dropped here while + -- transform_stream_chunk handled them, so a non-streaming + -- agent turn looked like a plain text answer and the tool + -- loop died. Gemini's args are already an object. + local args = part.functionCall.args + if type(args) == "string" then + local aok, decoded = pcall(json.decode, args) + args = aok and decoded or {} + elseif type(args) ~= "table" then + args = {} + end + table.insert(tools, { + id = part.functionCall.id or ("call_" .. #tools), + type = "function", + name = part.functionCall.name or "", + arguments = args + }) end end end if cand.finishReason then unified.finish_reason = cand.finishReason end + if #tools > 0 then + unified.tool_calls = tools + -- Gemini reports finishReason "STOP" even when it emitted a + -- functionCall; clients keyed on finish_reason would treat that as + -- a completed answer and never run the tool. + unified.finish_reason = "tool_calls" + end end return json.encode(unified) diff --git a/internal/lua/adapters/ollama.lua b/internal/lua/adapters/ollama.lua index 5916ce9..541c343 100644 --- a/internal/lua/adapters/ollama.lua +++ b/internal/lua/adapters/ollama.lua @@ -58,13 +58,39 @@ function adapter.transform_response(raw_body) local unified = { content = "", finish_reason = resp.done_reason or "", - tool_calls = {}, -- key must be token_usage to match Go's UnifiedResponse json tag token_usage = { prompt = p, completion = c, total = p + c } } if resp.message then unified.content = resp.message.content or "" + -- Non-streaming tool calls were previously dropped: the field was + -- initialized to an empty table and never filled, while + -- transform_stream_chunk handled them. A non-streaming agent turn thus + -- looked like a plain answer and the tool loop stopped. + if type(resp.message.tool_calls) == "table" and #resp.message.tool_calls > 0 then + local tcs = {} + for _, tc in ipairs(resp.message.tool_calls) do + local fn = tc["function"] or {} + -- Ollama sends arguments as an object already + local args = fn.arguments + if type(args) == "string" then + local aok, decoded = pcall(json.decode, args) + args = aok and decoded or {} + elseif type(args) ~= "table" then + args = {} + end + table.insert(tcs, { + id = tc.id or ("call_" .. #tcs), + type = tc.type or "function", + name = fn.name or "", + arguments = args + }) + end + unified.tool_calls = tcs + -- Ollama reports done_reason "stop" alongside tool calls + unified.finish_reason = "tool_calls" + end end return json.encode(unified) diff --git a/internal/lua/adapters/sensenova.lua b/internal/lua/adapters/sensenova.lua index e78ca45..7df098d 100644 --- a/internal/lua/adapters/sensenova.lua +++ b/internal/lua/adapters/sensenova.lua @@ -61,6 +61,34 @@ function adapter.transform_response(raw_body) -- sensenova-6.8-flash-lite 用 reasoning 字段而不是 reasoning_content unified.reasoning_content = ch.message.reasoning end + -- Tool calls MUST be forwarded. Dropping them while keeping + -- finish_reason="tool_calls" makes the client replay an assistant + -- message whose function name/arguments are empty, and sensenova + -- then rejects the next turn with + -- 400 invalid tool_call function, function/name/arguments cannot be empty + -- i.e. a tool-using conversation dies on its second request. + if type(ch.message.tool_calls) == "table" and #ch.message.tool_calls > 0 then + local tcs = {} + for _, tc in ipairs(ch.message.tool_calls) do + local fn = tc["function"] or {} + -- arguments arrives as a JSON *string* on the wire; the + -- unified shape expects a decoded object. + local args = fn.arguments + if type(args) == "string" then + local args_ok, decoded = pcall(json.decode, args) + args = args_ok and decoded or {} + elseif type(args) ~= "table" then + args = {} + end + table.insert(tcs, { + id = tc.id, + type = tc.type or "function", + name = fn.name, + arguments = args + }) + end + unified.tool_calls = tcs + end end unified.finish_reason = ch.finish_reason or "" end diff --git a/internal/lua/adapters/trae.lua b/internal/lua/adapters/trae.lua index 4a153ed..d05b45a 100644 --- a/internal/lua/adapters/trae.lua +++ b/internal/lua/adapters/trae.lua @@ -25,6 +25,78 @@ function adapter.transform_request(raw_body) return json.encode(req) end +-- parse_text_tool_calls extracts tool calls that an upstream emitted as PLAIN +-- TEXT instead of using the OpenAI tool_calls field. +-- +-- trae-local-api's OpenAI endpoint (/v1/chat/completions) does not read the +-- request's `tools` array at all, so the relayed model is never told the tool +-- schema; it falls back to printing +-- +-- {"name": "get_weather", "arguments": {"location": "北京"}} +-- +-- into message.content, leaves message.tool_calls null, and reports +-- finish_reason="stop". A client following the OpenAI contract therefore never +-- sees a tool call: the agent loop terminates unexpectedly mid-conversation +-- (and a hand-written replay produces an empty function name next turn). +-- +-- Both tag spellings are accepted: the same codebase's Anthropic endpoint +-- instructs models to emit , and models mix the two. Key names vary +-- too (arguments / params / input), so all are tried. +-- +-- Returns (tool_calls_array_or_nil, content_with_blocks_removed). +local function parse_text_tool_calls(content) + if type(content) ~= "string" or content == "" then return nil, content end + if not (content:find("` allows attributes; %s* handles the "" spacing + -- that trae-local-api's own prompt example uses. + collect("]*>%s*(.-)%s*") + collect("]*>%s*(.-)%s*") + + if #tcs == 0 then return nil, content end + + -- drop the blocks from user-visible content; keep any surrounding prose + local stripped = content:gsub("]*>%s*.-%s*", "") + stripped = stripped:gsub("]*>%s*.-%s*", "") + stripped = stripped:gsub("^%s+", ""):gsub("%s+$", "") + return tcs, stripped +end + function adapter.transform_response(raw_body) -- trae-local-api 偶发在非流式请求中返回 SSE 格式数据， -- 表现为多个 data: {...} 行或混合了 reasoning_chunk 等。 @@ -71,15 +143,21 @@ function adapter.transform_response(raw_body) if ch.message.reasoning_content then unified.reasoning_content = ch.message.reasoning_content end - if type(ch.message.tool_calls) == "table" then + if type(ch.message.tool_calls) == "table" and #ch.message.tool_calls > 0 then local tcs = {} for _, tc in ipairs(ch.message.tool_calls) do - local args_ok, args = pcall(json.decode, tc["function"].arguments) - if not args_ok then args = {} end + local fn = tc["function"] or {} + local args = fn.arguments + if type(args) == "string" then + local args_ok, decoded = pcall(json.decode, args) + args = args_ok and decoded or {} + elseif type(args) ~= "table" then + args = {} + end table.insert(tcs, { id = tc.id, type = tc.type or "function", - name = tc["function"].name, + name = fn.name, arguments = args }) end @@ -87,6 +165,22 @@ function adapter.transform_response(raw_body) end end unified.finish_reason = ch.finish_reason or "" + + -- Fallback: trae-local-api's OpenAI endpoint drops the request's + -- `tools` array entirely, so the relayed model is never told the tool + -- schema and instead PRINTS a {...} block into + -- content, leaving message.tool_calls null and finish_reason="stop". + -- A client following the OpenAI contract then sees a normal completion + -- and its agent loop terminates mid-conversation. Recover the + -- structured call so the loop can continue. + if unified.tool_calls == nil then + local recovered, cleaned = parse_text_tool_calls(unified.content) + if recovered then + unified.tool_calls = recovered + unified.content = cleaned + unified.finish_reason = "tool_calls" + end + end end return json.encode(unified) diff --git a/internal/lua/vm_test.go b/internal/lua/vm_test.go index f23618d..46f1feb 100644 --- a/internal/lua/vm_test.go +++ b/internal/lua/vm_test.go @@ -1373,3 +1373,229 @@ func TestBurstLeftoverIsReclaimedUnderSteadyTraffic(t *testing.T) { created, idle) } } + +// TestNonStreamToolCallsPreserved is the regression for the bug that killed +// tool-using conversations on their SECOND request: several adapters handled +// tool_calls in transform_stream_chunk but dropped them in +// transform_response. A client then received finish_reason:"tool_calls" with no +// tool_calls payload, replayed an assistant message whose function +// name/arguments were empty, and the upstream rejected the next turn with +// +// 400 invalid tool_call function, function/name/arguments cannot be empty +// +// Every OpenAI-shaped adapter must forward non-streaming tool calls. +func TestNonStreamToolCallsPreserved(t *testing.T) { + vm := NewVM(freshAdapterDir(t)) + if err := vm.Start(); err != nil { + t.Fatal(err) + } + defer vm.Stop() + + // upstream shape: arguments is a JSON *string*, as OpenAI sends it + body := `{"choices":[{"index":0,"message":{"role":"assistant","content":"",` + + `"tool_calls":[{"index":0,"id":"call_abc","type":"function",` + + `"function":{"name":"get_weather","arguments":"{\"city\":\"Beijing\"}"}}]},` + + `"finish_reason":"tool_calls"}],` + + `"usage":{"prompt_tokens":10,"completion_tokens":5,"total_tokens":15}}` + + for _, name := range []string{ + "openai", "sensenova", "trae", "deepseek", "github", + "groq", "kimicode", "mistral", "opencode", "agentrouter", + } { + out, err := vm.Transform(name, "transform_response", body) + if err != nil { + t.Fatalf("%s transform_response: %v", name, err) + } + var got struct { + FinishReason string `json:"finish_reason"` + ToolCalls []struct { + ID string `json:"id"` + Type string `json:"type"` + Name string `json:"name"` + Arguments map[string]interface{} `json:"arguments"` + } `json:"tool_calls"` + } + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("%s: unmarshal %v (%s)", name, err, out) + } + if len(got.ToolCalls) != 1 { + t.Fatalf("%s: dropped non-streaming tool_calls (client would replay an empty function): %s", name, out) + } + tc := got.ToolCalls[0] + if tc.Name != "get_weather" { + t.Errorf("%s: tool name lost: %s", name, out) + } + if tc.ID != "call_abc" { + t.Errorf("%s: tool id lost: %s", name, out) + } + // arguments must be decoded into an object, not left as a string + if tc.Arguments == nil || tc.Arguments["city"] != "Beijing" { + t.Errorf("%s: arguments not decoded: %s", name, out) + } + if got.FinishReason != "tool_calls" { + t.Errorf("%s: finish_reason lost: %s", name, out) + } + } +} + +// TestGeminiNonStreamToolCalls covers Gemini's native shape (functionCall parts +// under candidates[].content.parts). The streaming path handled these; the +// non-streaming path silently returned text only. +func TestGeminiNonStreamToolCalls(t *testing.T) { + vm := NewVM(freshAdapterDir(t)) + if err := vm.Start(); err != nil { + t.Fatal(err) + } + defer vm.Stop() + + body := `{"candidates":[{"content":{"parts":[` + + `{"text":"checking"},` + + `{"functionCall":{"name":"get_weather","args":{"city":"Beijing"}}}` + + `]},"finishReason":"STOP"}],` + + `"usageMetadata":{"promptTokenCount":8,"candidatesTokenCount":4,"totalTokenCount":12}}` + + out, err := vm.Transform("gemini", "transform_response", body) + if err != nil { + t.Fatalf("gemini transform_response: %v", err) + } + var got struct { + Content string `json:"content"` + FinishReason string `json:"finish_reason"` + ToolCalls []struct { + Name string `json:"name"` + Arguments map[string]interface{} `json:"arguments"` + } `json:"tool_calls"` + } + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("unmarshal: %v (%s)", err, out) + } + if len(got.ToolCalls) != 1 || got.ToolCalls[0].Name != "get_weather" { + t.Fatalf("gemini dropped non-streaming functionCall: %s", out) + } + if got.ToolCalls[0].Arguments["city"] != "Beijing" { + t.Errorf("gemini args lost: %s", out) + } + // a tool call must not be reported as a plain stop + if got.FinishReason != "tool_calls" { + t.Errorf("gemini finish_reason should become tool_calls: %s", out) + } +} + +// TestOllamaNonStreamToolCalls covers Ollama's non-streaming shape +// (message.tool_calls with an already-decoded arguments object). The adapter +// initialized tool_calls to an empty table and never filled it. +func TestOllamaNonStreamToolCalls(t *testing.T) { + vm := NewVM(freshAdapterDir(t)) + if err := vm.Start(); err != nil { + t.Fatal(err) + } + defer vm.Stop() + + body := `{"message":{"role":"assistant","content":"",` + + `"tool_calls":[{"function":{"name":"get_weather","arguments":{"city":"Beijing"}}}]},` + + `"done_reason":"stop","prompt_eval_count":7,"eval_count":3}` + + out, err := vm.Transform("ollama", "transform_response", body) + if err != nil { + t.Fatalf("ollama transform_response: %v", err) + } + var got struct { + FinishReason string `json:"finish_reason"` + ToolCalls []struct { + Name string `json:"name"` + Arguments map[string]interface{} `json:"arguments"` + } `json:"tool_calls"` + } + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("unmarshal: %v (%s)", err, out) + } + if len(got.ToolCalls) != 1 || got.ToolCalls[0].Name != "get_weather" { + t.Fatalf("ollama dropped non-streaming tool_calls: %s", out) + } + if got.ToolCalls[0].Arguments["city"] != "Beijing" { + t.Errorf("ollama args lost: %s", out) + } + if got.FinishReason != "tool_calls" { + t.Errorf("ollama finish_reason should become tool_calls: %s", out) + } +} + +// TestTraeTextToolCallRecovery covers the trae-specific failure: its upstream +// (trae-local-api's OpenAI endpoint) never reads the request's `tools` array, so +// the relayed model prints a block as TEXT, leaves +// message.tool_calls null and reports finish_reason:"stop". An OpenAI client +// then sees an ordinary completion and its agent loop ends mid-conversation. +// The adapter must recover the structured call, strip the block from content, +// and correct finish_reason. +func TestTraeTextToolCallRecovery(t *testing.T) { + vm := NewVM(freshAdapterDir(t)) + if err := vm.Start(); err != nil { + t.Fatal(err) + } + defer vm.Stop() + + cases := []struct { + name string + body string + }{ + { + "tool_call tag with arguments", + `{"choices":[{"message":{"role":"assistant","content":"\n{\"name\": \"get_weather\", \"arguments\": {\"city\": \"Beijing\"}}\n"},"finish_reason":"stop"}]}`, + }, + { + // the same codebase's Anthropic endpoint prompts for and "params" + "toolcall tag with params", + `{"choices":[{"message":{"role":"assistant","content":"{\"name\": \"get_weather\", \"params\": {\"city\": \"Beijing\"}}"},"finish_reason":"stop"}]}`, + }, + { + "prose around the block is preserved", + `{"choices":[{"message":{"role":"assistant","content":"Let me check.\n{\"name\":\"get_weather\",\"arguments\":{\"city\":\"Beijing\"}}"},"finish_reason":"stop"}]}`, + }, + } + + for _, c := range cases { + out, err := vm.Transform("trae", "transform_response", c.body) + if err != nil { + t.Fatalf("%s: transform: %v", c.name, err) + } + var got struct { + Content string `json:"content"` + FinishReason string `json:"finish_reason"` + ToolCalls []struct { + Name string `json:"name"` + Arguments map[string]interface{} `json:"arguments"` + } `json:"tool_calls"` + } + if err := json.Unmarshal([]byte(out), &got); err != nil { + t.Fatalf("%s: unmarshal %v (%s)", c.name, err, out) + } + if len(got.ToolCalls) != 1 { + t.Fatalf("%s: text tool call not recovered: %s", c.name, out) + } + if got.ToolCalls[0].Name != "get_weather" { + t.Errorf("%s: name wrong: %s", c.name, out) + } + if got.ToolCalls[0].Arguments["city"] != "Beijing" { + t.Errorf("%s: args wrong: %s", c.name, out) + } + if got.FinishReason != "tool_calls" { + t.Errorf("%s: finish_reason must be corrected to tool_calls: %s", c.name, out) + } + if strings.Contains(got.Content, "