-
Notifications
You must be signed in to change notification settings - Fork 1.3k
[fix]: set id and summary on Gemini non-streaming Responses reasoning items #6369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: dev
Are you sure you want to change the base?
Changes from all commits
8414e95
e656c23
751317e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,238 @@ | ||
| package gemini | ||
|
|
||
| import ( | ||
| "strings" | ||
| "testing" | ||
|
|
||
| schemas "github.com/maximhq/bifrost/core/schemas" | ||
| ) | ||
|
|
||
| // Regression test for issue #6329: the non-streaming Gemini -> Responses conversion | ||
| // used to emit reasoning output items without `id` and (when no thought signature was | ||
| // present) without `summary`. Strict OpenAI Responses clients (e.g. the Vercel AI SDK) | ||
| // reject the whole output array on that shape: | ||
| // | ||
| // output[0].id: expected string, received undefined | ||
| // output[0].summary: expected array, received undefined | ||
| // | ||
| // which also discards a perfectly valid function_call that follows the reasoning item. | ||
| func TestReasoningItemIDAndSummary(t *testing.T) { | ||
| buildResp := func(parts []*Part) *GenerateContentResponse { | ||
| return &GenerateContentResponse{ | ||
| ModelVersion: "gemini-2.5-pro", | ||
| Candidates: []*Candidate{ | ||
| { | ||
| FinishReason: FinishReasonStop, | ||
| Content: &Content{ | ||
| Role: "model", | ||
| Parts: parts, | ||
| }, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| assertValidReasoningItem := func(t *testing.T, reasoning schemas.ResponsesMessage) { | ||
| t.Helper() | ||
| if reasoning.Type == nil || *reasoning.Type != schemas.ResponsesMessageTypeReasoning { | ||
| t.Fatalf("expected a reasoning item, got type %+v", reasoning.Type) | ||
| } | ||
| if reasoning.ID == nil || strings.TrimSpace(*reasoning.ID) == "" { | ||
| t.Errorf("reasoning item has no id: OpenAI Responses clients require output[].id to be a string") | ||
| } else if !strings.HasPrefix(*reasoning.ID, "rs_") { | ||
| t.Errorf("reasoning item id %q does not use the rs_ prefix used across providers", *reasoning.ID) | ||
| } | ||
| // Bifrost serializes `summary` via the embedded *ResponsesReasoning, so a nil | ||
| // ResponsesReasoning drops the key from the JSON entirely and a nil Summary | ||
| // serializes as null instead of an array. | ||
| if reasoning.ResponsesReasoning == nil { | ||
| t.Errorf("reasoning item has nil ResponsesReasoning: `summary` will be absent from serialized JSON") | ||
| } else if reasoning.ResponsesReasoning.Summary == nil { | ||
| t.Errorf("reasoning item has nil Summary: `summary` serializes as null, not an array") | ||
| } | ||
| } | ||
|
|
||
| t.Run("thought without signature", func(t *testing.T) { | ||
| resp := buildResp([]*Part{ | ||
| {Thought: true, Text: "Thinking about which tool to call."}, | ||
| {FunctionCall: &FunctionCall{Name: "get_weather", Args: []byte(`{"location":"Paris"}`)}}, | ||
| }).ToResponsesBifrostResponsesResponse() | ||
| if resp == nil || len(resp.Output) < 2 { | ||
| t.Fatalf("expected reasoning + function_call output items, got %+v", resp) | ||
| } | ||
|
|
||
| reasoning := resp.Output[0] | ||
| assertValidReasoningItem(t, reasoning) | ||
| if reasoning.ResponsesReasoning != nil && reasoning.ResponsesReasoning.EncryptedContent != nil { | ||
| t.Errorf("signature-less thought must not carry encrypted_content, got %q", *reasoning.ResponsesReasoning.EncryptedContent) | ||
| } | ||
|
|
||
| // The function call following the reasoning item stays consumable. | ||
| fc := resp.Output[1] | ||
| if fc.Type == nil || *fc.Type != schemas.ResponsesMessageTypeFunctionCall { | ||
| t.Fatalf("output[1] is not a function_call item: %+v", fc.Type) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("thought with signature", func(t *testing.T) { | ||
| resp := buildResp([]*Part{ | ||
| {Thought: true, Text: "Thinking.", ThoughtSignature: []byte("opaque-signature-bytes")}, | ||
| }).ToResponsesBifrostResponsesResponse() | ||
| if resp == nil || len(resp.Output) < 1 { | ||
| t.Fatalf("expected a reasoning output item, got %+v", resp) | ||
| } | ||
|
|
||
| reasoning := resp.Output[0] | ||
| assertValidReasoningItem(t, reasoning) | ||
| if reasoning.ResponsesReasoning == nil || reasoning.ResponsesReasoning.EncryptedContent == nil { | ||
| t.Errorf("thought signature must be preserved as encrypted_content") | ||
| } | ||
| }) | ||
|
|
||
| t.Run("standalone thought signature part", func(t *testing.T) { | ||
| resp := buildResp([]*Part{ | ||
| {ThoughtSignature: []byte("opaque-signature-bytes")}, | ||
| }).ToResponsesBifrostResponsesResponse() | ||
| if resp == nil || len(resp.Output) < 1 { | ||
| t.Fatalf("expected a reasoning output item, got %+v", resp) | ||
| } | ||
| assertValidReasoningItem(t, resp.Output[0]) | ||
| }) | ||
| } | ||
|
|
||
| // Reasoning items built by the Gemini converter carry their text as reasoning content | ||
| // blocks (summary stays an empty array for OpenAI-compat clients). Replay back to | ||
| // Gemini must read those blocks — the thinking guide requires thought blocks to be | ||
| // resent unmodified, so text and signature both have to survive the round trip. | ||
| func TestReasoningItemRoundTripToGeminiContents(t *testing.T) { | ||
| roundTrip := func(t *testing.T, parts []*Part) []Content { | ||
| t.Helper() | ||
| resp := (&GenerateContentResponse{ | ||
| ModelVersion: "gemini-2.5-pro", | ||
| Candidates: []*Candidate{ | ||
| { | ||
| FinishReason: FinishReasonStop, | ||
| Content: &Content{Role: "model", Parts: parts}, | ||
| }, | ||
| }, | ||
| }).ToResponsesBifrostResponsesResponse() | ||
| if resp == nil { | ||
| t.Fatal("nil bifrost response") | ||
| } | ||
| contents, _, err := convertResponsesMessagesToGeminiContents(resp.Output, "gemini-2.5-pro", schemas.Gemini) | ||
| if err != nil { | ||
| t.Fatalf("convert back to gemini contents: %v", err) | ||
| } | ||
| return contents | ||
| } | ||
|
|
||
| collectThoughts := func(contents []Content) (texts []string, signatures int) { | ||
| for _, c := range contents { | ||
| for _, p := range c.Parts { | ||
| if p.Thought && p.Text != "" { | ||
| texts = append(texts, p.Text) | ||
| } | ||
| if len(p.ThoughtSignature) > 0 { | ||
| signatures++ | ||
| } | ||
| } | ||
| } | ||
| return texts, signatures | ||
| } | ||
|
|
||
| t.Run("unsigned thought text survives replay", func(t *testing.T) { | ||
| contents := roundTrip(t, []*Part{ | ||
| {Thought: true, Text: "Reasoning about the answer."}, | ||
| {Text: "The answer is 42."}, | ||
| }) | ||
| texts, _ := collectThoughts(contents) | ||
| if len(texts) != 1 || texts[0] != "Reasoning about the answer." { | ||
| t.Errorf("unsigned thought text lost on replay, got thought texts %q", texts) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("signed thought keeps text and signature", func(t *testing.T) { | ||
| contents := roundTrip(t, []*Part{ | ||
| {Thought: true, Text: "Signed reasoning.", ThoughtSignature: []byte("opaque-signature-bytes")}, | ||
| {Text: "Done."}, | ||
| }) | ||
| texts, signatures := collectThoughts(contents) | ||
| if len(texts) != 1 || texts[0] != "Signed reasoning." { | ||
| t.Errorf("signed thought text lost on replay, got thought texts %q", texts) | ||
| } | ||
| if signatures != 1 { | ||
| t.Errorf("expected the thought signature exactly once on replay, got %d", signatures) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| // ToGeminiResponsesResponse is the other Responses -> Gemini reader. Its generic | ||
| // content-block path already emits reasoning content blocks as thought parts, so its | ||
| // reasoning branch must only fall back to the Summary array — an item carrying the | ||
| // same text in both places (content blocks from the Gemini converter, summary from an | ||
| // OpenAI-ingress mirror) must produce exactly one thought part, and a summary-only | ||
| // item must still produce it. | ||
| func TestReasoningItemNoDuplicateThoughtInGeminiResponse(t *testing.T) { | ||
| collectThoughtTexts := func(resp *GenerateContentResponse) []string { | ||
| var texts []string | ||
| if resp == nil { | ||
| return texts | ||
| } | ||
| for _, cand := range resp.Candidates { | ||
| if cand == nil || cand.Content == nil { | ||
| continue | ||
| } | ||
| for _, p := range cand.Content.Parts { | ||
| if p.Thought && p.Text != "" { | ||
| texts = append(texts, p.Text) | ||
| } | ||
| } | ||
| } | ||
| return texts | ||
| } | ||
|
|
||
| reasoningItem := func(withBlock, withSummary bool) schemas.ResponsesMessage { | ||
| txt := "Reasoning here." | ||
| msg := schemas.ResponsesMessage{ | ||
| ID: schemas.Ptr("rs_x"), | ||
| Role: schemas.Ptr(schemas.ResponsesInputMessageRoleAssistant), | ||
| Type: schemas.Ptr(schemas.ResponsesMessageTypeReasoning), | ||
| ResponsesReasoning: &schemas.ResponsesReasoning{Summary: []schemas.ResponsesReasoningSummary{}}, | ||
| } | ||
| if withBlock { | ||
| msg.Content = &schemas.ResponsesMessageContent{ | ||
| ContentBlocks: []schemas.ResponsesMessageContentBlock{ | ||
| {Type: schemas.ResponsesOutputMessageContentTypeReasoning, Text: &txt}, | ||
| }, | ||
| } | ||
| } | ||
| if withSummary { | ||
| msg.ResponsesReasoning.Summary = []schemas.ResponsesReasoningSummary{ | ||
| {Type: schemas.ResponsesReasoningContentBlockTypeSummaryText, Text: txt}, | ||
| } | ||
| } | ||
| return msg | ||
| } | ||
|
|
||
| t.Run("text in both content blocks and summary emits one part", func(t *testing.T) { | ||
| resp := ToGeminiResponsesResponse(&schemas.BifrostResponsesResponse{ | ||
| Model: "gemini-2.5-pro", | ||
| Output: []schemas.ResponsesMessage{reasoningItem(true, true)}, | ||
| }) | ||
| texts := collectThoughtTexts(resp) | ||
| if len(texts) != 1 || texts[0] != "Reasoning here." { | ||
| t.Errorf("expected exactly one thought part, got %q", texts) | ||
| } | ||
| }) | ||
|
|
||
| t.Run("summary-only item still emits its part", func(t *testing.T) { | ||
| resp := ToGeminiResponsesResponse(&schemas.BifrostResponsesResponse{ | ||
| Model: "gemini-2.5-pro", | ||
| Output: []schemas.ResponsesMessage{reasoningItem(false, true)}, | ||
| }) | ||
| texts := collectThoughtTexts(resp) | ||
| if len(texts) != 1 || texts[0] != "Reasoning here." { | ||
| t.Errorf("expected the summary fallback to emit one thought part, got %q", texts) | ||
| } | ||
| }) | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -344,16 +344,49 @@ func (response *GenerateContentResponse) ToResponsesBifrostResponsesResponse() * | |
| return bifrostResp | ||
| } | ||
|
|
||
| // thoughtTextParts renders a reasoning item's summary as Gemini thought parts. | ||
| // thoughtTextParts renders a reasoning item's text as Gemini thought parts. | ||
| // | ||
| // Gemini's thinking guide requires thought blocks to be resent unmodified, so | ||
| // wherever a reasoning item's signature is taken its text has to travel with it. | ||
| func thoughtTextParts(reasoning *schemas.ResponsesReasoning) []*Part { | ||
| if reasoning == nil { | ||
| // Gemini's own outbound converter stores thought text as reasoning content | ||
| // blocks (with summary left an empty array for OpenAI-compat clients), while | ||
| // OpenAI-ingress replay carries it in summary — so content blocks are read | ||
| // first and summary is the fallback. | ||
| func thoughtTextParts(msg *schemas.ResponsesMessage) []*Part { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice complement to the id fix - without this, One gap: the other replay reader was not updated. Suggested fix in currentParts = append(currentParts, thoughtTextParts(&msg)...)That also unblocks the natural follow-up of populating |
||
| if msg == nil { | ||
| return nil | ||
| } | ||
| if parts := reasoningBlockThoughtParts(msg); len(parts) > 0 { | ||
| return parts | ||
| } | ||
| return reasoningSummaryThoughtParts(msg) | ||
| } | ||
|
|
||
| // reasoningBlockThoughtParts renders the reasoning content blocks of a reasoning | ||
| // item as Gemini thought parts. | ||
| func reasoningBlockThoughtParts(msg *schemas.ResponsesMessage) []*Part { | ||
| if msg == nil || msg.Content == nil { | ||
| return nil | ||
| } | ||
| var parts []*Part | ||
| for _, block := range msg.Content.ContentBlocks { | ||
| if block.Type == schemas.ResponsesOutputMessageContentTypeReasoning && | ||
| block.Text != nil && *block.Text != "" { | ||
| parts = append(parts, &Part{Text: *block.Text, Thought: true}) | ||
| } | ||
| } | ||
| return parts | ||
| } | ||
|
|
||
| // reasoningSummaryThoughtParts renders a reasoning item's Summary array as Gemini | ||
| // thought parts. Callers that have already emitted the item's content blocks | ||
| // through another path use this directly instead of thoughtTextParts. | ||
| func reasoningSummaryThoughtParts(msg *schemas.ResponsesMessage) []*Part { | ||
| if msg == nil || msg.ResponsesReasoning == nil { | ||
| return nil | ||
| } | ||
| var parts []*Part | ||
| for _, summaryBlock := range reasoning.Summary { | ||
| for _, summaryBlock := range msg.ResponsesReasoning.Summary { | ||
| if summaryBlock.Text == "" { | ||
| continue | ||
| } | ||
|
|
@@ -561,7 +594,7 @@ func ToGeminiResponsesResponse(bifrostResp *schemas.BifrostResponsesResponse) *G | |
| // because the normal reasoning branch below also | ||
| // emits a signature-only part - that path would | ||
| // send the same signature twice. | ||
| consumedThoughtText = thoughtTextParts(nextMsg.ResponsesReasoning) | ||
| consumedThoughtText = thoughtTextParts(&nextMsg) | ||
| } | ||
| } | ||
| } | ||
|
|
@@ -611,16 +644,13 @@ func ToGeminiResponsesResponse(bifrostResp *schemas.BifrostResponsesResponse) *G | |
| continue | ||
| } | ||
|
|
||
| // Reasoning content is in the Summary array | ||
| if len(msg.ResponsesReasoning.Summary) > 0 { | ||
| for _, summaryBlock := range msg.ResponsesReasoning.Summary { | ||
| if summaryBlock.Text != "" { | ||
| currentParts = append(currentParts, &Part{ | ||
| Text: summaryBlock.Text, | ||
| Thought: true, | ||
| }) | ||
| } | ||
| } | ||
| // The content-block loop above already emitted this item's reasoning | ||
| // content blocks as thought parts, so fall back to the Summary array | ||
| // only when the item carries no thought text in its content blocks | ||
| // (the same content-first rule as thoughtTextParts). Text present in | ||
| // both places must not be emitted twice. | ||
| if len(reasoningBlockThoughtParts(&msg)) == 0 { | ||
| currentParts = append(currentParts, reasoningSummaryThoughtParts(&msg)...) | ||
| } | ||
| if msg.ResponsesReasoning.EncryptedContent != nil { | ||
| decodedSig := thoughtSignatureFromEncryptedContent(msg.ResponsesReasoning.EncryptedContent) | ||
|
|
@@ -3088,6 +3118,7 @@ func reasoningFromThoughtSignature(part *Part) (schemas.ResponsesMessage, bool) | |
| } | ||
| thoughtSig := base64.StdEncoding.EncodeToString(part.ThoughtSignature) | ||
| return schemas.ResponsesMessage{ | ||
| ID: schemas.Ptr("rs_" + schemas.GetRandomString(50)), | ||
| Role: schemas.Ptr(schemas.ResponsesInputMessageRoleAssistant), | ||
| Type: schemas.Ptr(schemas.ResponsesMessageTypeReasoning), | ||
| ResponsesReasoning: &schemas.ResponsesReasoning{ | ||
|
|
@@ -3225,6 +3256,7 @@ func convertGeminiCandidatesToResponsesOutput(candidates []*Candidate) []schemas | |
| if part.Text != "" || len(part.ThoughtSignature) > 0 { | ||
| text := part.Text | ||
| msg := schemas.ResponsesMessage{ | ||
| ID: schemas.Ptr("rs_" + schemas.GetRandomString(50)), | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct format and consistent with Follow-up rather than a change request: |
||
| Role: schemas.Ptr(schemas.ResponsesInputMessageRoleAssistant), | ||
| Content: &schemas.ResponsesMessageContent{ | ||
| ContentBlocks: []schemas.ResponsesMessageContentBlock{ | ||
|
|
@@ -3235,17 +3267,21 @@ func convertGeminiCandidatesToResponsesOutput(candidates []*Candidate) []schemas | |
| }, | ||
| }, | ||
| Type: schemas.Ptr(schemas.ResponsesMessageTypeReasoning), | ||
| // Strict OpenAI Responses clients require both id and a summary | ||
| // array on every reasoning item; set summary unconditionally so | ||
| // signature-less thoughts serialize as `"summary": []` instead of | ||
| // omitting the key. | ||
| ResponsesReasoning: &schemas.ResponsesReasoning{ | ||
| Summary: []schemas.ResponsesReasoningSummary{}, | ||
| }, | ||
| } | ||
| if len(part.ThoughtSignature) > 0 { | ||
| // Stored base64-encoded, which is the form | ||
| // encrypted_content carries on the wire and the form | ||
| // thoughtSignatureFromEncryptedContent decodes on the way | ||
| // back out -- so the round trip is symmetric by construction. | ||
| encoded := base64.StdEncoding.EncodeToString(part.ThoughtSignature) | ||
| msg.ResponsesReasoning = &schemas.ResponsesReasoning{ | ||
| Summary: []schemas.ResponsesReasoningSummary{}, | ||
| EncryptedContent: &encoded, | ||
| } | ||
| msg.ResponsesReasoning.EncryptedContent = &encoded | ||
| msg.Content.ContentBlocks[0].Signature = &encoded | ||
| } | ||
| messages = append(messages, msg) | ||
|
|
@@ -3510,16 +3546,9 @@ func convertGeminiCandidatesToResponsesOutput(candidates []*Candidate) []schemas | |
|
|
||
| case part.ThoughtSignature != nil: | ||
| // Handle thought signature | ||
| thoughtSig := base64.StdEncoding.EncodeToString(part.ThoughtSignature) | ||
| msg := schemas.ResponsesMessage{ | ||
| Role: schemas.Ptr(schemas.ResponsesInputMessageRoleAssistant), | ||
| Type: schemas.Ptr(schemas.ResponsesMessageTypeReasoning), | ||
| ResponsesReasoning: &schemas.ResponsesReasoning{ | ||
| Summary: []schemas.ResponsesReasoningSummary{}, | ||
| EncryptedContent: &thoughtSig, | ||
| }, | ||
| if msg, ok := reasoningFromThoughtSignature(part); ok { | ||
| messages = append(messages, msg) | ||
| } | ||
| messages = append(messages, msg) | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -4144,7 +4173,7 @@ func convertResponsesMessagesToGeminiContents(messages []schemas.ResponsesMessag | |
| // A reasoning message with no text still has nothing to add here, so it | ||
| // keeps being skipped. | ||
| if msg.Type != nil && *msg.Type == schemas.ResponsesMessageTypeReasoning && msg.ResponsesReasoning != nil { | ||
| parts := thoughtTextParts(msg.ResponsesReasoning) | ||
| parts := thoughtTextParts(&msg) | ||
|
|
||
| // The signature is carried by the PRECEDING function call's | ||
| // look-ahead - but only when there is one. A standalone signed | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert the preserved signature value.
Line 87 only checks that
EncryptedContentexists. The test passes if conversion corrupts or replaces the signature. CompareEncryptedContentwith the expected base64 encoding ofopaque-signature-bytes.🤖 Prompt for AI Agents