Skip to content

Conversation

@jeffbryner
Copy link
Contributor

fixes #2053

@seanzhou1023
Copy link
Collaborator

please update the commit message to follow https://www.conventionalcommits.org/en/v1.0.0/ and update the PR title to match it.

@jeffbryner jeffbryner changed the title Add required parameters with defaults if missing fix: Add required parameters with defaults if missing Jul 22, 2025
@codeninja
Copy link

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
Copy link
Contributor Author

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.
@codeninja I don't believe this code has been merged into 1.8 as the PR is still open.

@codeninja
Copy link

@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.

@jeffbryner
Copy link
Contributor Author

jeffbryner commented Jul 27, 2025

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.

@jeffbryner
Copy link
Contributor Author

Thanks!

copybara-service bot pushed a commit that referenced this pull request Nov 10, 2025
Merge #2054

fixes #2053

Co-authored-by: Xuan Yang <xygoogle@google.com>
COPYBARA_INTEGRATE_REVIEW=#2054 from jeffbryner:fix-required-params dd75a0b
PiperOrigin-RevId: 830251203
@adk-bot
Copy link
Collaborator

adk-bot commented Nov 10, 2025

Thank you @jeffbryner for your contribution! 🎉

Your changes have been successfully imported and merged via Copybara in commit 352dd99.

Closing this PR as the changes are now in the main branch.

@adk-bot adk-bot closed this Nov 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

OpenAPI tool ignores required parameters for rest interfaces

5 participants