-
-
Notifications
You must be signed in to change notification settings - Fork 136
Refactor tool configuration handling and add deep filtering for parameters #84
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes a tool-calling regression where tool schemas containing $schema
fields were causing "Unknown name" JSON payload errors from the Gemini API. The fix replaces shallow filtering with deep recursive filtering to thoroughly remove all keys starting with '$' from nested parts of tool JSON schemas.
Key changes:
- Replaced shallow parameter filtering with recursive deep filtering
- Added proper handling of arrays and nested objects in tool schemas
- Improved code organization with extracted helper function
let toolConfig = {}; | ||
|
||
// Recursive function to deep-filter keys starting with '$' (removes internal/system keys) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The comment should include JSDoc format documentation explaining the function's parameters, return value, and purpose for better code documentation standards.
/** | |
* Recursively filters out keys starting with '$' from objects and arrays. | |
* Removes internal/system keys from the provided object or array. | |
* | |
* @param {unknown} obj - The object or array to filter. | |
* @returns {unknown} A new object or array with keys starting with '$' removed. | |
*/ |
Copilot uses AI. Check for mistakes.
|
||
// Recursive function to deep-filter keys starting with '$' (removes internal/system keys) | ||
|
||
const deepFilter = (obj: unknown): unknown => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Consider extracting this recursive utility function to a separate utility file since it's a general-purpose deep filtering function that could be reused elsewhere in the codebase.
Copilot uses AI. Check for mistakes.
.filter((key) => !key.startsWith("$")) | ||
.reduce( | ||
(acc, key) => { | ||
acc[key] = deepFilter((obj as Record<string, unknown>)[key]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The type assertion could be avoided by using a type guard to check if the object has string keys, which would be safer and more explicit about the type checking.
Copilot uses AI. Check for mistakes.
I've applied a fix for the tool-calling regression. possible cause of issue Bug: Invalid Tool Schema Causes "Unknown name" JSON Payload Error Unknown name "$schema" #74
The problem was that a shallow filter was failing to remove the $schema field from nested
parts of the tool's JSON schema, causing the Gemini API to reject the request.
I've replaced it with a recursive filter that thoroughly cleans the entire tool definition
before sending it to the API. This should resolve the invalid JSON payload error.