-
Notifications
You must be signed in to change notification settings - Fork 3.3k
fix: Support list[BaseModel] annotation about FunctionTool args #3186
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
Changes from 7 commits
985e948
f07a5aa
b139406
2bea40c
fe575f7
464de3b
2ecf493
7a4aa5c
efa45ac
49f3987
7589882
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 |
|---|---|---|
|
|
@@ -102,6 +102,7 @@ def _preprocess_args(self, args: dict[str, Any]) -> dict[str, Any]: | |
|
|
||
| Currently handles: | ||
| - Converting JSON dictionaries to Pydantic model instances where expected | ||
| - Converting lists of JSON dictionaries to lists of Pydantic model instances | ||
|
|
||
| Future extensions could include: | ||
| - Type coercion for other complex types | ||
|
|
@@ -129,8 +130,39 @@ def _preprocess_args(self, args: dict[str, Any]) -> dict[str, Any]: | |
| if len(non_none_types) == 1: | ||
| target_type = non_none_types[0] | ||
|
|
||
| # Check if the target type is a list | ||
| if get_origin(target_type) is list: | ||
| list_args = get_args(target_type) | ||
| if list_args: | ||
| element_type = list_args[0] | ||
|
|
||
| # Check if the element type is a Pydantic model | ||
| if inspect.isclass(element_type) and issubclass( | ||
| element_type, pydantic.BaseModel | ||
| ): | ||
| # Skip conversion if the value is None | ||
| if args[param_name] is None: | ||
| continue | ||
|
|
||
| # Convert list elements to Pydantic models | ||
| if isinstance(args[param_name], list): | ||
| converted_list = [] | ||
| for item in args[param_name]: | ||
| try: | ||
| converted_list.append(element_type.model_validate(item)) | ||
| except Exception as e: | ||
| logger.warning( | ||
| f"Failed to convert item in '{param_name}' to Pydantic" | ||
| f' model {element_type.__name__}: {e}' | ||
| ) | ||
|
|
||
| # Keep the original value if conversion fails | ||
| converted_list.append(item) | ||
|
Collaborator
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. When a list element fails Pydantic validation you now append the original dict back into converted_list. The downstream tool function still receives that list, so any comprehensions or attribute access (user.name) will now raise (dict has no attribute name). I think we skipped the bad element before, so the tool could still run on the valid ones. If we want to preserve the original data for feedback, we either need to keep the entire argument unconverted so tool decides or continue skipping/raising |
||
|
|
||
| converted_args[param_name] = converted_list | ||
|
|
||
| # Check if the target type is a Pydantic model | ||
| if inspect.isclass(target_type) and issubclass( | ||
| elif inspect.isclass(target_type) and issubclass( | ||
| target_type, pydantic.BaseModel | ||
| ): | ||
| # Skip conversion if the value is None and the parameter is Optional | ||
|
|
@@ -146,7 +178,7 @@ def _preprocess_args(self, args: dict[str, Any]) -> dict[str, Any]: | |
| except Exception as e: | ||
| logger.warning( | ||
| f"Failed to convert argument '{param_name}' to Pydantic model" | ||
| f' {target_type.__name__}: {e}' | ||
| f' model {target_type.__name__}: {e}' | ||
| ) | ||
| # Keep the original value if conversion fails | ||
| pass | ||
|
|
||
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.
While using
except Exceptionis robust, it's generally better practice to catch more specific exceptions. This prevents accidentally catching and silencing unrelated errors (likeKeyboardInterruptorSystemExit) and makes the code's intent clearer. In this case,pydantic.ValidationErroris the most expected exception during model validation.