-
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?
Changes from 1 commit
5387476
598fee6
35c8949
816f2e8
936cca2
0b09400
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,7 +22,7 @@ | |
# TODO: crewai requires python 3.10 as minimum | ||
# from crewai_tools import FileReadTool | ||
from pydantic import BaseModel | ||
|
||
from enum import Enum | ||
|
||
def test_string_input(): | ||
def simple_function(input_str: str) -> str: | ||
|
@@ -219,6 +219,24 @@ def simple_function( | |
assert function_decl.parameters.properties['input_dir'].type == 'ARRAY' | ||
assert function_decl.parameters.properties['input_dir'].items.type == 'OBJECT' | ||
|
||
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'] | ||
Comment on lines
223
to
240
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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:
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 commentThe reason will be displayed to describe this comment to others. Learn more. Added test cases for default value as well as invalid value |
||
|
||
|
||
def test_basemodel_list(): | ||
class ChildInput(BaseModel): | ||
|
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 aValueError
.There are two issues with the current implementation:
_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.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
.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.
This is due to reason that the default value get wipped of by this function
Uh oh!
There was an error while loading. Please reload this page.
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