fix(amp): preserve lowercase glob tool name#2766
fix(amp): preserve lowercase glob tool name#2766edlsh wants to merge 1 commit intorouter-for-me:devfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces tool name normalization to ensure compatibility with Amp's case-sensitive tool whitelist. It adds a mapping of canonical tool names and a function to update tool names in both streaming and non-streaming API responses. Unit tests were added to verify the normalization logic. Feedback was provided regarding an optimization in the streaming logic to reduce redundant JSON parsing when accessing content_block fields.
| if gjson.GetBytes(data, "content_block.type").String() == "tool_use" { | ||
| name := gjson.GetBytes(data, "content_block.name").String() | ||
| if canonical, ok := ampCanonicalToolNames[strings.ToLower(name)]; ok && name != canonical { |
There was a problem hiding this comment.
The code performs redundant JSON parsing by calling gjson.GetBytes twice for the same content_block object. It is more efficient to retrieve the content_block once and then use Get on the resulting gjson.Result to access its fields. This is particularly relevant in streaming mode where this logic is executed for every chunk.
// Streaming: content_block.name in content_block_start events
contentBlock := gjson.GetBytes(data, "content_block")
if contentBlock.Get("type").String() == "tool_use" {
name := contentBlock.Get("name").String()
Summary
globname that Amp expects instead of rewriting it toGlobTesting