Feat/unpack pydantic args #2498
Conversation
marvin-context-protocol
bot
added
the
enhancement
Test Failure AnalysisSummary: The static analysis job failed due to a type checking error when dynamically setting the Root Cause: In placeholder_fn.__signature__ = new_sig
placeholder_fn.__annotations__ = new_annotationsPython's type checker ( Suggested Solution: Add type ignore comments to suppress the type checker warnings for these dynamic attribute assignments: placeholder_fn.__signature__ = new_sig # type: ignore[attr-defined]
placeholder_fn.__annotations__ = new_annotations # type: ignore[attr-defined]This is the standard pattern used elsewhere in the FastMCP codebase for similar situations (see AGENTS.md line 116: "Use Detailed AnalysisError OutputWhy This HappensThe feature being added unpacks Pydantic model arguments into flat parameters for the tool schema. To achieve this, the code creates a placeholder function and dynamically modifies its signature and annotations to reflect the unpacked parameters. While Python allows this at runtime, static type checkers don't have type information for these writable attributes on function objects. Alternative Approaches
The type ignore approach is cleanest and follows the project's existing patterns. Related Files
|
WalkthroughThis pull request introduces support for unpacking Pydantic model arguments into individual fields when registering tools. A new Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/fastmcp/tools/tool.py (2)
259-260: Add stricter type annotation forunpacked_models_map.The type
dict | Noneis too loose. For better type safety and documentation, specify the expected key-value types.- unpack_pydantic_args: bool = False - unpacked_models_map: dict | None = None + unpack_pydantic_args: bool = False + unpacked_models_map: dict[str, type] | None = NoneNote: Using
typeinstead oftype[pydantic.BaseModel]avoids a forward reference issue, but the intent could also be documented in a comment.
494-495: Optional: Importpydanticat module level.Since
pydanticis already imported at the module level (Line 21:from pydantic import Field, ...), the conditional import here is redundant. However, this is a minor style preference and doesn't affect functionality.- if unpack_pydantic_args: - import pydantic + if unpack_pydantic_args: + from pydantic import BaseModel as PydanticBaseModelOr add
BaseModelto the existing import at line 21 and use it directly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
tests/test_unpack_pydantic_args.pyis excluded by none and included by nonetests/tools/test_tool.pyis excluded by none and included by none
📒 Files selected for processing (2)
src/fastmcp/server/server.py(5 hunks)src/fastmcp/tools/tool.py(11 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
src/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
src/**/*.py: Python source code must use Python ≥3.10 with full type annotations
Never use bare except - be specific with exception types
Follow Ruff linting and Prettier formatting standards - run uv run prek run --all-files before committing
Prioritize readable, understandable code - clarity over cleverness, avoid obfuscated or confusing patterns
Files:
src/fastmcp/server/server.pysrc/fastmcp/tools/tool.py
🪛 GitHub Actions: Run static analysis
src/fastmcp/tools/tool.py
[error] 543-543: ruff-check failed. Unresolved attribute __signature__ on type def placeholder_fn(...) -> Unknown.
[error] 1-1: ruff-format hook reformatted 1 file (1 file reformatted, 379 files left unchanged).
[error] 1-1: ty check failed during type checking. Diagnostics reported by the type checker (multiple attribute/missing-field warnings).
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: Run tests with lowest-direct dependencies
- GitHub Check: Run tests: Python 3.10 on windows-latest
- GitHub Check: label-issue-or-pr
🔇 Additional comments (3)
src/fastmcp/server/server.py (1)
1383-1384: LGTM! Parameter threading is correct and consistent.The
unpack_pydantic_argsparameter is properly added to all overloads and forwarded through both the direct registration path (Line 1507) and the deferred decoration path viapartial(Line 1541).Also applies to: 1401-1402, 1418-1419, 1507-1508, 1541-1542
src/fastmcp/tools/tool.py (2)
347-367: Reassembly logic is correct, assuming field collisions are prevented.The logic properly:
- Iterates through unpacked models and their expected fields
- Collects matching argument values
- Constructs model instances
- Tracks consumed keys to avoid passing them through again
- Passes remaining arguments as-is
This works correctly when field names are unique, which depends on the validation suggested in the previous comment.
191-208: LGTM! Parameter and data threading is correct.The
unpack_pydantic_argsparameter andunpacked_models_mapare correctly threaded through:
Tool.from_function→FunctionTool.from_functionFunctionTool.from_function→ParsedFunction.from_functionParsedFunction→FunctionToolinstance attributesAlso applies to: 291-295, 337-339
| unpacked_models_map = {} | ||
| fn_for_schema = fn | ||
|
|
||
| if unpack_pydantic_args: | ||
| import pydantic | ||
|
|
||
| original_sig = inspect.signature(fn) | ||
| new_params = [] | ||
| has_pydantic_model = False | ||
|
|
||
| for param in original_sig.parameters.values(): | ||
| # Check if the parameter type is a Pydantic model | ||
| if isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel): | ||
| has_pydantic_model = True | ||
| unpacked_models_map[param.name] = param.annotation | ||
| # Unpack the model's fields into new parameters | ||
| for field_name, field in param.annotation.model_fields.items(): | ||
| # Create a new parameter for each field | ||
| new_param = inspect.Parameter( | ||
| name=field_name, | ||
| kind=inspect.Parameter.POSITIONAL_OR_KEYWORD, | ||
| default=field.default if not field.is_required() else inspect.Parameter.empty, | ||
| annotation=field.annotation, | ||
| ) | ||
| new_params.append(new_param) | ||
| else: | ||
| new_params.append(param) | ||
|
|
||
| if has_pydantic_model: | ||
| # Sort parameters to ensure non-defaults come first | ||
| required_params = [p for p in new_params if p.default == inspect.Parameter.empty] | ||
| optional_params = [p for p in new_params if p.default != inspect.Parameter.empty] | ||
| new_params = required_params + optional_params | ||
|
|
||
| # Create a new function with the unpacked signature for schema generation | ||
| def placeholder_fn(*args, **kwargs): ... | ||
|
|
||
| new_sig = original_sig.replace(parameters=new_params) | ||
| new_annotations = {p.name: p.annotation for p in new_params if p.annotation != inspect.Parameter.empty} | ||
|
|
||
| placeholder_fn.__signature__ = new_sig | ||
| placeholder_fn.__annotations__ = new_annotations | ||
| fn_for_schema = placeholder_fn |
There was a problem hiding this comment.
Critical: Field name collisions are not handled.
The unpacking logic doesn't validate or handle field name collisions. This can cause silent data corruption or schema generation errors in several scenarios:
-
Multiple Pydantic models with overlapping field names:
class User(BaseModel): name: str class Admin(BaseModel): name: str @tool(unpack_pydantic_args=True) def process(user: User, admin: Admin): # Both have 'name' ...
-
Pydantic field collides with non-Pydantic parameter:
class User(BaseModel): name: str @tool(unpack_pydantic_args=True) def greet(name: str, user: User): # 'name' appears twice ...
Add validation to detect and reject conflicting field names:
if unpack_pydantic_args:
import pydantic
original_sig = inspect.signature(fn)
new_params = []
has_pydantic_model = False
+ seen_field_names: set[str] = set()
+
+ # First pass: collect non-Pydantic param names
+ for param in original_sig.parameters.values():
+ if not (isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel)):
+ if param.name in seen_field_names:
+ raise ValueError(f"Duplicate parameter name: {param.name}")
+ seen_field_names.add(param.name)
for param in original_sig.parameters.values():
# Check if the parameter type is a Pydantic model
if isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel):
has_pydantic_model = True
unpacked_models_map[param.name] = param.annotation
# Unpack the model's fields into new parameters
for field_name, field in param.annotation.model_fields.items():
+ if field_name in seen_field_names:
+ raise ValueError(
+ f"Field name '{field_name}' from Pydantic model '{param.annotation.__name__}' "
+ f"conflicts with another parameter. Cannot unpack."
+ )
+ seen_field_names.add(field_name)
# Create a new parameter for each field
new_param = inspect.Parameter(|
|
||
| for param in original_sig.parameters.values(): | ||
| # Check if the parameter type is a Pydantic model | ||
| if isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel): |
There was a problem hiding this comment.
Pipeline failure: Line exceeds length limit or needs reformatting.
The pipeline indicates ruff-format reformatted this file. This line with the compound isinstance/issubclass check may be the culprit.
Consider reformatting for line length compliance:
- if isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel):
+ if (
+ isinstance(param.annotation, type)
+ and issubclass(param.annotation, pydantic.BaseModel)
+ ):📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if isinstance(param.annotation, type) and issubclass(param.annotation, pydantic.BaseModel): | |
| if ( | |
| isinstance(param.annotation, type) | |
| and issubclass(param.annotation, pydantic.BaseModel) | |
| ): |
🤖 Prompt for AI Agents
In src/fastmcp/tools/tool.py around line 503, the compound check "if
isinstance(param.annotation, type) and issubclass(param.annotation,
pydantic.BaseModel):" is too long for the formatter; refactor it to meet
line-length rules by splitting the expression across multiple lines or assigning
the annotation to a local variable first (e.g., ann = param.annotation) and then
perform the two checks on separate lines using short names and proper
indentation so the line length limit is not exceeded.
| new_param = inspect.Parameter( | ||
| name=field_name, | ||
| kind=inspect.Parameter.POSITIONAL_OR_KEYWORD, | ||
| default=field.default if not field.is_required() else inspect.Parameter.empty, | ||
| annotation=field.annotation, | ||
| ) | ||
| new_params.append(new_param) |
There was a problem hiding this comment.
Potential issue with default_factory fields.
When a Pydantic field uses default_factory instead of a direct default value, field.is_required() returns False but field.default is PydanticUndefined. This will cause the unpacked parameter to incorrectly use PydanticUndefined as its default value.
for field_name, field in param.annotation.model_fields.items():
# Create a new parameter for each field
+ # Handle default vs default_factory
+ if field.is_required():
+ default = inspect.Parameter.empty
+ elif field.default is not pydantic_core.PydanticUndefined:
+ default = field.default
+ else:
+ # Field has default_factory - cannot represent as static default
+ # Use empty to make it required in schema (factory runs at validation)
+ default = inspect.Parameter.empty
new_param = inspect.Parameter(
name=field_name,
kind=inspect.Parameter.POSITIONAL_OR_KEYWORD,
- default=field.default if not field.is_required() else inspect.Parameter.empty,
+ default=default,
annotation=field.annotation,
)Note: Fields with default_factory will appear as required in the unpacked schema since we can't represent factory functions as static defaults. Consider documenting this limitation.
🤖 Prompt for AI Agents
In src/fastmcp/tools/tool.py around lines 509 to 515, the code uses
field.default even when a field has a default_factory (where field.is_required()
is False but field.default == PydanticUndefined), which would set the parameter
default to PydanticUndefined; change the logic to detect PydanticUndefined (from
pydantic.fields import Undefined or PydanticUndefined) and treat such fields as
having no static default by using inspect.Parameter.empty, and mark them as
required in the unpacked signature (and add a short inline comment or doc note
that fields with default_factory cannot expose the factory as a static default
and therefore appear required).
| placeholder_fn.__signature__ = new_sig | ||
| placeholder_fn.__annotations__ = new_annotations |
There was a problem hiding this comment.
Fix pipeline failure: Add type ignore comment for __signature__ assignment.
The pipeline is failing because the type checker doesn't recognize that __signature__ can be assigned to function objects. This is valid Python for customizing function introspection.
- placeholder_fn.__signature__ = new_sig
+ placeholder_fn.__signature__ = new_sig # type: ignore[attr-defined]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| placeholder_fn.__signature__ = new_sig | |
| placeholder_fn.__annotations__ = new_annotations | |
| placeholder_fn.__signature__ = new_sig # type: ignore[attr-defined] | |
| placeholder_fn.__annotations__ = new_annotations |
🤖 Prompt for AI Agents
In src/fastmcp/tools/tool.py around lines 531-532, the assignment to
placeholder_fn.__signature__ triggers the type checker because __signature__ is
not recognized as an attribute on arbitrary function objects; update the
assignment line to append a type-ignore comment (e.g. "# type:
ignore[attr-defined]") so the type checker ignores this attribute assignment and
leave the existing placeholder_fn.__annotations__ assignment unchanged.
1 participant
Description
Problem & Solution
Currently, when a Pydantic model is used as a type hint for a tool argument in FastMCP, it is exposed in the tool's JSON schema as a single nested object parameter. This requires Language Models (LLMs) to construct a complex nested JSON structure to call the tool, which can be less intuitive and more prone to structural errors compared to flat argument lists.
This PR introduces a new
unpack_pydantic_args
parameter to the @tool decorator (and Tool.from_function). When set to True, FastMCP inspects Pydantic model arguments and "unpacks" their fields into individual top-level arguments in the generated tool schema. When the tool is called, FastMCP automatically reconstructs the Pydantic model instances from the flat arguments before invoking the underlying function. This provides a cleaner, more "function-like" interface for LLMs while maintaining the type safety and validation benefits of Pydantic models within the application code.
Contributors Checklist
Review Checklist