Python: fix: preserve null tool arguments#6123
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
This PR fixes tool argument serialization so explicitly provided null/None values for required-but-nullable parameters are preserved, while unknown null arguments are not reintroduced—covering both direct FunctionTool.invoke calls and the auto function-calling flow.
Changes:
- Added
_dump_tool_argumentsto preserve explicitly providedNonefields when dumping validated Pydantic models. - Updated tool invocation and auto-invocation flows to use
_dump_tool_argumentsinstead ofmodel_dump(exclude_none=True). - Added tests to validate preservation of nullable required args and dropping of unknown null args, including function-calling scenarios.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| python/packages/core/agent_framework/_tools.py | Adds helper to dump tool args while preserving explicit None, and wires it into invoke/auto-invoke paths. |
| python/packages/core/tests/core/test_tools.py | Adds tests for direct tool invocation behavior with nullable required args and unknown null args. |
| python/packages/core/tests/core/test_function_invocation_logic.py | Adds test ensuring base client function-calling preserves null arguments through execution. |
| def _dump_tool_arguments(model: BaseModel, *, include_none_from: Iterable[str] | None = None) -> dict[str, Any]: | ||
| arguments = model.model_dump(exclude_none=True) | ||
| field_names = type(model).model_fields | ||
| for name in include_none_from if include_none_from is not None else model.model_fields_set: |
| exclude_none=True | ||
| parsed_arguments = _dump_tool_arguments( | ||
| self.input_model.model_validate(parsed_arguments), | ||
| include_none_from=parsed_arguments, |
| ): | ||
| raise TypeError(f"Expected {self.input_model.__name__}, got {type(arguments).__name__}") | ||
| parsed_arguments = arguments.model_dump(exclude_none=True) | ||
| parsed_arguments = _dump_tool_arguments(arguments) |
|
|
||
| def _dump_tool_arguments(model: BaseModel, *, include_none_from: Iterable[str] | None = None) -> dict[str, Any]: | ||
| arguments = model.model_dump(exclude_none=True) | ||
| field_names = type(model).model_fields |
There was a problem hiding this comment.
What happens with an aliased field on the mapping path? include_none_from there is the raw caller keys, but field_names / getattr use Python field names. With a custom input_model using Field(alias=...) + populate_by_name, an explicit null sent under the alias key fails name in field_names, so the None is dropped, re-introducing the exact bug this fixes (aliased models only; auto-generated @tool models are unaffected). Could we resolve alias -> field name first? One option, which also folds in the non-str-key guard:
| field_names = type(model).model_fields | |
| field_names = type(model).model_fields | |
| alias_to_field = {f.alias: n for n, f in field_names.items() if isinstance(f.alias, str)} | |
| for name in include_none_from if include_none_from is not None else model.model_fields_set: | |
| if not isinstance(name, str): | |
| continue | |
| resolved = alias_to_field.get(name, name) | |
| if resolved in field_names and getattr(model, resolved, None) is None: | |
| arguments[resolved] = None |
Python Test Coverage Report •
Python Unit Test Overview
|
||||||||||||||||||||||||||||||
|
Please make sure you're not duplicating work. #5944 was the first PR introduced with the fix. |
Fixes #5934.
This keeps explicitly provided
nulltool arguments when the inferred Pydantic input model validates the payload. The oldmodel_dump(exclude_none=True)path removed those keys before schema validation and invocation, so a required-but-nullable argument looked missing.The helper still omits default
Nonefields, and it only restores fields known to the validated model so unknownnullkeys are not reintroduced after validation.To verify
uv run --no-sync pytest python\packages\core\tests\core\test_tools.py::test_tool_invoke_preserves_required_nullable_argument python\packages\core\tests\core\test_tools.py::test_tool_invoke_does_not_reintroduce_unknown_null_arguments python\packages\core\tests\core\test_function_invocation_logic.py::test_base_client_with_function_calling_preserves_null_arguments -quv run --no-sync ruff check agent_framework\_tools.py tests\core\test_tools.py tests\core\test_function_invocation_logic.pyuv run --no-sync mypy agent_framework\_tools.pypython -m py_compile python\packages\core\agent_framework\_tools.py python\packages\core\tests\core\test_tools.py python\packages\core\tests\core\test_function_invocation_logic.pygit diff --check