Conversation
📝 WalkthroughWalkthroughAdds a shared ChangesthoughtSignature bypass token fix
Sequence Diagram(s)sequenceDiagram
participant Client
participant GeminiHandler
participant GeminiAPI
Client->>GeminiHandler: send turn 1 with tools
GeminiHandler->>GeminiAPI: generateContentStream
GeminiAPI-->>GeminiHandler: stream chunk with thoughtSignature + functionCall
GeminiHandler->>GeminiHandler: store thoughtSignature via getThoughtSignature()
Client->>GeminiHandler: send turn 2 (tool result)
GeminiHandler->>GeminiAPI: generateContentStream with stored thoughtSignature on functionCall
Note over GeminiHandler,GeminiAPI: falls back to GEMINI_THOUGHT_SIGNATURE_BYPASS if no stored signature
Estimated code review effort🎯 2 (Simple) | ⏱️ ~15 minutes Possibly related issues
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
🧹 Nitpick comments (3)
src/api/providers/__tests__/gemini.spec.ts (1)
176-176: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate base64 computation instead of importing the shared constant.
Same issue as in
gemini-format.spec.ts:Buffer.from("skip_thought_signature_validator").toString("base64")reimplementsGEMINI_THOUGHT_SIGNATURE_BYPASSinstead of importing it fromgemini-format.ts.♻️ Proposed fix
+import { GEMINI_THOUGHT_SIGNATURE_BYPASS } from "../../transform/gemini-format" ... - const expectedBypass = Buffer.from("skip_thought_signature_validator").toString("base64") + const expectedBypass = GEMINI_THOUGHT_SIGNATURE_BYPASS🤖 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 `@src/api/providers/__tests__/gemini.spec.ts` at line 176, The gemini test is re-creating the bypass token instead of using the shared source of truth; update the `gemini.spec.ts` test to import `GEMINI_THOUGHT_SIGNATURE_BYPASS` from `gemini-format.ts` and use that constant in place of the inline `Buffer.from(...).toString("base64")` computation. This keeps the expectation aligned with the production value and avoids duplicating the base64 logic in the test.src/api/transform/__tests__/gemini-format.spec.ts (1)
126-130: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winImport the shared constant instead of recomputing it.
expectedBypassTokenreimplements the sameBuffer.from(...).toString("base64")formula used ingemini-format.tsrather than importingGEMINI_THOUGHT_SIGNATURE_BYPASS. This makes the assertion tautological against the constant's own definition — it won't catch drift if the constant's value changes.♻️ Proposed fix
-import { convertAnthropicMessageToGemini } from "../gemini-format" +import { convertAnthropicMessageToGemini, GEMINI_THOUGHT_SIGNATURE_BYPASS } from "../gemini-format" ... - const expectedBypassToken = Buffer.from("skip_thought_signature_validator").toString("base64") + const expectedBypassToken = GEMINI_THOUGHT_SIGNATURE_BYPASSAlso applies to: 141-141
🤖 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 `@src/api/transform/__tests__/gemini-format.spec.ts` around lines 126 - 130, The test in gemini-format.spec.ts is recomputing the bypass token instead of verifying the shared value, so update the assertion to import and use GEMINI_THOUGHT_SIGNATURE_BYPASS from gemini-format.ts (or the module that defines it) rather than calling Buffer.from(...).toString("base64"). Keep the expectation tied to the shared constant in the relevant test cases, including the repeated assertion around the later line referenced in the review, so the test will fail if the constant drifts.src/api/providers/lite-llm.ts (1)
193-198: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueStale comment doesn't mention base64 encoding.
This comment (unchanged by this PR) still describes
"skip_thought_signature_validator"as the bypass value without noting it must be base64-encoded — the exact distinction this PR fixes elsewhere. Worth aligning for consistency, though low priority since it's not on the changed lines.🤖 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 `@src/api/providers/lite-llm.ts` around lines 193 - 198, Update the stale Gemini comment in lite-llm.ts so it matches the new behavior: the note around isGeminiModel and the thought-signature bypass should explicitly mention that "skip_thought_signature_validator" must be base64-encoded. Keep the existing explanation about Gemini 3 tool-call validation, but align the wording with the fix applied elsewhere so the comment is accurate and consistent.
🤖 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.
Nitpick comments:
In `@src/api/providers/__tests__/gemini.spec.ts`:
- Line 176: The gemini test is re-creating the bypass token instead of using the
shared source of truth; update the `gemini.spec.ts` test to import
`GEMINI_THOUGHT_SIGNATURE_BYPASS` from `gemini-format.ts` and use that constant
in place of the inline `Buffer.from(...).toString("base64")` computation. This
keeps the expectation aligned with the production value and avoids duplicating
the base64 logic in the test.
In `@src/api/providers/lite-llm.ts`:
- Around line 193-198: Update the stale Gemini comment in lite-llm.ts so it
matches the new behavior: the note around isGeminiModel and the
thought-signature bypass should explicitly mention that
"skip_thought_signature_validator" must be base64-encoded. Keep the existing
explanation about Gemini 3 tool-call validation, but align the wording with the
fix applied elsewhere so the comment is accurate and consistent.
In `@src/api/transform/__tests__/gemini-format.spec.ts`:
- Around line 126-130: The test in gemini-format.spec.ts is recomputing the
bypass token instead of verifying the shared value, so update the assertion to
import and use GEMINI_THOUGHT_SIGNATURE_BYPASS from gemini-format.ts (or the
module that defines it) rather than calling Buffer.from(...).toString("base64").
Keep the expectation tied to the shared constant in the relevant test cases,
including the repeated assertion around the later line referenced in the review,
so the test will fail if the constant drifts.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 2230a2c8-0516-4e7e-956e-98212f135049
📒 Files selected for processing (4)
src/api/providers/__tests__/gemini.spec.tssrc/api/providers/lite-llm.tssrc/api/transform/__tests__/gemini-format.spec.tssrc/api/transform/gemini-format.ts
Related GitHub Issue
Closes: #536
Description
Gemini 3.x (including
gemini-3.1-pro-previewandgemini-3.5-flash) validatesPart.thoughtSignaturestrictly on Vertex AI. The field is documented by the Google GenAI SDK as "Encoded as base64 string". The fallback bypass token ingemini-format.tswas being sent as a plain string ("skip_thought_signature_validator"), while the same token inlite-llm.tswas already correctly base64-encoded.Vertex AI enforces base64 validation more strictly than the direct Gemini API. When the plain string was sent on turn 2+ (after a tool call), Vertex returned an empty response, triggering the infinite retry loop reported in #536.
How:
GEMINI_THOUGHT_SIGNATURE_BYPASSas a named export insrc/api/transform/gemini-format.ts— the single source of truth for the base64-encoded bypass token.gemini-format.tsto use the constant instead of the inline plain string.lite-llm.tsto import and use the same constant, eliminating the duplicateBuffer.from(...)computation.Reviewers should note:
thoughtSignatureround-trip from real API responses is unaffected — only the fallback/bypass path changed.Test Procedure
TDD approach: failing tests written first, then fix applied.
src/api/transform/__tests__/gemini-format.spec.ts— updated existing tool-use test to assert base64-encoded bypass token; added assertion documents the exact Vertex AI failure mode.src/api/providers/__tests__/gemini.spec.ts— addedthoughtSignature round-trip (issue #536)suite covering: capture from stream, round-trip through history on turn 2, base64 fallback for cross-model history, disabled-reasoning edge case, and no-tools boundary.src/api/providers/__tests__/lite-llm.spec.ts— existing tests continue to pass unchanged (they already asserted the base64 value).Pre-Submission Checklist
Documentation Updates
Get in Touch
edelauna@gmail.com
Summary by CodeRabbit
Bug Fixes
Tests