From 9f62c8df28dc749ea976865450a458917bf45042 Mon Sep 17 00:00:00 2001 From: Luis Pater Date: Wed, 15 Jul 2026 03:14:05 +0800 Subject: [PATCH] feat(translator): enhance handling of custom tool calls and improve tool call batching logic - Introduced support for custom tool calls in request serialization, including preservation of call IDs, input, and output. - Enhanced tool call batching logic to handle ambiguous, missing, and reused call IDs across assistant messages. - Updated processing of tool call outputs to ensure proper matching with pending calls and introduced synthesized IDs when necessary. - Added extensive test cases to validate behavior for custom tool call histories, mixed call types, and edge cases with ambiguous or orphaned IDs. Closes: #4256 --- .../chat-completions/codex_openai_request.go | 126 +++++++-- .../codex_openai_request_test.go | 267 ++++++++++++++++++ 2 files changed, 375 insertions(+), 18 deletions(-) diff --git a/internal/translator/codex/openai/chat-completions/codex_openai_request.go b/internal/translator/codex/openai/chat-completions/codex_openai_request.go index 046216b42..a6f3ede9c 100644 --- a/internal/translator/codex/openai/chat-completions/codex_openai_request.go +++ b/internal/translator/codex/openai/chat-completions/codex_openai_request.go @@ -93,6 +93,14 @@ func ConvertOpenAIRequestToCodex(modelName string, inputRawJSON []byte, stream b // Extract system instructions from first system message (string or text object) messages := gjson.GetBytes(rawJSON, "messages") + type pendingToolCall struct { + callID string + sourceCallID string + callType string + consumed bool + } + var pendingToolCalls []pendingToolCall + ambiguousToolCallIDs := map[string]struct{}{} // if messages.IsArray() { // arr := messages.Array() // for i := 0; i < len(arr); i++ { @@ -119,18 +127,46 @@ func ConvertOpenAIRequestToCodex(modelName string, inputRawJSON []byte, stream b switch role { case "tool": - // Handle tool response messages as top-level function_call_output objects + // Handle tool response messages as top-level tool call output objects. toolCallID := m.Get("tool_call_id").String() - content := m.Get("content") + if _, ambiguous := ambiguousToolCallIDs[toolCallID]; toolCallID != "" && ambiguous { + continue + } - // Create function_call_output object - funcOutput := []byte(`{}`) - funcOutput, _ = sjson.SetBytes(funcOutput, "type", "function_call_output") - funcOutput, _ = sjson.SetBytes(funcOutput, "call_id", toolCallID) - funcOutput = setToolCallOutputContent(funcOutput, content) - out, _ = sjson.SetRawBytes(out, "input.-1", funcOutput) + pendingIndex := -1 + for index := range pendingToolCalls { + pendingCall := &pendingToolCalls[index] + if pendingCall.consumed { + continue + } + if toolCallID == "" || pendingCall.sourceCallID == toolCallID || pendingCall.callID == toolCallID { + pendingIndex = index + break + } + } + + if pendingIndex < 0 { + continue + } + pendingCall := &pendingToolCalls[pendingIndex] + pendingCall.consumed = true + toolCallID = pendingCall.callID + outputType := "function_call_output" + if pendingCall.callType == "custom" { + outputType = "custom_tool_call_output" + } + + toolOutput := []byte(`{}`) + toolOutput, _ = sjson.SetBytes(toolOutput, "type", outputType) + toolOutput, _ = sjson.SetBytes(toolOutput, "call_id", toolCallID) + toolOutput = setToolCallOutputContent(toolOutput, m.Get("content")) + out, _ = sjson.SetRawBytes(out, "input.-1", toolOutput) default: + // A new conversational message starts a new tool-call batch. + pendingToolCalls = nil + ambiguousToolCallIDs = map[string]struct{}{} + // Handle regular messages msg := []byte(`{}`) msg, _ = sjson.SetBytes(msg, "type", "message") @@ -223,24 +259,78 @@ func ConvertOpenAIRequestToCodex(modelName string, inputRawJSON []byte, stream b toolCalls := m.Get("tool_calls") if toolCalls.Exists() && toolCalls.IsArray() { toolCallsArr := toolCalls.Array() + callIDCounts := map[string]int{} + usedCallIDs := map[string]struct{}{} + for _, tc := range toolCallsArr { + toolCallType := tc.Get("type").String() + callID := tc.Get("id").String() + if (toolCallType == "function" || toolCallType == "custom") && callID != "" { + callIDCounts[callID]++ + usedCallIDs[callID] = struct{}{} + } + } + for callID, count := range callIDCounts { + if count > 1 { + ambiguousToolCallIDs[callID] = struct{}{} + } + } + for j := 0; j < len(toolCallsArr); j++ { tc := toolCallsArr[j] - if tc.Get("type").String() == "function" { + toolCallType := tc.Get("type").String() + if toolCallType != "function" && toolCallType != "custom" { + continue + } + sourceCallID := tc.Get("id").String() + if _, ambiguous := ambiguousToolCallIDs[sourceCallID]; sourceCallID != "" && ambiguous { + continue + } + callID := sourceCallID + if callID == "" { + baseCallID := "call_missing_" + strconv.Itoa(i) + "_" + strconv.Itoa(j) + callID = baseCallID + for suffix := 1; ; suffix++ { + if _, used := usedCallIDs[callID]; !used { + break + } + callID = baseCallID + "_" + strconv.Itoa(suffix) + } + usedCallIDs[callID] = struct{}{} + } + pendingToolCalls = append(pendingToolCalls, pendingToolCall{ + callID: callID, + sourceCallID: sourceCallID, + callType: toolCallType, + }) + + switch toolCallType { + case "function": // Create function_call as top-level object funcCall := []byte(`{}`) funcCall, _ = sjson.SetBytes(funcCall, "type", "function_call") - funcCall, _ = sjson.SetBytes(funcCall, "call_id", tc.Get("id").String()) - { - name := tc.Get("function.name").String() - if short, ok := originalToolNameMap[name]; ok { - name = short - } else { - name = shortenNameIfNeeded(name) - } - funcCall, _ = sjson.SetBytes(funcCall, "name", name) + funcCall, _ = sjson.SetBytes(funcCall, "call_id", callID) + name := tc.Get("function.name").String() + if short, ok := originalToolNameMap[name]; ok { + name = short + } else { + name = shortenNameIfNeeded(name) } + funcCall, _ = sjson.SetBytes(funcCall, "name", name) funcCall, _ = sjson.SetBytes(funcCall, "arguments", tc.Get("function.arguments").String()) out, _ = sjson.SetRawBytes(out, "input.-1", funcCall) + case "custom": + customCall := []byte(`{}`) + customCall, _ = sjson.SetBytes(customCall, "type", "custom_tool_call") + customCall, _ = sjson.SetBytes(customCall, "call_id", callID) + name := tc.Get("custom.name").String() + if short, ok := originalToolNameMap[name]; ok { + name = short + } else { + name = shortenNameIfNeeded(name) + } + customCall, _ = sjson.SetBytes(customCall, "name", name) + customCall, _ = sjson.SetBytes(customCall, "input", tc.Get("custom.input").String()) + out, _ = sjson.SetRawBytes(out, "input.-1", customCall) } } } diff --git a/internal/translator/codex/openai/chat-completions/codex_openai_request_test.go b/internal/translator/codex/openai/chat-completions/codex_openai_request_test.go index 5be9c8b85..32773ce23 100644 --- a/internal/translator/codex/openai/chat-completions/codex_openai_request_test.go +++ b/internal/translator/codex/openai/chat-completions/codex_openai_request_test.go @@ -804,6 +804,273 @@ func TestCallIDsMatchBetweenCallAndOutput(t *testing.T) { } } +func TestCustomToolCallHistory(t *testing.T) { + input := []byte(`{ + "model": "gpt-5.6-sol", + "messages": [ + {"role": "user", "content": "Update the specification."}, + { + "role": "assistant", + "content": "I will update the file.", + "tool_calls": [ + { + "id": "call_apply_patch", + "type": "custom", + "custom": { + "name": "apply_patch", + "input": "*** Begin Patch\n*** Add File: spec.md\n+done\n*** End Patch" + } + } + ] + }, + { + "role": "tool", + "tool_call_id": "call_apply_patch", + "content": "Added spec.md" + } + ], + "tools": [ + { + "type": "function", + "function": { + "name": "apply_patch", + "description": "Apply a freeform patch.", + "parameters": { + "type": "object", + "properties": {"input": {"type": "string"}}, + "required": ["input"] + } + } + } + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 4 { + t.Fatalf("expected 4 input items, got %d: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + + customCall := items[2] + if customCall.Get("type").String() != "custom_tool_call" { + t.Fatalf("expected custom_tool_call, got %s", customCall.Raw) + } + if customCall.Get("call_id").String() != "call_apply_patch" { + t.Fatalf("expected custom call_id to be preserved, got %s", customCall.Raw) + } + if customCall.Get("name").String() != "apply_patch" { + t.Fatalf("expected custom tool name apply_patch, got %s", customCall.Raw) + } + if customCall.Get("input").String() != "*** Begin Patch\n*** Add File: spec.md\n+done\n*** End Patch" { + t.Fatalf("expected custom tool input to be preserved, got %s", customCall.Raw) + } + + customOutput := items[3] + if customOutput.Get("type").String() != "custom_tool_call_output" { + t.Fatalf("expected custom_tool_call_output, got %s", customOutput.Raw) + } + if customOutput.Get("call_id").String() != "call_apply_patch" { + t.Fatalf("expected custom output call_id to be preserved, got %s", customOutput.Raw) + } + if customOutput.Get("output").String() != "Added spec.md" { + t.Fatalf("expected custom tool output to be preserved, got %s", customOutput.Raw) + } +} + +func TestMixedToolCallHistoryPreservesCallFamilies(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"user","content":"Run both tools."}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_function","type":"function","function":{"name":"lookup","arguments":"{}"}}, + {"id":"call_custom","type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","tool_call_id":"call_custom","content":"patched"}, + {"role":"tool","tool_call_id":"call_function","content":"found"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 5 { + t.Fatalf("expected 5 input items, got %d: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + + expectedTypes := []string{"message", "function_call", "custom_tool_call", "custom_tool_call_output", "function_call_output"} + for i, expectedType := range expectedTypes { + if got := items[i].Get("type").String(); got != expectedType { + t.Fatalf("item %d: expected type %s, got %s: %s", i, expectedType, got, items[i].Raw) + } + } + if got := items[3].Get("call_id").String(); got != "call_custom" { + t.Fatalf("expected custom output call_id call_custom, got %s", items[3].Raw) + } + if got := items[4].Get("call_id").String(); got != "call_function" { + t.Fatalf("expected function output call_id call_function, got %s", items[4].Raw) + } +} + +func TestToolCallHistoryAllowsReusedCallIDAcrossRounds(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"user","content":"Run the first tool."}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_reused","type":"function","function":{"name":"lookup","arguments":"{}"}} + ]}, + {"role":"tool","tool_call_id":"call_reused","content":"found"}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_reused","type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","tool_call_id":"call_reused","content":"patched"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 5 { + t.Fatalf("expected 5 input items, got %d: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + if got := items[2].Get("type").String(); got != "function_call_output" { + t.Fatalf("expected first reused call output to remain function_call_output, got %s", items[2].Raw) + } + if got := items[4].Get("type").String(); got != "custom_tool_call_output" { + t.Fatalf("expected second reused call output to be custom_tool_call_output, got %s", items[4].Raw) + } +} + +func TestCustomToolCallHistorySynthesizesMissingCallID(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"tool","content":"orphan"}, + {"role":"assistant","content":null,"tool_calls":[ + {"type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","content":"patched"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 2 { + t.Fatalf("expected orphan output to be dropped and missing ID pair preserved, got %d items: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + if got := items[0].Get("type").String(); got != "custom_tool_call" { + t.Fatalf("expected custom_tool_call, got %s", items[0].Raw) + } + if got := items[1].Get("type").String(); got != "custom_tool_call_output" { + t.Fatalf("expected custom_tool_call_output, got %s", items[1].Raw) + } + callID := items[0].Get("call_id").String() + if callID == "" { + t.Fatalf("expected synthesized call_id, got %s", items[0].Raw) + } + if got := items[1].Get("call_id").String(); got != callID { + t.Fatalf("expected synthesized call_id %q on output, got %s", callID, items[1].Raw) + } +} + +func TestToolCallHistoryClearsUnmatchedCallAtNewBatch(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_reused","type":"custom","custom":{"name":"apply_patch","input":"old patch"}} + ]}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_reused","type":"function","function":{"name":"lookup","arguments":"{}"}} + ]}, + {"role":"tool","tool_call_id":"call_reused","content":"found"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 3 { + t.Fatalf("expected two calls and one output, got %d items: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + if got := items[2].Get("type").String(); got != "function_call_output" { + t.Fatalf("expected new batch output to match function call, got %s", items[2].Raw) + } +} + +func TestToolCallOutputWithoutIDUsesPendingCall(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_explicit","type":"function","function":{"name":"lookup","arguments":"{}"}}, + {"type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","content":"found"}, + {"role":"tool","content":"patched"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 4 { + t.Fatalf("expected two calls and two outputs, got %d items: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + if got := items[2].Get("type").String(); got != "function_call_output" { + t.Fatalf("expected first empty-ID output to match function call, got %s", items[2].Raw) + } + if got := items[2].Get("call_id").String(); got != "call_explicit" { + t.Fatalf("expected explicit pending call_id, got %s", items[2].Raw) + } + if got := items[3].Get("type").String(); got != "custom_tool_call_output" { + t.Fatalf("expected second empty-ID output to match custom call, got %s", items[3].Raw) + } + if got := items[3].Get("call_id").String(); got == "" { + t.Fatalf("expected synthesized custom output call_id, got %s", items[3].Raw) + } +} + +func TestAmbiguousDuplicateToolCallIDsAreDropped(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"user","content":"Run both tools."}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_duplicate","type":"function","function":{"name":"lookup","arguments":"{}"}}, + {"id":"call_duplicate","type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","tool_call_id":"call_duplicate","content":"first"}, + {"role":"tool","tool_call_id":"call_duplicate","content":"second"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 1 || items[0].Get("role").String() != "user" { + t.Fatalf("expected ambiguous calls and outputs to be dropped, got %s", gjson.GetBytes(out, "input").Raw) + } +} + +func TestOrphanAndDuplicateToolCallOutputsAreDropped(t *testing.T) { + input := []byte(`{ + "messages": [ + {"role":"tool","tool_call_id":"call_orphan","content":"orphan"}, + {"role":"assistant","content":null,"tool_calls":[ + {"id":"call_custom","type":"custom","custom":{"name":"apply_patch","input":"patch"}} + ]}, + {"role":"tool","tool_call_id":"call_custom","content":"patched"}, + {"role":"tool","tool_call_id":"call_custom","content":"duplicate"} + ] + }`) + + out := ConvertOpenAIRequestToCodex("gpt-5.6-sol", input, true) + items := gjson.GetBytes(out, "input").Array() + if len(items) != 2 { + t.Fatalf("expected only the matched call and first output, got %d items: %s", len(items), gjson.GetBytes(out, "input").Raw) + } + if got := items[0].Get("type").String(); got != "custom_tool_call" { + t.Fatalf("expected custom_tool_call, got %s", items[0].Raw) + } + if got := items[1].Get("type").String(); got != "custom_tool_call_output" { + t.Fatalf("expected custom_tool_call_output, got %s", items[1].Raw) + } + if got := items[1].Get("output").String(); got != "patched" { + t.Fatalf("expected first matched output to be preserved, got %s", items[1].Raw) + } +} + // Tools array should carry over to the Responses format output. func TestToolsDefinitionTranslated(t *testing.T) { input := []byte(`{