fix(go/plugins/googlegenai): fixed tool thought signature#4798
fix(go/plugins/googlegenai): fixed tool thought signature#4798
Conversation
Summary of ChangesHello @apascal07, 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 resolves an issue where the Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
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 fix for handling tool thought signatures by extracting the logic into a new metadataSignature helper function. This is a great improvement as it centralizes the logic, removes duplicated code, and correctly handles signatures that have been base64-encoded as strings after a JSON roundtrip. The changes are logical and include relevant tests. I have a few minor suggestions to improve code organization and reduce comment duplication for better long-term maintainability.
| // Restore ThoughtSignature if present in metadata. | ||
| // Handle both []byte (original) and string (after JSON clone roundtrip | ||
| // which base64-encodes []byte values). |
There was a problem hiding this comment.
This detailed comment is duplicated below (lines 1022-1024). Since the new metadataSignature function is well-commented, you can simplify the comments at the call sites to a single line to avoid repetition.
| // Restore ThoughtSignature if present in metadata. | |
| // Handle both []byte (original) and string (after JSON clone roundtrip | |
| // which base64-encodes []byte values). | |
| // Restore ThoughtSignature if present in metadata. |
| // Restore ThoughtSignature if present in metadata. | ||
| // Handle both []byte (original) and string (after JSON clone roundtrip | ||
| // which base64-encodes []byte values). |
There was a problem hiding this comment.
This is a duplicate of the comment above (lines 1011-1013). To improve maintainability, it's best to avoid duplicating comments. A single-line comment would be sufficient here, as the details are in the metadataSignature function's own comment.
| // Restore ThoughtSignature if present in metadata. | |
| // Handle both []byte (original) and string (after JSON clone roundtrip | |
| // which base64-encodes []byte values). | |
| // Restore ThoughtSignature if present in metadata. |
| // metadataSignature extracts the thought signature from part metadata. | ||
| // It handles both []byte (original value) and string (base64-encoded | ||
| // after a JSON clone roundtrip). | ||
| func metadataSignature(metadata map[string]any) []byte { | ||
| switch sig := metadata["signature"].(type) { | ||
| case []byte: | ||
| return sig | ||
| case string: | ||
| decoded, err := base64.StdEncoding.DecodeString(sig) | ||
| if err != nil { | ||
| return nil | ||
| } | ||
| return decoded | ||
| } | ||
| return nil | ||
| } | ||
|
|
There was a problem hiding this comment.
The placement of this new function separates the comment for validToolName from its implementation, which harms readability. For better code organization, please consider moving this new metadataSignature function to after the validToolName function.
2 participants
Checklist (if applicable):