fix: preserve Gemini thought and content in same chunk - #5094
Conversation
Agent-Model: gpt-5 Agent-Family: openai Agent-Session: 019e556d-e697-7580-9ae0-e03202597043 Agent-Step: 0.0.1
WalkthroughThis PR improves reasoning content handling across LLM provider conversions. Gemini responses now separate "thought" parts into dedicated ChangesReasoning Content Separation
Sequence Diagram(s)sequenceDiagram
participant GeminiResp as Gemini Response.Parts
participant ReasoningBld as Reasoning Builder
participant ContentBld as Content Builder
participant OpenAIChoice as OpenAI Choice
GeminiResp->>ReasoningBld: Thought part
GeminiResp->>ContentBld: Text/Code/Image part
ReasoningBld->>OpenAIChoice: Set ReasoningContent
ContentBld->>OpenAIChoice: Set Message.Content
sequenceDiagram
participant OpenAIDelta as OpenAI Delta (reasoning/text)
participant EmitHelper as emitThinkingDelta/emitTextDelta
participant SSEEmit as Emit SSE Event
participant StateTrack as Update LastMessagesType
OpenAIDelta->>EmitHelper: Delta received
alt Message type changed
EmitHelper->>SSEEmit: Emit content_block_start
end
EmitHelper->>SSEEmit: Emit content_block_delta
EmitHelper->>StateTrack: Cache message type
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
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 `@relay/channel/gemini/relay-gemini.go`:
- Around line 1095-1100: The helper writeSeparated currently adds a newline and
increments the appended counter even for empty strings; update the function(s)
(the writeSeparated closure at/around writeSeparated and the second identical
closure later) to first return early when s == "" (or len(s) == 0) so no newline
is written and appended is not incremented; ensure the logic still writes a
preceding '\n' only when *appended > 0 and only increments *appended after
actually writing a non-empty string.
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: aedd4a2b-92db-4cb7-88f1-fac582d72767
📒 Files selected for processing (4)
relay/channel/gemini/relay-gemini.gorelay/channel/gemini/relay_gemini_reasoning_test.goservice/convert.goservice/convert_stream_claude_test.go
| writeSeparated := func(builder *strings.Builder, appended *int, s string) { | ||
| if *appended > 0 { | ||
| builder.WriteByte('\n') | ||
| } | ||
| appended++ | ||
| builder.WriteString(s) | ||
| *appended += 1 |
There was a problem hiding this comment.
Guard writeSeparated against empty strings.
Line 1095 and Line 1209 currently increment the append counter even when s == "". With empty text parts (e.g., Line 1129/1272 path), this can create phantom separators/newlines and formatting drift.
💡 Proposed fix
- writeSeparated := func(builder *strings.Builder, appended *int, s string) {
+ writeSeparated := func(builder *strings.Builder, appended *int, s string) {
+ if s == "" {
+ return
+ }
if *appended > 0 {
builder.WriteByte('\n')
}
builder.WriteString(s)
*appended += 1
}
@@
- writeSeparated := func(builder *strings.Builder, appended *int, s string) {
+ writeSeparated := func(builder *strings.Builder, appended *int, s string) {
+ if s == "" {
+ return
+ }
if *appended > 0 {
builder.WriteByte('\n')
}
builder.WriteString(s)
*appended += 1
}Also applies to: 1209-1214
🤖 Prompt for 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.
In `@relay/channel/gemini/relay-gemini.go` around lines 1095 - 1100, The helper
writeSeparated currently adds a newline and increments the appended counter even
for empty strings; update the function(s) (the writeSeparated closure at/around
writeSeparated and the second identical closure later) to first return early
when s == "" (or len(s) == 0) so no newline is written and appended is not
incremented; ensure the logic still writes a preceding '\n' only when *appended
> 0 and only increments *appended after actually writing a non-empty string.
51fdfc5 to
2b6f1df
Compare
Summary
thoughtparts from normal text parts when buildingreasoning_contentthinking_deltaandtext_deltato both emit from the same OpenAI delta during Claude SSE conversionRoot cause
When a Gemini chunk contained both
part.Thought=trueand normal answer text,new-apicurrently mixed the two behaviors:relay/channel/gemini/relay-gemini.gotreated the whole chunk as reasoning once any thought part appeared, so final answer text could be folded intoreasoning_content.service/convert.goused mutually exclusive reasoning/content emission for Claude SSE, so a chunk with both fields could drop the finaltext_delta.This change keeps reasoning and answer text separate all the way through the relay/conversion pipeline.
Tests
go test ./relay/channel/gemini ./serviceSummary by CodeRabbit
Refactor
Tests