-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Add warning about OpenAI models + dict typed tools #3712
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?
Add warning about OpenAI models + dict typed tools #3712
Conversation
|
|
||
| def _map_tool_definition(self, f: ToolDefinition) -> responses.FunctionToolParam: | ||
| if _has_dict_typed_params(f.parameters_json_schema): | ||
| warnings.warn( |
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.
Let's move the warning into the helper method so that we don't repeat the text
| tools.append({'type': 'image_generation'}) | ||
| return tools | ||
|
|
||
| def _map_tool_definition(self, f: ToolDefinition) -> responses.FunctionToolParam: |
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.
I think we'll also need to do the check in _map_json_schema
| 'properties': { | ||
| 'dict_list': {'type': 'array', 'items': {'type': 'object', 'additionalProperties': {'type': 'integer'}}} | ||
| } | ||
| } |
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.
I'd prefer to test this with a tool function that actually has a dict argument, or a BaseModel argument that itself has a dict field, instead of testing the schemas directly.
So maybe can we build an agent with a tool like that, then run it, and test that a warning was emitted?
| f"Tool '{f.name}' has dict-typed parameters that OpenAI's API will silently ignore. " | ||
| f'Use a Pydantic BaseModel with explicit fields instead of dict types, ' | ||
| f'or switch to a different provider which supports dict types. ' | ||
| f'See: https://github.com/pydantic/pydantic-ai/issues/3654', |
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.
| f"Tool '{f.name}' has dict-typed parameters that OpenAI's API will silently ignore. " | |
| f'Use a Pydantic BaseModel with explicit fields instead of dict types, ' | |
| f'or switch to a different provider which supports dict types. ' | |
| f'See: https://github.com/pydantic/pydantic-ai/issues/3654', | |
| f"Tool {f.name!r} has `dict`-typed parameters that OpenAI's API will silently ignore. " | |
| f'Use a Pydantic `BaseModel`, `dataclass`, or `TypedDict` with explicit fields instead, ' | |
| f'or switch to a different provider which supports `dict` types. ' | |
| f'See: https://github.com/pydantic/pydantic-ai/issues/3654', |
Closes #3654 by adding a warning when an OpenAI-powered agent has a tool with a
dict-typed argument.