fix(schemas): add ExtraContent field to ChatStreamResponseChoiceDelta#4569
fix(schemas): add ExtraContent field to ChatStreamResponseChoiceDelta#4569nghodkicisco wants to merge 3 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughA new ChangesStreaming Delta and Message Schema Extensions
Sequence DiagramsequenceDiagram
participant ChatStreamDelta as ChatStreamResponseChoiceDelta
participant CoreDeepCopy as DeepCopyChatMessage
participant DeltaDeepCopy as deepCopyChatStreamResponseChoiceDelta
ChatStreamDelta->>DeltaDeepCopy: original delta with ExtraContent
DeltaDeepCopy->>DeltaDeepCopy: initialize struct fields
DeltaDeepCopy->>DeltaDeepCopy: copy ExtraContent bytes to result
DeltaDeepCopy->>DeltaDeepCopy: return delta without shared refs
ChatStreamDelta->>CoreDeepCopy: message with tool calls and ReasoningDetails
CoreDeepCopy->>CoreDeepCopy: deep copy tool call ExtraContent
CoreDeepCopy->>CoreDeepCopy: deep copy ReasoningDetails slice and fields
CoreDeepCopy->>CoreDeepCopy: return message without shared refs
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 golangci-lint (2.12.2)level=error msg="[linters_context] typechecking error: pattern ./...: directory prefix . does not contain main module or its selected dependencies" Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@nghodkicisco there a couple of deep copy flows we have - can you confirm you have covered all those ? |
Thanks Akshay, reviewing and adding fixes. |
2aaaba4 to
eb6f7c4
Compare
|
@akshaydeo Added the gap fixes. Kindly review. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@plugins/jsonparser/utils.go`:
- Line 337: The ReasoningDetails field assignment in the delta copy operation
performs a shallow copy by directly assigning the slice reference, which is
unsafe because the slice elements contain pointer fields that can be mutated
across different accumulators/consumers. Replace the direct assignment of
ReasoningDetails with a deep copy by creating a new slice and iterating through
the original ReasoningDetails elements, copying each element (including any
nested pointer fields) to the new slice, then assign this new slice to the
copied delta's ReasoningDetails field to prevent mutation leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: c899cc98-cf7b-4d25-9dd4-03930dcd9fb1
📒 Files selected for processing (1)
plugins/jsonparser/utils.go
d1a3c38 to
654de46
Compare
The merge-base changed after approval.
|
Hello @akshaydeo , notnsue where al these commits came from. Can you help me understand what all to keep in the PR, or would you need me to make any changes to get this fix in latest release? Thanks |
|
Rebase your branch with dev - that should fix this 🙇♂️ |
Preserve provider-specific metadata on streaming delta chunks. Some OpenAI-compatible providers (e.g. Google's Gemini via OpenAI endpoint) return extra_content on delta objects to convey thinking markers (extra_content.google.thought: true) and encrypted thought signatures (extra_content.google.thought_signature) needed for multi-turn continuation with extended thinking enabled. Without this field, any proxy/gateway layer that deserializes streaming chunks through Bifrost's typed structs silently drops extra_content, breaking downstream consumers that rely on thought_signature for multi-turn Gemini conversations. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ChatMessage DeepCopyChatMessage was not copying: - ChatAssistantMessageToolCall.ExtraContent (thought_signature) - ChatAssistantMessage.ReasoningDetails (thinking content) This could cause shared mutation between plugin accumulators when processing Gemini responses with extended thinking enabled. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… copy The jsonparser plugin's deepCopyChatStreamResponseChoiceDelta was missing ExtraContent (provider metadata like google.thought_signature) and ReasoningDetails/Audio fields. Add them to prevent shared mutation between plugin accumulators when processing Gemini thinking responses. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
4df6698 to
89d577e
Compare
Thanks @akshaydeo , done, kindly help review the same and lemme know if anything else needed before merging. Thanks. |
PR Title
fix(schemas): add ExtraContent to ChatStreamResponseChoiceDelta
Problem Statement
When using Bifrost as a proxy layer between clients and OpenAI-compatible LLM providers (such as Google's Gemini via their OpenAI-compatible endpoint), streaming delta chunks that carry provider-specific metadata in an
extra_contentfield are silently dropped during deserialization.Google's Gemini models with extended thinking enabled return streaming chunks in this format:
The
extra_contentfield serves two purposes:google.thought: true— marks chunks as internal reasoning/thinking (distinct from the final answer)google.thought_signature— an encrypted opaque token that MUST be replayed on subsequent turns for Gemini to accept the conversation historyWithout preserving
extra_contenton the delta, any proxy that routes through Bifrost's typed deserialization path:thought_signature→ Gemini rejects subsequent turns with HTTP 400: "function call in content block N is missing a thought_signature"This effectively breaks multi-turn agentic conversations with Gemini when extended thinking is enabled and traffic flows through a Bifrost-based gateway.
What This Solves
Adding
ExtraContent json.RawMessagetoChatStreamResponseChoiceDeltaensures that:thought_signatureis preserved end-to-end, so Gemini accepts replayed conversation history on subsequent turnsChanges
ExtraContent json.RawMessagefield withjson:"extra_content,omitempty"toChatStreamResponseChoiceDeltaUnmarshalJSONneeded — the existingtype Aliaspattern automatically captures all struct fields including the new oneextra_contentNote
ChatAssistantMessageToolCallalready has anExtraContentfield (added previously) which handlesextra_contenton tool call objects. This PR completes the coverage by adding it to the delta itself, where thinking markers and the final-answer signature live.