-
Notifications
You must be signed in to change notification settings - Fork 2k
feat: Added support for enums as arguments for function tools #3088
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?
feat: Added support for enums as arguments for function tools #3088
Conversation
Summary of ChangesHello @SOORAJTS2001, 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 introduces a significant enhancement by enabling the use of Python 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 adds support for using enums as parameters in function tools. The implementation correctly identifies enum types and extracts their values for the schema. However, there is a bug in handling default values for enum parameters, which will cause an exception. I've provided a code suggestion to fix this. Additionally, the unit tests for this new feature could be expanded to cover cases with default values to prevent such issues in the future. I've included examples of test cases that should be added.
if not _is_default_value_compatible(param.default, param.annotation): | ||
raise ValueError(default_value_error_msg) | ||
schema.default = param.default |
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 handling of default values for Enum
parameters is incorrect and will lead to a ValueError
.
There are two issues with the current implementation:
- The
_is_default_value_compatible
function does not supportEnum
types and will always returnFalse
, causing an exception to be raised on line 154 if a default value is present. - On line 155,
schema.default
is assigned theEnum
member itself (param.default
) instead of its value. Since the schema type isSTRING
, the default value should also be a string (i.e.,param.default.value
).
The suggested change below fixes both problems by performing the compatibility check locally and ensuring the correct value is assigned to schema.default
.
if not _is_default_value_compatible(param.default, param.annotation): | |
raise ValueError(default_value_error_msg) | |
schema.default = param.default | |
default_value = param.default.value if isinstance(param.default, Enum) else param.default | |
if default_value not in schema.enum: | |
raise ValueError(default_value_error_msg) | |
schema.default = default_value |
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.
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.
As per this PR Gemini API now supports default value, hence commented the above function
def test_enums(): | ||
|
||
class InputEnum(Enum): | ||
AGENT = "agent" | ||
TOOL = "tool" | ||
|
||
def simple_function(input:InputEnum): | ||
return input.value | ||
|
||
function_decl = _automatic_function_calling_util.build_function_declaration( | ||
func=simple_function | ||
) | ||
|
||
assert function_decl.name == 'simple_function' | ||
assert function_decl.parameters.type == 'OBJECT' | ||
assert function_decl.parameters.properties['input'].type == 'STRING' | ||
assert function_decl.parameters.properties['input'].enum == ['agent', 'tool'] |
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 new test for enums only covers the basic case without a default value. To ensure the implementation is robust and to prevent regressions, it's important to add test cases for function parameters with default values. This would have helped catch the bug with default value handling in the current implementation.
Please consider adding tests for the following scenarios:
- A valid default value provided as an enum member (e.g.,
param: MyEnum = MyEnum.VALUE
). - A valid default value provided as a raw value (e.g.,
param: MyEnum = 'value'
). - An invalid default value, which should raise a
ValueError
.
Here's an example of how you could structure these tests:
import pytest
def test_enum_with_default_member():
class InputEnum(Enum):
AGENT = "agent"
TOOL = "tool"
def simple_function(input: InputEnum = InputEnum.AGENT):
return input.value
function_decl = _automatic_function_calling_util.build_function_declaration(
func=simple_function
)
assert function_decl.parameters.properties['input'].default == 'agent'
def test_enum_with_invalid_default():
class InputEnum(Enum):
AGENT = "agent"
TOOL = "tool"
def simple_function(input: InputEnum = "invalid_default"):
return input.value
with pytest.raises(ValueError):
_automatic_function_calling_util.build_function_declaration(
func=simple_function
)
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.
Added test cases for default value as well as invalid value
fix: Add more test cases inside `test_build_function_declaration.py` for passing Enums as arguments
@seanzhou1023 please take a look at this |
Hi @surajksharma07 ! |
Hi @SOORAJTS2001, Thank you for your contribution to the ADK project. The changes in Before we proceed with the final review and merge, could you please address the following points?
Once these adjustments are made, we can proceed with the final testing and merge. Let us know if you have any questions. |
![]() Hi @surajksharma07 |
@seanzhou1023 please take a look into this This PR from @SOORAJTS2001 adds support for The implementation looks good, and the contributor has added comprehensive unit tests covering the main logic and edge cases as requested. Thanks. |
src/google/adk/tools/_function_parameter_parse_util.py
Ran the whole unittests

For unittests/tools

Test result for test_build_function_declaration.py
