From afebdf9efc9f129a753ae70c2e5721723896b9d5 Mon Sep 17 00:00:00 2001 From: tejas ghatte Date: Tue, 30 Jun 2026 23:41:26 +0530 Subject: [PATCH] fix: gemini openai through signature compatibility --- core/providers/gemini/responses.go | 4 + core/providers/gemini/types.go | 2 +- core/providers/openai/chat_test.go | 106 ++++++++++++++++++++++++ core/providers/openai/responses.go | 14 ++++ core/providers/openai/responses_test.go | 62 ++++++++++++++ core/providers/openai/utils.go | 40 ++++++++- core/providers/utils/utils.go | 16 ++++ core/providers/utils/utils_test.go | 21 +++++ 8 files changed, 263 insertions(+), 2 deletions(-) diff --git a/core/providers/gemini/responses.go b/core/providers/gemini/responses.go index 99b28bd2171..d16dc2cfe7f 100644 --- a/core/providers/gemini/responses.go +++ b/core/providers/gemini/responses.go @@ -3233,6 +3233,10 @@ func convertResponsesMessagesToGeminiContents(messages []schemas.ResponsesMessag } } + if part.ThoughtSignature == nil { + part.ThoughtSignature = []byte(skipThoughtSignatureValidator) + } + content.Parts = append(content.Parts, part) } diff --git a/core/providers/gemini/types.go b/core/providers/gemini/types.go index 6802a6a8d33..9699ca309d7 100644 --- a/core/providers/gemini/types.go +++ b/core/providers/gemini/types.go @@ -42,7 +42,7 @@ var thinkingBudgetRanges = []struct { } // thoughtSignatureSeparator is used to separate the base ID from the thought signature in tool IDs -const thoughtSignatureSeparator = "_ts_" +const thoughtSignatureSeparator = providerUtils.ThoughtSignatureSeparator type Role string diff --git a/core/providers/openai/chat_test.go b/core/providers/openai/chat_test.go index d02946ae547..0d8a7d530f6 100644 --- a/core/providers/openai/chat_test.go +++ b/core/providers/openai/chat_test.go @@ -1225,3 +1225,109 @@ func TestOpenAIInbound_ServerToolNameSurvives(t *testing.T) { t.Fatalf("ToBifrostChatRequest dropped name: %+v", bifReq.Params) } } + +// When a conversation switches from Gemini to OpenAI, Gemini's thoughtSignature is +// embedded in the tool call_id as "_ts_" and can exceed OpenAI's 64-char +// limit. The chat converter must strip it to the base ID on the wire while leaving the +// caller's input intact (so a later Gemini turn can still recover the signature). +func TestToOpenAIChatRequest_StripsThoughtSignatureFromToolCallIDs(t *testing.T) { + embeddedID := "search" + providerUtils.ThoughtSignatureSeparator + strings.Repeat("A", 6000) + + req := &schemas.BifrostChatRequest{ + Provider: schemas.OpenAI, + Model: "gpt-4o", + Input: []schemas.ChatMessage{ + { + Role: schemas.ChatMessageRoleAssistant, + ChatAssistantMessage: &schemas.ChatAssistantMessage{ + ToolCalls: []schemas.ChatAssistantMessageToolCall{{ + ID: schemas.Ptr(embeddedID), + Type: schemas.Ptr("function"), + Function: schemas.ChatAssistantMessageToolCallFunction{ + Name: schemas.Ptr("search"), + Arguments: "{}", + }, + }}, + }, + }, + { + Role: schemas.ChatMessageRoleTool, + ChatToolMessage: &schemas.ChatToolMessage{ToolCallID: schemas.Ptr(embeddedID)}, + Content: &schemas.ChatMessageContent{ContentStr: schemas.Ptr("result")}, + }, + }, + } + + ctx, cancel := schemas.NewBifrostContextWithCancel(nil) + defer cancel() + result := ToOpenAIChatRequest(ctx, req) + require.NotNil(t, result) + + gotCallID := *result.Messages[0].OpenAIChatAssistantMessage.ToolCalls[0].ID + gotToolCallID := *result.Messages[1].ChatToolMessage.ToolCallID + + if gotCallID != "search" { + t.Errorf("assistant tool call ID: got %q, want %q", gotCallID, "search") + } + if len(gotCallID) > 64 { + t.Errorf("assistant tool call ID exceeds OpenAI's 64-char limit: %d chars", len(gotCallID)) + } + if gotToolCallID != gotCallID { + t.Errorf("tool result ID %q must match assistant call ID %q", gotToolCallID, gotCallID) + } + + // The caller's history must be untouched. + if *req.Input[0].ChatAssistantMessage.ToolCalls[0].ID != embeddedID { + t.Error("original assistant tool call ID was mutated") + } + if *req.Input[1].ChatToolMessage.ToolCallID != embeddedID { + t.Error("original tool result tool_call_id was mutated") + } +} + +// A short call id that merely contains "_ts_" (e.g. two distinct raw upstream ids) must be +// left intact: stripping only kicks in above OpenAI's 64-char limit, so distinct ids never +// collapse into one. +func TestToOpenAIChatRequest_PreservesShortToolCallIDsContainingSeparator(t *testing.T) { + req := &schemas.BifrostChatRequest{ + Provider: schemas.OpenAI, + Model: "gpt-4o", + Input: []schemas.ChatMessage{ + { + Role: schemas.ChatMessageRoleAssistant, + ChatAssistantMessage: &schemas.ChatAssistantMessage{ + ToolCalls: []schemas.ChatAssistantMessageToolCall{ + { + ID: schemas.Ptr("search_ts_a"), + Type: schemas.Ptr("function"), + Function: schemas.ChatAssistantMessageToolCallFunction{Name: schemas.Ptr("search"), Arguments: "{}"}, + }, + { + ID: schemas.Ptr("search_ts_b"), + Type: schemas.Ptr("function"), + Function: schemas.ChatAssistantMessageToolCallFunction{Name: schemas.Ptr("search"), Arguments: "{}"}, + }, + }, + }, + }, + { + Role: schemas.ChatMessageRoleTool, + ChatToolMessage: &schemas.ChatToolMessage{ToolCallID: schemas.Ptr("search_ts_a")}, + Content: &schemas.ChatMessageContent{ContentStr: schemas.Ptr("r")}, + }, + }, + } + + ctx, cancel := schemas.NewBifrostContextWithCancel(nil) + defer cancel() + result := ToOpenAIChatRequest(ctx, req) + require.NotNil(t, result) + + got := result.Messages[0].OpenAIChatAssistantMessage.ToolCalls + if *got[0].ID != "search_ts_a" || *got[1].ID != "search_ts_b" { + t.Errorf("distinct short ids must be preserved, got %q and %q", *got[0].ID, *got[1].ID) + } + if *result.Messages[1].ChatToolMessage.ToolCallID != "search_ts_a" { + t.Errorf("short tool_call_id must be preserved, got %q", *result.Messages[1].ChatToolMessage.ToolCallID) + } +} diff --git a/core/providers/openai/responses.go b/core/providers/openai/responses.go index 3ab57be9658..152fcc5a3c4 100644 --- a/core/providers/openai/responses.go +++ b/core/providers/openai/responses.go @@ -92,6 +92,20 @@ func ToOpenAIResponsesRequest(ctx *schemas.BifrostContext, bifrostReq *schemas.B } } + // Strip provider reasoning signatures (e.g. Gemini thoughtSignatures smuggled into + // call_id as "_ts_") from tool call IDs, but only when the id exceeds + // OpenAI's limit — shorter IDs are left intact so distinct upstream IDs are preserved. + // Deterministic, so a call and its output still match. Clone first — the + // ResponsesToolMessage pointer is shared with the caller's input. + if message.ResponsesToolMessage != nil && message.ResponsesToolMessage.CallID != nil && + len(*message.ResponsesToolMessage.CallID) > MaxToolCallIDLength { + if stripped := utils.StripThoughtSignature(*message.ResponsesToolMessage.CallID); stripped != *message.ResponsesToolMessage.CallID { + toolMsgCopy := *message.ResponsesToolMessage + toolMsgCopy.CallID = &stripped + message.ResponsesToolMessage = &toolMsgCopy + } + } + if message.ResponsesReasoning != nil { isGptOss := strings.Contains(capModel, "gpt-oss") isReasoning := isOpenAIReasoningModel(capModel) diff --git a/core/providers/openai/responses_test.go b/core/providers/openai/responses_test.go index 999cb277bb4..65151ab0b8b 100644 --- a/core/providers/openai/responses_test.go +++ b/core/providers/openai/responses_test.go @@ -2018,3 +2018,65 @@ func TestToOpenAIResponsesRequest_OpenRouterServerToolsPreserved(t *testing.T) { } }) } + +// Reverse-direction guard for the Responses path: a Gemini thoughtSignature embedded in +// call_id ("_ts_") must be stripped to the base ID before reaching OpenAI, +// which rejects input[].id over 64 chars. The call and its output strip identically so +// they still pair, and the caller's input is left intact. +func TestToOpenAIResponsesRequest_StripsThoughtSignatureFromCallID(t *testing.T) { + // "_ts_" is the separator used by the native Gemini converters to embed signatures. + embeddedID := "search_ts_" + strings.Repeat("A", 6000) + + req := &schemas.BifrostResponsesRequest{ + Provider: schemas.OpenAI, + Model: "gpt-4o", + Input: []schemas.ResponsesMessage{ + { + Type: schemas.Ptr(schemas.ResponsesMessageTypeFunctionCall), + ResponsesToolMessage: &schemas.ResponsesToolMessage{ + CallID: schemas.Ptr(embeddedID), + Name: schemas.Ptr("search"), + Arguments: schemas.Ptr("{}"), + }, + }, + { + Type: schemas.Ptr(schemas.ResponsesMessageTypeFunctionCallOutput), + ResponsesToolMessage: &schemas.ResponsesToolMessage{ + CallID: schemas.Ptr(embeddedID), + Output: &schemas.ResponsesToolMessageOutputStruct{ + ResponsesToolCallOutputStr: schemas.Ptr("result"), + }, + }, + }, + }, + } + + ctx, cancel := schemas.NewBifrostContextWithCancel(nil) + defer cancel() + result := ToOpenAIResponsesRequest(ctx, req) + if result == nil { + t.Fatal("expected non-nil result") + } + + out := result.Input.OpenAIResponsesRequestInputArray + callID := *out[0].ResponsesToolMessage.CallID + outputCallID := *out[1].ResponsesToolMessage.CallID + + if callID != "search" { + t.Errorf("function_call id: got %q, want %q", callID, "search") + } + if len(callID) > 64 { + t.Errorf("function_call id exceeds OpenAI's 64-char limit: %d chars", len(callID)) + } + if outputCallID != callID { + t.Errorf("function_call_output id %q must match function_call id %q", outputCallID, callID) + } + + // The caller's history must be untouched so a later Gemini turn can recover the signature. + if *req.Input[0].ResponsesToolMessage.CallID != embeddedID { + t.Error("original function_call call_id was mutated") + } + if *req.Input[1].ResponsesToolMessage.CallID != embeddedID { + t.Error("original function_call_output call_id was mutated") + } +} diff --git a/core/providers/openai/utils.go b/core/providers/openai/utils.go index 881324980a4..1da19c7afca 100644 --- a/core/providers/openai/utils.go +++ b/core/providers/openai/utils.go @@ -3,6 +3,7 @@ package openai import ( "strings" + "github.com/maximhq/bifrost/core/providers/utils" "github.com/maximhq/bifrost/core/schemas" ) @@ -40,12 +41,46 @@ func ConvertBifrostMessagesToOpenAIMessages(messages []schemas.ChatMessage) []Op Content: message.Content, ChatToolMessage: message.ChatToolMessage, } + // Strip provider reasoning signatures (e.g. Gemini thoughtSignatures embedded in + // call_id as "_ts_") from the tool result's tool_call_id, but only when it + // exceeds OpenAI's limit — shorter IDs are left intact so distinct upstream IDs are + // preserved. Clone first — ChatToolMessage is shared with the caller's input. + if message.ChatToolMessage != nil && message.ChatToolMessage.ToolCallID != nil && + len(*message.ChatToolMessage.ToolCallID) > MaxToolCallIDLength { + if stripped := utils.StripThoughtSignature(*message.ChatToolMessage.ToolCallID); stripped != *message.ChatToolMessage.ToolCallID { + toolMsgCopy := *message.ChatToolMessage + toolMsgCopy.ToolCallID = &stripped + openaiMessages[i].ChatToolMessage = &toolMsgCopy + } + } if message.ChatAssistantMessage != nil { + // Strip the same embedded signature from over-long assistant tool call IDs. Clone the + // slice only when a strip is actually needed so the caller's input is never mutated. + toolCalls := message.ChatAssistantMessage.ToolCalls + needsStrip := false + for j := range toolCalls { + if toolCalls[j].ID != nil && len(*toolCalls[j].ID) > MaxToolCallIDLength && + strings.Contains(*toolCalls[j].ID, utils.ThoughtSignatureSeparator) { + needsStrip = true + break + } + } + if needsStrip { + cloned := make([]schemas.ChatAssistantMessageToolCall, len(toolCalls)) + copy(cloned, toolCalls) + for j := range cloned { + if cloned[j].ID != nil && len(*cloned[j].ID) > MaxToolCallIDLength { + stripped := utils.StripThoughtSignature(*cloned[j].ID) + cloned[j].ID = &stripped + } + } + toolCalls = cloned + } openaiMessages[i].OpenAIChatAssistantMessage = &OpenAIChatAssistantMessage{ Refusal: message.ChatAssistantMessage.Refusal, Reasoning: message.ChatAssistantMessage.Reasoning, Annotations: message.ChatAssistantMessage.Annotations, - ToolCalls: message.ChatAssistantMessage.ToolCalls, + ToolCalls: toolCalls, } } } @@ -144,6 +179,9 @@ func supportsMaxReasoningEffort(model string) bool { // MaxUserFieldLength for OpenAI enforces a 64 character maximum on the user field const MaxUserFieldLength = 64 +// MaxToolCallIDLength is OpenAI's 64 character maximum on tool call IDs (call_id / input[].id). +const MaxToolCallIDLength = 64 + // SanitizeUserField returns nil if user exceeds MaxUserFieldLength, otherwise returns the original value func SanitizeUserField(user *string) *string { if user != nil && len(*user) > MaxUserFieldLength { diff --git a/core/providers/utils/utils.go b/core/providers/utils/utils.go index 03822dae129..681577a3df0 100644 --- a/core/providers/utils/utils.go +++ b/core/providers/utils/utils.go @@ -35,6 +35,22 @@ import ( "github.com/valyala/fasthttp/fasthttpproxy" ) +// ThoughtSignatureSeparator delimits a tool call's base ID from a provider reasoning +// signature embedded in the call_id (e.g. Gemini thoughtSignatures), formatted as +// "_ts_". +const ThoughtSignatureSeparator = "_ts_" + +// StripThoughtSignature returns the base tool-call ID without any embedded provider +// reasoning signature. It is deterministic, so a tool call and its matching output strip +// to the same ID. Providers that cannot use the signature (e.g. OpenAI, which caps call_id +// at 64 chars) call this before sending the ID upstream. +func StripThoughtSignature(callID string) string { + if base, _, found := strings.Cut(callID, ThoughtSignatureSeparator); found { + return base + } + return callID +} + // sortedAPI is a sonic encoder/decoder that sorts map keys during marshaling. // This ensures deterministic JSON output for map[string]interface{} values, // which is critical for LLM prompt caching (e.g., Anthropic cache keying). diff --git a/core/providers/utils/utils_test.go b/core/providers/utils/utils_test.go index 517da6bbaf3..bffa482abf7 100644 --- a/core/providers/utils/utils_test.go +++ b/core/providers/utils/utils_test.go @@ -1878,3 +1878,24 @@ func TestExtractPassthroughProviderResponseHeaders(t *testing.T) { t.Fatalf("benign header x-request-id was dropped: %v", headers) } } + +func TestStripThoughtSignature(t *testing.T) { + cases := []struct { + name string + in string + want string + }{ + {"no separator", "call_abc123", "call_abc123"}, + {"gemini embedded signature", "search_ts_QUJDREVG", "search"}, + {"base id is also a gemini id", "fc_123_ts_QUJD", "fc_123"}, + {"separator only", "_ts_QUJD", ""}, + {"empty", "", ""}, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + if got := StripThoughtSignature(tc.in); got != tc.want { + t.Errorf("StripThoughtSignature(%q) = %q, want %q", tc.in, got, tc.want) + } + }) + } +}