fix(core): reject **kwargs keys that collide with named tool parameters - #4674
Open
rajarshidattapy wants to merge 2 commits into
Open
fix(core): reject **kwargs keys that collide with named tool parameters#4674rajarshidattapy wants to merge 2 commits into
rajarshidattapy wants to merge 2 commits into
Conversation
`FuncSchema.to_call_args` splats the `**kwargs` dict last, after the named parameters, with no intersection check. A schema-valid payload whose kwargs dict carries a key naming one of the function's own parameters therefore had two wrong outcomes: for a keyword-only parameter the validated value was silently replaced, and for a positional-or-keyword parameter the call raised `TypeError: got multiple values for argument`, which escaped the invocation path as an internal error rather than as feedback the model could act on. Such a call cannot be expressed in Python, so `to_call_args` now raises `ModelBehaviorError` naming the conflicting keys, matching how the invocation path already reports validation failures. Positional-only parameters and `*args` are deliberately left out of the reserved set: `f(1, a=2)` for `def f(a, /, **kw)` is a legal call that routes `a=2` into `**kw`, and those cases keep working. The context parameter is covered too -- it is passed positionally, so a kwargs key of the same name broke the call the same way.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request fixes
FuncSchema.to_call_argsaccepting**kwargspayloads that no Python call can express, which either silently discarded a validated argument or crashed the tool call with an unhandledTypeError.The bug
A non-strict function tool that takes
**kwargsaccepts schema-valid input whose kwargs dict names one of the function's own parameters — the generated Pydantic model has a separate field for each named parameter and nothing constrains the keys inside the kwargs field.to_call_argssplats that dict last, after the named parameters, with no intersection check. Both outcomes are wrong:The invocation path converts validation failures into
ModelBehaviorErrorone line earlier, but theTypeErrorfrom the call itself was not converted, so the run failed with an internal error instead of the model receiving actionable feedback.The fix
to_call_argsnow raisesModelBehaviorErrornaming the conflicting keys, consistent with how the invocation path already reports validation failures. The check lives into_call_argsrather than at the call site because every caller routes through it.Only parameters that the call binds by name are reserved. Two cases are deliberately left working, because the key genuinely belongs to
**kwargsthere:f(1, a=2)fordef f(a, /, **kw)is legal and routesa=2intokw.*args— the name binds no argument, sov(1, rest=5)fordef v(*rest, **kw)is legal.The fix also covers a case the report did not mention but that shares the root cause: the context parameter is passed positionally by the invocation path, so a kwargs key naming it (
def f(ctx: RunContextWrapper[Any], **kw)withkw={"ctx": ...}) failed with the samegot multiple valuesTypeError. It is now reported the same way.Behavior notes
strict_json_schema=False) are affected; strict mode already rejects**kwargstools at definition time.Tests
tests/test_function_schema.pygains a parametrized rejection test covering keyword-only, positional-or-keyword, keyword-only-after-*args, and context-parameter collisions, plus two tests pinning the positional-only and*argscases that must keep working.tests/test_function_tool.pygains an end-to-end check that the collision reaches the caller asModelBehaviorErrorrather thanTypeError. The rejection tests fail onmain.Resolves #4669.