gemini reasoning fixes - #5984
Merged
Merged
Conversation
Contributor
📝 WalkthroughSummary by CodeRabbit
WalkthroughGemini Responses conversion now preserves reasoning summaries and signatures during replay. The provider harness adds direct-versus-Bifrost cache parity checks, deferred feature routing, and centralized collection identity matching. ChangesGemini reasoning replay
Cache parity harness
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant DirectProvider
participant BifrostGateway
participant NewmanAssertions
DirectProvider->>NewmanAssertions: Record direct cache-hit rates
BifrostGateway->>NewmanAssertions: Record Bifrost cache-hit rates
NewmanAssertions->>NewmanAssertions: Compare Bifrost with direct baseline
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
Contributor
Author
This stack of pull requests is managed by Graphite. Learn more about stacking. |
This was referenced Aug 9, 2026
Merged
akshaydeo
marked this pull request as ready for review
August 9, 2026 01:14
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@core/providers/gemini/reasoningreplay_test.go`:
- Around line 150-171: Update the assertions in the candidate-part inspection
around sawSignature and sawThoughtText to count matching thought text and
signature-bearing function-call parts instead of using booleans. Require exactly
one occurrence of each, while preserving the existing raw signature and summary
value checks.
In `@core/providers/gemini/responses.go`:
- Around line 3513-3519: The standalone reasoning handling in thoughtTextParts
must append encrypted content’s decoded ThoughtSignature exactly once, while
avoiding duplication when the preceding function call already consumed it;
update the logic around ResponsesReasoning in responses.go accordingly. In
core/providers/gemini/reasoningreplay_test.go lines 177-207, add or adjust
assertions to verify the original signature appears exactly once.
In `@tests/e2e/api/collections/provider-harness.json`:
- Around line 48797-48809: Update the replay-capture logic around
geminiReasoningReplayInput5982 to require a reasoning item with non-empty
encrypted_content before storing the replay. Track whether input contains such
an item, and skip assigning or persisting the replay when none exists, while
preserving the existing summary handling for valid reasoning items.
🪄 Autofix
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: 6333aed6-1259-41d9-9aef-d1388c34226b
📒 Files selected for processing (3)
core/providers/gemini/reasoningreplay_test.gocore/providers/gemini/responses.gotests/e2e/api/collections/provider-harness.json
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 02:14
0e83b8b to
528ff4b
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 02:14
f484dd3 to
8ec8fab
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 08:22
8ec8fab to
3ae9a28
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
tests/e2e/api/runners/lib/direct-cache-parity.mjs (2)
171-190: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueWrite rounds swallow HTTP errors without any signal.
Line 174 returns on any response code at or above 400 and records nothing. No
pm.testruns, so a failed write round produces a passing item. The read round still asserts "round 2 responded", so a systematic auth or model failure is caught there. A single failed write round, however, only shows up as a lower hit rate, which the best-of-rounds logic then reports as inconclusive. Add a console line so the cause is visible intmp/newman-cli-cache-parity.log.♻️ Proposed change
-if (pm.response.code >= 400) { return; } +if (pm.response.code >= 400) { + console.log('[direct-cache-parity] ${cell.id} ${leg} round ${round} write failed (' + + pm.response.code + '): ' + pm.response.text()); + return; +}🤖 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 `@tests/e2e/api/runners/lib/direct-cache-parity.mjs` around lines 171 - 190, Add a console error/log message before the early return in roundOneScript when pm.response.code is at least 400, including the cell, leg, round, and HTTP status so failed write rounds are visible in tmp/newman-cli-cache-parity.log; preserve the existing return behavior and successful-round handling.
128-140: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe Gemini shape ignores
cell.bifrostModel.
bifrostUrlbuilds the path fromcell.directModel, andbodynever uses itsmodelargument, because Gemini carries the model in the URL. Line 314 still computesmodelfromcell.bifrostModel || cell.directModel, so that value has no effect for Gemini cells. No Gemini cell setsbifrostModeltoday, so the behavior is correct now. Add the fallback to keep the contract uniform if a Gemini cell later needs an explicit gateway model id.♻️ Proposed change
- bifrostUrl: (cell) => `{{baseUrl}}/genai/v1beta/models/${cell.directModel}:generateContent`, + bifrostUrl: (cell) => + `{{baseUrl}}/genai/v1beta/models/${cell.bifrostModel || cell.directModel}:generateContent`,🤖 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 `@tests/e2e/api/runners/lib/direct-cache-parity.mjs` around lines 128 - 140, Update the Gemini shape’s bifrostUrl to build the model path from the provided model argument, preserving the fallback computed as cell.bifrostModel || cell.directModel at the caller. Keep directUrl based on cell.directModel and leave the request body unchanged.
🤖 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 `@Makefile`:
- Around line 2153-2155: Update the deferred-feature loop around DEFERRED_KEYS
so it sets CACHE_PASS and SKIP_MAIN only when FEATURE=cache-parity is unscoped,
preserving the existing main-pass filtering for RERUN_FAILED or FOLDER inputs;
alternatively ensure those scopes are forwarded to the cache pass. Revise the
help text near the interactive-menu guidance to state that FEATURE=cache-parity
is also protected, while other spellings such as FEATURE=cache still require
PARALLEL=0.
In `@tests/e2e/api/runners/lib/direct-cache-parity.mjs`:
- Around line 193-226: Update tests/e2e/api/runners/lib/direct-cache-parity.mjs
lines 193-226 by passing the round number into roundTwoScript and using it in
the assertion and console labels, so Gemini reports its actual final round
(round 6) instead of round 2. Also update the cost comment at lines 150-163 to
state that Gemini cells require 12 live calls rather than four.
- Around line 180-187: Update the round-1 reset logic in the generated script
around roundTwoScript so it clears both seriesVarFor(cell.id, leg) and
varFor(cell.id, leg), including when the write round returns an error. Perform
both unsets before the pm.response.code >= 400 guard, while preserving the
existing round-2 series accumulation behavior.
In `@tests/e2e/api/runners/lib/haystack.mjs`:
- Around line 28-33: Update identityOf to preserve description.content when
item.description is an object with a string content field, while retaining
existing string-description handling and folder behavior. Add a regression test
covering an object-form description such as { content, type } and verify its
content appears in the generated identity.
In `@tests/e2e/api/runners/lib/haystack.test.mjs`:
- Around line 1-75: Move the direct Node.js tests around buildHaystack,
including the test helper and provider-routing cases, out of tests/e2e/** into
the repository’s non-E2E unit-test location. Preserve their assertions and
direct execution style there; do not convert them to E2E fixture or
data-testid-based tests.
---
Nitpick comments:
In `@tests/e2e/api/runners/lib/direct-cache-parity.mjs`:
- Around line 171-190: Add a console error/log message before the early return
in roundOneScript when pm.response.code is at least 400, including the cell,
leg, round, and HTTP status so failed write rounds are visible in
tmp/newman-cli-cache-parity.log; preserve the existing return behavior and
successful-round handling.
- Around line 128-140: Update the Gemini shape’s bifrostUrl to build the model
path from the provided model argument, preserving the fallback computed as
cell.bifrostModel || cell.directModel at the caller. Keep directUrl based on
cell.directModel and leave the request body unchanged.
🪄 Autofix
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: fe736cc1-1d9e-4c28-9d3f-1b4076aa23a3
📒 Files selected for processing (6)
Makefiletests/e2e/api/runners/augment-provider-harness.mjstests/e2e/api/runners/filter-collection.mjstests/e2e/api/runners/lib/direct-cache-parity.mjstests/e2e/api/runners/lib/haystack.mjstests/e2e/api/runners/lib/haystack.test.mjs
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 08:38
3ae9a28 to
fe11728
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 08:38
528ff4b to
4279ccb
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 15:14
fe11728 to
9ab7e63
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 15:14
4279ccb to
41db2b1
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 15:19
9ab7e63 to
11b1779
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
Makefile (3)
1867-1873: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep CI output quiet during
install-newman.When a package is missing,
npm installstill writes stdout even when CI mode is enabled. This violates the documented status-table-only output. The recipe also checks only$$CI; use both$(CI)and$$CIwhenCI=1is passed as a make variable.🤖 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 `@Makefile` around lines 1867 - 1873, Update the install-newman recipe so npm install stdout is suppressed when CI is enabled, while preserving stderr and failure propagation. Use a CI condition that recognizes both the Make variable $(CI) and the shell environment variable $$CI, and apply it to both Newman installation commands and the readiness message.
1775-1799: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winPreserve failure diagnostics in
CLI_HARNESS_FILTER.The
/unsupported for /and/not configured in bifrost/patterns match anywhere in a line. They can remove a realt.Fatalor provider error. Match the exact harness skip strings or anchor the patterns to the harness prefix.🤖 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 `@Makefile` around lines 1775 - 1799, Update the awk rules in CLI_HARNESS_FILTER so skip-message filtering matches only the exact harness-generated skip strings or lines with their known harness prefix, rather than any line containing “unsupported for ” or “not configured in bifrost”. Preserve filtering of legitimate skip messages while leaving t.Fatal bodies and provider errors untouched.
1862-1865: 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick winKeep
HARNESS_PROVIDERSas the single provider source.
HARNESS_PROVIDERScontains eight providers, but the help at Line 1902 lists only six. Parallel mode also hard-codes another list at Line 2192. UseHARNESS_PROVIDERSforPROVIDERSand keep the help text aligned.♻️ Proposed fix
- PROVIDERS="openai anthropic bedrock gemini vertex azure passthrough openrouter"; \ + PROVIDERS="$(HARNESS_PROVIDERS)"; \🤖 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 `@Makefile` around lines 1862 - 1865, Use HARNESS_PROVIDERS as the single source for the PROVIDERS value, removing any separate provider list or hard-coded assignment in the parallel-mode configuration. Update the help text near the harness options to enumerate all providers from HARNESS_PROVIDERS, including passthrough and openrouter, so documentation and execution remain aligned.
🤖 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 `@Makefile`:
- Around line 2155-2157: Update the deferred-key check in the Makefile loop over
DEFERRED_KEYS to normalize FEATURE and each deferred key for case-insensitive
matching, then tokenize comma-separated FEATURE predicates and detect deferred
keys within the complete predicate. Preserve the existing RERUN_FAILED guard,
and pass the full normalized FEATURE predicate to the deferred filter so
additional conditions such as “foo” are not discarded.
- Around line 2353-2354: Update the cache parity filter command near the
provider option so its failure branch exits nonzero instead of only logging and
continuing. Ensure a failed deferred cache collection prevents the cache-only
path from succeeding and preserves the existing error message.
In `@tests/e2e/api/runners/lib/direct-cache-parity.mjs`:
- Around line 286-290: Update the inconclusive branch in the direct-cache parity
runner to retain single-run flake protection while tracking repeated one-sided
results across runs; once the aggregate threshold is reached with direct still
engaging and Bifrost never engaging, fail via pm.test instead of returning
successfully. Add a generated-item unit test covering this persistent-failure
path.
- Line 202: Update roundOneScript so failed responses (status 400 or higher)
execute the same response assertion as roundTwoScript before returning, ensuring
each failed cache-flow request fails its test. Preserve the existing variable
reset before the response guard.
---
Outside diff comments:
In `@Makefile`:
- Around line 1867-1873: Update the install-newman recipe so npm install stdout
is suppressed when CI is enabled, while preserving stderr and failure
propagation. Use a CI condition that recognizes both the Make variable $(CI) and
the shell environment variable $$CI, and apply it to both Newman installation
commands and the readiness message.
- Around line 1775-1799: Update the awk rules in CLI_HARNESS_FILTER so
skip-message filtering matches only the exact harness-generated skip strings or
lines with their known harness prefix, rather than any line containing
“unsupported for ” or “not configured in bifrost”. Preserve filtering of
legitimate skip messages while leaving t.Fatal bodies and provider errors
untouched.
- Around line 1862-1865: Use HARNESS_PROVIDERS as the single source for the
PROVIDERS value, removing any separate provider list or hard-coded assignment in
the parallel-mode configuration. Update the help text near the harness options
to enumerate all providers from HARNESS_PROVIDERS, including passthrough and
openrouter, so documentation and execution remain aligned.
🪄 Autofix
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: dff0f0e4-e339-4578-946e-690d0d3c3520
📒 Files selected for processing (6)
Makefiletests/e2e/api/runners/lib/direct-cache-parity.mjstests/e2e/api/runners/lib/direct-cache-parity.test.mjstests/e2e/api/runners/lib/harness-deferral.test.mjstests/e2e/api/runners/lib/haystack.mjstests/e2e/api/runners/lib/haystack.test.mjs
Contributor
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 `@tests/e2e/api/runners/lib/haystack.test.mjs`:
- Around line 88-91: Strengthen the test “a non-string description content is
still ignored” by asserting that object-valued description content does not
appear in the generated haystack. Compare each result with the corresponding
no-description output or assert the nested sentinel is absent, while preserving
coverage for both non-string description shapes.
🪄 Autofix
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: b8f20d5e-dc9d-49e3-9fb8-eb463c9202f4
📒 Files selected for processing (11)
Makefilecore/providers/gemini/reasoningreplay_test.gocore/providers/gemini/responses.gotests/e2e/api/collections/provider-harness.jsontests/e2e/api/runners/augment-provider-harness.mjstests/e2e/api/runners/filter-collection.mjstests/e2e/api/runners/lib/direct-cache-parity.mjstests/e2e/api/runners/lib/direct-cache-parity.test.mjstests/e2e/api/runners/lib/harness-deferral.test.mjstests/e2e/api/runners/lib/haystack.mjstests/e2e/api/runners/lib/haystack.test.mjs
🚧 Files skipped from review as they are similar to previous changes (9)
- tests/e2e/api/collections/provider-harness.json
- tests/e2e/api/runners/lib/direct-cache-parity.test.mjs
- tests/e2e/api/runners/augment-provider-harness.mjs
- Makefile
- core/providers/gemini/responses.go
- tests/e2e/api/runners/filter-collection.mjs
- tests/e2e/api/runners/lib/direct-cache-parity.mjs
- tests/e2e/api/runners/lib/haystack.mjs
- core/providers/gemini/reasoningreplay_test.go
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 15:31
11b1779 to
ed89655
Compare
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
tests/e2e/api/runners/lib/direct-cache-parity.test.mjs (1)
13-13: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winAdd a regression check for direct-before-Bifrost ordering.
ITEMSis generated from an ordering that the parity flow requires. No test compares the positions of the direct and Bifrost items for the same cell. If Bifrost runs first, its read script has no direct baseline and skips the parity assertion. The unit suite can remain green while parity coverage becomes ineffective.Add an assertion that each cell's direct round 1 item appears before its Bifrost round 1 item. The companion generator documents this runtime requirement. (raw.githubusercontent.com)
🤖 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 `@tests/e2e/api/runners/lib/direct-cache-parity.test.mjs` at line 13, Add a regression assertion in the direct-cache parity test that, for every cell, the direct round 1 item occurs before the corresponding Bifrost round 1 item in ITEMS. Use the companion generator’s cell and item identifiers to match each pair, ensuring the required ordering is validated before the parity flow runs.
🤖 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 `@tests/e2e/api/runners/lib/direct-cache-parity.test.mjs`:
- Around line 29-48: Strengthen the reset tests for the generated script around
itemNamed("gemini/gemini-2.5-flash direct round 1") by matching the exact
operation that clears dcp_gemini_2_5_flash_direct_hit, rather than accepting
arbitrary references or comments, and require both reset and error-guard indexes
to be non-negative before comparing their order. Tighten the error-path
assertion to verify the failure expectation and that the round-1 assertion is
located inside the pm.response.code >= 400 guard, not merely that a pm.test and
round label occur afterward.
---
Nitpick comments:
In `@tests/e2e/api/runners/lib/direct-cache-parity.test.mjs`:
- Line 13: Add a regression assertion in the direct-cache parity test that, for
every cell, the direct round 1 item occurs before the corresponding Bifrost
round 1 item in ITEMS. Use the companion generator’s cell and item identifiers
to match each pair, ensuring the required ordering is validated before the
parity flow runs.
🪄 Autofix
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: 0b55f2b1-3b01-45fc-ad52-8cdae824d06d
📒 Files selected for processing (4)
Makefiletests/e2e/api/runners/lib/direct-cache-parity.mjstests/e2e/api/runners/lib/direct-cache-parity.test.mjstests/e2e/api/runners/lib/harness-deferral.test.mjs
🚧 Files skipped from review as they are similar to previous changes (2)
- tests/e2e/api/runners/lib/direct-cache-parity.mjs
- Makefile
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 15:45
ed89655 to
7ae71d7
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
2 times, most recently
from
August 9, 2026 15:49
e004065 to
d35b3e8
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 15:49
7ae71d7 to
1782cdb
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 16:01
d35b3e8 to
eee6e49
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 16:01
1782cdb to
0c12959
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 16:09
eee6e49 to
ce1ee47
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 16:09
0c12959 to
5f0a4aa
Compare
akshaydeo
force-pushed
the
08-08-fixes_cohere_issue
branch
from
August 9, 2026 16:15
ce1ee47 to
da05279
Compare
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 16:15
5f0a4aa to
23df3f3
Compare
Contributor
Author
Merge activity
|
akshaydeo
changed the base branch from
08-08-fixes_cohere_issue
to
graphite-base/5984
August 9, 2026 16:21
akshaydeo
dismissed
coderabbitai[bot]’s stale review
August 9, 2026 16:23
The base branch was changed.
Contributor
|
Preview deployment for your docs. Learn more about Mintlify Previews.
💡 Tip: Enable Workflows to automatically generate PRs for you. |
akshaydeo
force-pushed
the
08-08-gemini_reasoning_fixes
branch
from
August 9, 2026 16:24
23df3f3 to
e80f5d5
Compare
akshaydeo
added a commit
that referenced
this pull request
Aug 10, 2026
## Summary Fixes a series of bugs in reasoning/thinking block handling across the Anthropic, Bedrock, Cohere, and Gemini provider converters that caused replayed reasoning to be dropped, rejected, or corrupted during multi-turn conversations. Bumps the version to 1.7.8. ## Changes - Anthropic, Bedrock, and Cohere converters now fall through to `ResponsesReasoning` instead of being shadowed by an `else if`, preventing replayed reasoning from being silently dropped when `ContentBlocks` is non-nil but empty. - Bedrock no longer sends reasoning blocks with an absent `text` key — a nil pointer with `omitempty` caused Converse to reject the turn with "Member must not be null" on both the Responses and chat conversion paths. - Bedrock now attaches the replayed signature to the first reasoning summary block via `reasoningSignatureForBedrock`, rather than discarding it. - Bedrock invoke thinking blocks now always include the `thinking` key even when text is empty, preventing signature-only replay blocks from being dropped by Bifrost's decoder. - Cohere now emits encrypted reasoning alongside the summary rather than replacing it, and the `[ENCRYPTED_REASONING: ...]` marker is parsed back into `EncryptedContent` on ingress so it no longer surfaces as visible reasoning text to clients. - Gemini streaming path now base64-decodes `encrypted_content` before assigning it to `thoughtSignature`, fixing a double-encoding bug that caused Gemini to reject the signature. - Gemini now carries `thoughtSignature` from thought parts into `encrypted_content` and the reasoning block signature, and emits signature-only thought parts so clients can replay them. - Standalone Gemini reasoning messages are no longer skipped when converting Responses history to Gemini contents — thought text is resent as required, and the signature is emitted when no preceding function call consumed it. - A consumed reasoning item's thought text is now carried alongside the signature taken by a preceding Gemini function call, ensuring the thought block reaches Gemini unmodified. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` Validate multi-turn conversations with reasoning/thinking blocks against Anthropic, Bedrock, Cohere, and Gemini providers. Confirm that: - Replayed reasoning blocks are preserved across turns. - Bedrock does not return "Member must not be null" errors on reasoning turns. - Cohere encrypted reasoning does not appear as visible text to clients. - Gemini signatures are correctly decoded and accepted on streaming responses. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes #5982, #5984 ## Security considerations None beyond ensuring that encrypted reasoning content (`encrypted_content`) is handled correctly and not exposed to clients as plaintext. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
atharvamhaske
pushed a commit
to atharvamhaske/bifrost
that referenced
this pull request
Aug 13, 2026
## Summary Gemini's thinking guide requires that thought blocks be resent to the model exactly as received — neither removed nor modified. Two converters violated this requirement in different ways: the egress path harvested a reasoning item's signature when it followed a function call but discarded the item's summary text, sending a signature-only block back to Gemini. The ingress path skipped standalone reasoning messages entirely, so their thought text never reached Gemini at all. ## Changes - Introduced `thoughtTextParts`, a helper that renders a reasoning item's summary blocks as Gemini thought parts, used by both converters to ensure text travels alongside any harvested signature. - In the egress converter (`ToGeminiResponsesResponse`), when a reasoning item is consumed for its signature, its thought text is now collected and appended immediately after the function call part rather than being silently dropped. - In the ingress converter (`convertResponsesMessagesToGeminiContents`), standalone reasoning messages are no longer skipped outright. Their thought text is emitted as a model-role content block; the signature is still picked up by the look-ahead on the preceding function call, so it is not duplicated. - Added `TestEgressKeepsThoughtTextAlongsideSignature` to verify the egress fix: a reasoning item with both text and a signature that follows a function call must produce both a signed function call part and a thought text part. - Added `TestIngressSendsThoughtTextForStandaloneReasoning` to verify the ingress fix: a standalone reasoning message must produce a thought part in the converted contents. - Extended the e2e Postman collection to inject summary text into reasoning items before replay and assert that Gemini accepts the constructed thought parts without returning `INVALID_ARGUMENT`. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go test ./core/providers/gemini/... ``` The two new unit tests (`TestEgressKeepsThoughtTextAlongsideSignature` and `TestIngressSendsThoughtTextForStandaloneReasoning`) directly cover the fixed paths. The e2e Postman collection can be run against a live Gemini endpoint to confirm the API accepts replayed thought text. ## Breaking changes - [ ] Yes - [x] No ## Related issues ## Security considerations No auth, secrets, PII, or sandboxing implications. The change only affects how thought block content is serialised when constructing requests to the Gemini API. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [x] I added/updated tests where appropriate - [ ] I updated documentation where needed - [x] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
atharvamhaske
pushed a commit
to atharvamhaske/bifrost
that referenced
this pull request
Aug 13, 2026
## Summary Fixes a series of bugs in reasoning/thinking block handling across the Anthropic, Bedrock, Cohere, and Gemini provider converters that caused replayed reasoning to be dropped, rejected, or corrupted during multi-turn conversations. Bumps the version to 1.7.8. ## Changes - Anthropic, Bedrock, and Cohere converters now fall through to `ResponsesReasoning` instead of being shadowed by an `else if`, preventing replayed reasoning from being silently dropped when `ContentBlocks` is non-nil but empty. - Bedrock no longer sends reasoning blocks with an absent `text` key — a nil pointer with `omitempty` caused Converse to reject the turn with "Member must not be null" on both the Responses and chat conversion paths. - Bedrock now attaches the replayed signature to the first reasoning summary block via `reasoningSignatureForBedrock`, rather than discarding it. - Bedrock invoke thinking blocks now always include the `thinking` key even when text is empty, preventing signature-only replay blocks from being dropped by Bifrost's decoder. - Cohere now emits encrypted reasoning alongside the summary rather than replacing it, and the `[ENCRYPTED_REASONING: ...]` marker is parsed back into `EncryptedContent` on ingress so it no longer surfaces as visible reasoning text to clients. - Gemini streaming path now base64-decodes `encrypted_content` before assigning it to `thoughtSignature`, fixing a double-encoding bug that caused Gemini to reject the signature. - Gemini now carries `thoughtSignature` from thought parts into `encrypted_content` and the reasoning block signature, and emits signature-only thought parts so clients can replay them. - Standalone Gemini reasoning messages are no longer skipped when converting Responses history to Gemini contents — thought text is resent as required, and the signature is emitted when no preceding function call consumed it. - A consumed reasoning item's thought text is now carried alongside the signature taken by a preceding Gemini function call, ensuring the thought block reaches Gemini unmodified. ## Type of change - [x] Bug fix - [ ] Feature - [ ] Refactor - [ ] Documentation - [ ] Chore/CI ## Affected areas - [x] Core (Go) - [ ] Transports (HTTP) - [x] Providers/Integrations - [ ] Plugins - [ ] UI (React) - [ ] Docs ## How to test ```sh go version go test ./... ``` Validate multi-turn conversations with reasoning/thinking blocks against Anthropic, Bedrock, Cohere, and Gemini providers. Confirm that: - Replayed reasoning blocks are preserved across turns. - Bedrock does not return "Member must not be null" errors on reasoning turns. - Cohere encrypted reasoning does not appear as visible text to clients. - Gemini signatures are correctly decoded and accepted on streaming responses. ## Breaking changes - [ ] Yes - [x] No ## Related issues Closes maximhq#5982, maximhq#5984 ## Security considerations None beyond ensuring that encrypted reasoning content (`encrypted_content`) is handled correctly and not exposed to clients as plaintext. ## Checklist - [ ] I read `docs/contributing/README.md` and followed the guidelines - [ ] I added/updated tests where appropriate - [ ] I updated documentation where needed - [ ] I verified builds succeed (Go and UI) - [ ] I verified the CI pipeline passes locally if applicable
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
1 participant
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.
Summary
Gemini's thinking guide requires that thought blocks be resent to the model exactly as received — neither removed nor modified. Two converters violated this requirement in different ways: the egress path harvested a reasoning item's signature when it followed a function call but discarded the item's summary text, sending a signature-only block back to Gemini. The ingress path skipped standalone reasoning messages entirely, so their thought text never reached Gemini at all.
Changes
thoughtTextParts, a helper that renders a reasoning item's summary blocks as Gemini thought parts, used by both converters to ensure text travels alongside any harvested signature.ToGeminiResponsesResponse), when a reasoning item is consumed for its signature, its thought text is now collected and appended immediately after the function call part rather than being silently dropped.convertResponsesMessagesToGeminiContents), standalone reasoning messages are no longer skipped outright. Their thought text is emitted as a model-role content block; the signature is still picked up by the look-ahead on the preceding function call, so it is not duplicated.TestEgressKeepsThoughtTextAlongsideSignatureto verify the egress fix: a reasoning item with both text and a signature that follows a function call must produce both a signed function call part and a thought text part.TestIngressSendsThoughtTextForStandaloneReasoningto verify the ingress fix: a standalone reasoning message must produce a thought part in the converted contents.INVALID_ARGUMENT.Type of change
Affected areas
How to test
go test ./core/providers/gemini/...The two new unit tests (
TestEgressKeepsThoughtTextAlongsideSignatureandTestIngressSendsThoughtTextForStandaloneReasoning) directly cover the fixed paths. The e2e Postman collection can be run against a live Gemini endpoint to confirm the API accepts replayed thought text.Breaking changes
Related issues
Security considerations
No auth, secrets, PII, or sandboxing implications. The change only affects how thought block content is serialised when constructing requests to the Gemini API.
Checklist
docs/contributing/README.mdand followed the guidelines