mirror of
https://gitcode.com/JianFeeeee/ModelRouter.git
synced 2026-09-20 08:57:57 +00:00
fix(adapters): stop dropping non-streaming tool calls (agent loops died on turn 2)
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 <tool_call>{...}</tool_call> 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
(<tool_call>/<toolcall>, 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.
This commit is contained in:
@ -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 <tool_call> 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":"<tool_call>\n{\"name\": \"get_weather\", \"arguments\": {\"city\": \"Beijing\"}}\n</tool_call>"},"finish_reason":"stop"}]}`,
|
||||
},
|
||||
{
|
||||
// the same codebase's Anthropic endpoint prompts for <toolcall> and "params"
|
||||
"toolcall tag with params",
|
||||
`{"choices":[{"message":{"role":"assistant","content":"<toolcall>{\"name\": \"get_weather\", \"params\": {\"city\": \"Beijing\"}}</toolcall>"},"finish_reason":"stop"}]}`,
|
||||
},
|
||||
{
|
||||
"prose around the block is preserved",
|
||||
`{"choices":[{"message":{"role":"assistant","content":"Let me check.\n<tool_call>{\"name\":\"get_weather\",\"arguments\":{\"city\":\"Beijing\"}}</tool_call>"},"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, "<tool_call") || strings.Contains(got.Content, "<toolcall") {
|
||||
t.Errorf("%s: raw block left in user-visible content: %s", c.name, out)
|
||||
}
|
||||
}
|
||||
|
||||
// a plain text answer must be untouched
|
||||
plain := `{"choices":[{"message":{"role":"assistant","content":"just text"},"finish_reason":"stop"}]}`
|
||||
out, err := vm.Transform("trae", "transform_response", plain)
|
||||
if err != nil {
|
||||
t.Fatalf("plain: %v", err)
|
||||
}
|
||||
if strings.Contains(out, "tool_calls") {
|
||||
t.Errorf("plain answer must not gain tool_calls: %s", out)
|
||||
}
|
||||
if !strings.Contains(out, `"finish_reason":"stop"`) {
|
||||
t.Errorf("plain answer finish_reason changed: %s", out)
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user