-
Notifications
You must be signed in to change notification settings - Fork 2.4k
fix(tools): preserve integer precision in tool calls (#3592) #3594
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
Summary of ChangesHello @sarojrout, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a significant data integrity problem within the ADK's function calling mechanism, specifically for the Gemini API. By strategically altering how large integers are represented in the generated schema and implementing a robust runtime conversion logic, the system now guarantees that integer values maintain their full precision, eliminating unintended floating-point approximations during tool execution. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
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.
Code Review
This pull request effectively addresses a critical precision loss issue with large integers in Gemini function calls. The approach of converting integers to strings in the schema for the Gemini variant and then safely converting them back to Python ints at runtime is well-implemented. The changes are robust, considering various numeric types like float and Decimal during the conversion process. The inclusion of unit tests for both schema generation and runtime conversion is excellent. I have a minor suggestion to improve consistency between the schema generation and parsing logic.
|
|
||
| logger = logging.getLogger('google_adk.' + __name__) | ||
|
|
||
| _INTEGER_STRING_PATTERN = r'^-?\d+$' |
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 regex pattern for integer strings is inconsistent with the one used for parsing in function_tool.py. Here, it is r'^-?\d+$', while in function_tool.py it is r'^[-+]?\d+$', which also allows a leading +. While the Gemini model might not generate a leading +, it's best practice to keep the schema pattern and the parser logic consistent. This makes the system more robust. Please consider updating the pattern to allow for an optional positive sign.
| _INTEGER_STRING_PATTERN = r'^-?\d+$' | |
| _INTEGER_STRING_PATTERN = r'^[-+]?\d+$' |
| assert declaration.parameters is not None | ||
| param_schema = declaration.parameters.properties['count'] | ||
| assert param_schema.type == types.Type.STRING | ||
| assert param_schema.pattern == '^-?\\d+$' |
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.
|
Thanks for creating this PR! After talking with team, this might not be the best way to resolve this issue. I will comment in the issue for the alternatives. |
|
I will work on the review comments which you had given earlier and those are valid points. for example: |
|
@sarojrout Sorry, as I mentioned in #3594 (comment), this might not be the best way to resolve the issue, since it affects all the users of ADK. I'm reaching out the upstream team for a more elegant fix. See #3592 (comment), @yyyu-google is working on it. |
Please ensure you have read the contribution guide before creating a pull request.
Link to Issue or Description of Change
1. Link to an existing issue (if applicable):
2. Or, if no issue exists, describe the change:
If applicable, please follow the issue templates to provide as much detail as
possible.
Problem:
Gemini function calls serialize integer parameters as JSON numbers. When those exceed double precision, they arrive in ADK as floats (e.g. 1e16) and Python converts them to float, so user tools lose precision (10000000000000001 + 123456789 → 10000000123456788).
Solution:
During schema generation (Gemini variant), rewrite integer parameters/returns as strings with regex ^-?\d+$ so the model sends them as exact strings.
When the tool executes, FunctionTool now coerces such strings/Decimals/floats back to precise Python int values before invoking the code.
Added unit tests covering both the schema change and the runtime conversion.
Testing Plan
Please describe the tests that you ran to verify your changes. This is required
for all PRs that are not small documentation or typo fixes.
Unit Tests:
pytest tests/unittests/tools/test_function_tool.py::test_run_async_with_large_integer_strings tests/unittests/tools/test_from_function_with_options.py::test_from_function_with_options_int_parameter_gemini -q
Manual End-to-End (E2E) Tests:
Please provide instructions on how to manually test your changes, including any
necessary setup or configuration. Please provide logs or screenshots to help
reviewers better understand the fix.
Checklist
Additional context
Created a simple add(a: int, b: int) tool via FunctionTool.
Invoked it with Gemini-style strings ("10000000000000001" and "123456789") through ADK; observed exact result 10000000123456790.