Repository navigation
Python: pass explicit None to optional kernel function parameters unchanged - #14764
Anubhav Lal (anubhavlal07) wants to merge 2 commits into
Conversation
Explicit None for optional parameters crashed or became the string None.
|
@microsoft-github-policy-service agree |
Muhammad Abdullah Butt (1AbdullahButt)
left a comment
There was a problem hiding this comment.
The fix is right for Optional parameters. But the new value is not None check (kernel_function_from_method.py:180) also skips parsing for parameters that don't accept None.
Take def f(count: int) (required, not optional), invoked with KernelArguments(count=None), or by a model that sends {"count": null}. On main that raised FunctionExecutionException: Parameter count is expected to be parsed to <class 'int'>, a clear error the auto-invoke loop reports back to the model. With this change, None is passed into the function, and it fails later with a less useful error inside the user's code (or, worse, runs with None). The same goes for count: int = 5: an explicit null now overrides the default with None rather than raising.
The decorator already records whether NoneType is in the annotation (kernel_function_decorator.py:169-170 sets is_required = False for it), but is_required also becomes False for any parameter with a default, so it can't tell the two cases apart by itself. One option is to check the annotation itself, for example by keeping the skip only when type(None) in get_args(param.type_object) or when the parameter's default_value is None. Otherwise, keep raising as before. A test with a required count: int given None would pin the boundary.
The new test's second invoke (count="3" → 3) is a good check that coercion still runs for non-None values.
I read the code at the PR head, and git merge-tree against main is clean. I didn't run the tests.
Reviewed with help from Claude Code.
|
Thanks Muhammad Abdullah Butt (@1AbdullahButt), good catch. Fixed in 7236ac3. An explicit I used the metadata rather than |
Title: Python: skip type coercion for None arguments in KernelFunctionFromMethod
Motivation and Context
KernelFunctionFromMethod.gather_function_parameterspasses every supplied argument through_parse_parameter, which calls the parameter's type on the value. When an argument is present but explicitlyNone, this goes wrong for optional parameters:count: int | None = Nonegetsint(None), which raisesTypeError. The call fails withFunctionExecutionException: Parameter count is expected to be parsed to <class 'int'> but is not.label: str | None = Nonegetsstr(None), so the function receives the string"None"instead ofNone.item: MyDataclass | None = NonegetsMyDataclass(None)and fails.This happens when a caller passes
Nonedirectly. It also happens during auto function calling, where a model sendsnullfor an optional tool parameter ({"count": null}). Instead of running the tool withNone, the invocation fails.Minimal repro:
The .NET side has a similar report for null optional arguments (#14652).
Description
In
gather_function_parameters, aNonevalue now skips the type coercion step and is passed to the method unchanged. Non-Nonevalues are coerced exactly as before, so"3"still becomes3for anintparameter.Test added in
tests/unit/functions/test_kernel_function_from_method.py:test_explicit_none_for_optional_parameters_is_not_coercedinvokes a function with optionalint | None,Annotated[str | None, ...]and dataclass| Noneparameters set toNone, and checks that each one receivesNone. It also checks that string-to-int coercion still works for non-Nonevalues.Without the fix the test fails: the
intand dataclass parameters raise, and thestrparameter would receive"None".Tests:
tests/unit/functions/test_kernel_function_from_method.pypasses (73 tests). The newtest_explicit_none_for_optional_parameters_is_not_coercedfails onmainand passes with the change.ruff checkandmypyare clean on the changed file.Contribution Checklist
This change was prepared with the help of an AI assistant (Claude), and I reviewed and tested it.