Skip empty Gemini thought parts instead of emitting empty reasoning#543
Open
PratikDhanave wants to merge 3 commits into
Open
Skip empty Gemini thought parts instead of emitting empty reasoning#543PratikDhanave wants to merge 3 commits into
PratikDhanave wants to merge 3 commits into
Conversation
buildResponsePart appended a TextReasoningContent for every part with
Thought=true, unconditionally — unlike the text branch, which guards on
part.Text != "". A thought part carrying neither thinking text nor a
ThoughtSignature therefore produced an empty, information-free
TextReasoningContent{Text:"", ProtectedData:""} in the response.
Guard the thought branch on part.Text != "" || len(part.ThoughtSignature) > 0,
mirroring the text guard. A signature-only thought is still emitted so its
signature round-trips via ProtectedData in multi-turn requests; only the
fully-empty thought part is skipped.
There was a problem hiding this comment.
Pull request overview
This PR updates the Gemini provider’s response-part translation so that “thought” parts that carry no useful information (no thinking text and no thought signature) are omitted rather than being converted into an empty TextReasoningContent, aligning behavior with the existing “skip empty text parts” logic.
Changes:
- Skip emitting
TextReasoningContentforthought=trueparts when bothTextandThoughtSignatureare empty. - Add a black-box test covering an empty thought-only part in the response.
- Minor formatting change to a test call site for
RunText(...).
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| provider/geminiprovider/agent.go | Adds a guard to avoid appending empty reasoning content for fully-empty thought parts while still preserving signature-only thoughts. |
| provider/geminiprovider/agent_test.go | Adds a regression test for omitting empty thought parts and includes a minor formatting adjustment in an existing test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+796
to
+803
| for _, msg := range result.Messages { | ||
| for _, c := range msg.Contents { | ||
| if rc, ok := c.(*message.TextReasoningContent); ok { | ||
| t.Errorf("expected no reasoning content for an empty thought part, got Text=%q ProtectedData=%q", rc.Text, rc.ProtectedData) | ||
| } | ||
| } | ||
| } | ||
| } |
Address review feedback: the test asserted only the absence of reasoning content, so it would also pass if the provider dropped all response parts. Also assert the normal text part is still emitted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
In
buildResponsePart(provider/geminiprovider/agent.go), a response part withThought=trueis appended asTextReasoningContentunconditionally, while the sibling text branch guards onpart.Text != "":So a thought part carrying neither thinking text nor a
ThoughtSignatureproduces an empty, information-freeTextReasoningContent{Text:"", ProtectedData:""}in the response message.Fix
Guard the thought branch on
part.Text != "" || len(part.ThoughtSignature) > 0, mirroring the text guard. A signature-only thought (empty text, non-empty signature) is still emitted so the signature round-trips viaProtectedDatain multi-turn requests — only the fully-empty thought part is skipped.Test
TestResponseWithEmptyThoughtPartOmitted(black-box, inagent_test.go) returns a candidate with an empty{"thought": true}part alongside a normal text part, and asserts noTextReasoningContentis produced. Fails onmain(empty reasoning content emitted), passes with the fix. The existing thinking-content and thought-signature tests are unchanged.