Skip to content

fix(core): raise an actionable error for Pydantic-reserved tool parameter names - #4800

Open
nileshpatil6 wants to merge 2 commits into
openai:mainfrom
nileshpatil6:fix/reserved-pydantic-param-names
Open

fix(core): raise an actionable error for Pydantic-reserved tool parameter names#4800
nileshpatil6 wants to merge 2 commits into
openai:mainfrom
nileshpatil6:fix/reserved-pydantic-param-names

Conversation

@nileshpatil6

Copy link
Copy Markdown
Contributor

Summary

A tool parameter named model_post_init builds a schema and registers successfully, then breaks at every invocation. Pydantic treats the field as the model's post-init hook and calls the argument value:

@function_tool
def send(model_post_init: str) -> str:
    return model_post_init

function_schema() returns fine, then constructing the arguments model raises TypeError: 'str' object is not callable from inside Pydantic. Nothing points at the parameter name.

Two related names fail earlier with equally opaque errors: model_config is consumed by create_model as model configuration (TypeError: 'FieldInfo' object is not iterable), and protected-namespace names such as model_dump or model_validate surface a raw Pydantic ValueError.

Scope

This is the actionable failure referenced when #4715 was closed, not a change to which names are supported. It does not touch protected_namespaces and does not make these names usable as parameters. Renaming the parameter or using a wrapper stays the supported path, and the error message says so. Names Pydantic accepts today (model_copy, model_extra, model_fields, ...) keep working unchanged.

model_post_init is checked up front because it fails silently otherwise. The remaining names are identified only after create_model has already failed, by probing Pydantic with a trivial one field model, so the check follows the installed Pydantic version rather than a hardcoded list. Failures unrelated to a parameter name propagate unchanged.

Tests

Four regression tests covering each name and the multiple-name case. They fail on main with the raw TypeError / ValueError and pass with the change. tests/test_function_schema.py passes in full (57), along with the surrounding function_tool, decorator, strict_schema and agent-as-tool modules. Ruff format and check are clean.

@sylvesterkaczmarek sylvesterkaczmarek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is still a silent reserved-name case this detector misses. create_model() consumes some names as its own keyword arguments before they can become fields. On Pydantic 2.13.4, which is inside this repo's supported >=2.12.2,<3 range, both __doc__ and __module__ make model construction succeed, so _pydantic_rejects_param_name() returns False, but neither name appears in dynamic_model.model_fields.

That means a tool such as def f(__doc__: str): ... still gets a malformed arguments model instead of the new UserError; the value is absent from instance field storage and the call path can fall back to class metadata. Could we also guard the create_model control keywords, or more generally verify after construction that every requested field survived in dynamic_model.model_fields? A regression test with __doc__ would cover the silent-consumption case.

@nileshpatil6

Copy link
Copy Markdown
Contributor Author

Good catch, confirmed on pydantic 2.13.5:

__doc__            constructed=YES  in_model_fields=False
__module__         constructed=YES  in_model_fields=False
__base__           constructed=NO   (TypeError)
model_config       constructed=NO   (TypeError)

So the probe returns False for __doc__ and __module__ and the malformed model went through, exactly as you described.

Took the more general option you suggested rather than listing the control keywords: after create_model succeeds, any requested name missing from dynamic_model.model_fields raises the same UserError. That covers __doc__ and __module__ today and keeps holding if Pydantic consumes more names later, without me having to track their keyword list.

Added regression tests for both names. They fail on the previous commit and pass now. Full file: 59 passed. Ruff format, ruff check and mypy are clean.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants