v6.6.22 #38
v6.6.22 #38
Conversation
…-thinking-signature Revert "Fix invalid thinking signature when proxying Claude via Antigravity"
…content generation for more efficient handling of Claude requests
refactor(watcher): extract auth synthesizer to synthesizer package
Summary of ChangesHello @luispater, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request focuses on a significant refactoring of the authentication synthesis mechanism, centralizing the creation of authentication entries from various sources into a new Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a significant and well-executed refactoring of the watcher package by abstracting auth entry creation into a new synthesizer package. This greatly improves the separation of concerns and maintainability. The PR also includes a bug fix for how configuration prefixes are logged. However, I've identified a few potential regressions in the antigravity/claude translator related to the handling of "thinking" block signatures, which I've detailed in the comments. Specifically, the logic for handling unsigned thinking blocks in requests has changed, and signature handling appears to have been completely removed from the non-streaming response conversion, which could be critical.
| if contentTypeResult.Type == gjson.String && contentTypeResult.String() == "thinking" { | ||
| // Claude "thinking" blocks are internal-only. They also require a valid provider signature | ||
| // when replayed as conversation history. Since we cannot mint signatures, only forward | ||
| // thinking blocks when the client provides a non-empty signature; otherwise, drop them. | ||
| prompt := contentResult.Get("thinking").String() | ||
| signatureResult := contentResult.Get("signature") | ||
| if signatureResult.Type == gjson.String && signatureResult.String() != "" { | ||
| prompt := contentResult.Get("thinking").String() | ||
| clientContent.Parts = append(clientContent.Parts, client.Part{ | ||
| Text: prompt, | ||
| Thought: true, | ||
| ThoughtSignature: signatureResult.String(), | ||
| }) | ||
| signature := geminiCLIClaudeThoughtSignature | ||
| if signatureResult.Exists() { | ||
| signature = signatureResult.String() | ||
| } | ||
| partJSON := `{}` | ||
| partJSON, _ = sjson.Set(partJSON, "thought", true) | ||
| if prompt != "" { | ||
| partJSON, _ = sjson.Set(partJSON, "text", prompt) | ||
| } | ||
| if signature != "" { | ||
| partJSON, _ = sjson.Set(partJSON, "thoughtSignature", signature) | ||
| } | ||
| clientContentJSON, _ = sjson.SetRaw(clientContentJSON, "parts.-1", partJSON) |
There was a problem hiding this comment.
The logic for handling thinking blocks has been changed. Previously, thinking blocks without a client-provided signature were dropped, as noted by a comment in the original code. The new implementation adds a default signature (skip_thought_signature_validator) to all thinking blocks, regardless of whether a signature was provided. This is a significant change in behavior that may have unintended consequences, as the original implementation was more defensive about replaying thinking blocks. Was this change intentional? The removal of test/antigravity_claude_signature_test.go::TestAntigravityClaudeRequest_DropsUnsignedThinkingBlocks suggests it might be, but it's a critical change to make without clarification.
| contentBlocks = append(contentBlocks, map[string]interface{}{ | ||
| "type": "thinking", | ||
| "thinking": thinkingBuilder.String(), | ||
| } | ||
| if thinkingSignature != "" { | ||
| block["signature"] = thinkingSignature | ||
| } | ||
| contentBlocks = append(contentBlocks, block) | ||
| }) |
There was a problem hiding this comment.
In the non-streaming response converter (ConvertAntigravityResponseToClaudeNonStream), it appears that all handling for thinkingSignature has been removed. This seems to be a regression, as the signature might be required by the Claude API for maintaining conversation history. The streaming response converter still handles signatures, so this creates an inconsistency. Please restore the logic to handle thinkingSignature, which includes re-introducing the thinkingSignature variable, populating it from the response parts, and adding it to the thinking block here.
| contentBlocks = append(contentBlocks, map[string]interface{}{ | |
| "type": "thinking", | |
| "thinking": thinkingBuilder.String(), | |
| } | |
| if thinkingSignature != "" { | |
| block["signature"] = thinkingSignature | |
| } | |
| contentBlocks = append(contentBlocks, block) | |
| }) | |
| block := map[string]interface{}{ | |
| "type": "thinking", | |
| "thinking": thinkingBuilder.String(), | |
| } | |
| if thinkingSignature != "" { | |
| block["signature"] = thinkingSignature | |
| } | |
| contentBlocks = append(contentBlocks, block) |
| // Ensure we have an open thinking block to attach thinking/signature deltas to. | ||
| if params.ResponseType != 2 { | ||
| if partResult.Get("thought").Bool() { | ||
| if thoughtSignature := partResult.Get("thoughtSignature"); thoughtSignature.Exists() && thoughtSignature.String() != "" { |
There was a problem hiding this comment.
The fallback from thoughtSignature to thought_signature has been removed. If some backend responses still use the snake_case thought_signature key, this change could cause signatures to be missed. It would be safer to retain this fallback to ensure compatibility. For example:
thoughtSignature := partResult.Get("thoughtSignature")
if !thoughtSignature.Exists() {
thoughtSignature = partResult.Get("thought_signature")
}
if thoughtSignature.Exists() && thoughtSignature.String() != "" {
// ... existing signature handling logic
}2 participants
No description provided.