-
Notifications
You must be signed in to change notification settings - Fork 1.6k
fix: Add required parameters with defaults if missing #2054
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
please update the commit message to follow https://www.conventionalcommits.org/en/v1.0.0/ and update the PR title to match it. |
We have noticed a serious issue with tool calling that may be related to this which is preventing us from updating from 1.6 -> 1.8. When calling our tools with 1.8, ADK is sending undefined values for optional fields, rather than omitting them entirely from the request. This gets interpreted downstream by our GO API as an invalid value. Since Null != undefined and it's expecting the request to omit the values. I suspect it may be related to this code. Would it be possible to drop values entirely from the request which have undefined values? |
|
@jeffbryner I understand. I believe our issue is introduced by line 379 in this code, above your diff, as it stands in 1.8. I found my way here while researching our issue. I believe that the the API params are being polluted with undefined values which are making their way into the request. |
Ah, I see. Probably best as a separate bug? This PR handles the case where the model doesn't send default parameters (probably assuming they are defaulted in the API) and explicitly sets defined, default values. |
fixes #2053