Skip to content
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 25 additions & 7 deletions go/plugins/googlegenai/gemini.go
Original file line number Diff line number Diff line change
Expand Up @@ -1008,21 +1008,22 @@ func toGeminiPart(p *ai.Part) (*genai.Part, error) {
}
}
fc := genai.NewPartFromFunctionCall(toolReq.Name, input)
// 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).
Comment on lines +1011 to +1013
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
// 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.

if p.Metadata != nil {
if sig, ok := p.Metadata["signature"].([]byte); ok {
fc.ThoughtSignature = sig
}
fc.ThoughtSignature = metadataSignature(p.Metadata)
}
return fc, nil
default:
return nil, fmt.Errorf("unknown part in the request: %q", p.Kind)
}

// Restore ThoughtSignature if present in metadata.
// Handle both []byte (original) and string (after JSON clone roundtrip
// which base64-encodes []byte values).
Comment on lines +1022 to +1024
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

Suggested change
// 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.

if p.Metadata != nil {
if sig, ok := p.Metadata["signature"].([]byte); ok {
gp.ThoughtSignature = sig
}
gp.ThoughtSignature = metadataSignature(p.Metadata)
}

return gp, nil
Expand All @@ -1033,6 +1034,23 @@ func toGeminiPart(p *ai.Part) (*genai.Part, error) {
// - Start with a letter or an underscore
// - Must be alphanumeric and can include underscores, dots or dashes
// - Maximum length of 64 chars
// 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
}

Comment on lines +1037 to +1053
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

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.

func validToolName(n string) bool {
re := regexp.MustCompile(toolNameRegex)

Expand Down
Loading