Python: Fix bool parameter coercion so string "false" is not silently treated as True#14163
Python: Fix bool parameter coercion so string "false" is not silently treated as True#14163sohumt123 wants to merge 1 commit into
Conversation
…o True
When a kernel function declares a bool parameter and the incoming
argument is a string (e.g. an LLM tool call that quotes booleans, or
values coming through the template engine), _parse_parameter fell
through to the generic param_type(value) fallback. bool("false") is
True for any non-empty string, so enabled="false" silently ran the
function with enabled=True.
Parse bool-typed string arguments explicitly: "true"/"1" -> True,
"false"/"0" -> False (case-insensitive, whitespace-stripped), and
raise FunctionExecutionException for unparseable strings. Non-string
values keep the existing behavior. This matches the .NET SDK, which
parses "false" as false.
|
@sohumt123 please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
Pull request overview
Fixes Python kernel function argument coercion so string booleans (e.g., "false") are parsed to the correct bool value instead of falling back to Python’s bool("false") == True behavior, aligning with expected tool-call/template argument flows and cross-SDK behavior.
Changes:
- Add explicit string-to-bool parsing in
KernelFunctionFromMethod._parse_parameterfor"true"/"1"and"false"/"0"(case-insensitive, whitespace-trimmed), raising on invalid strings. - Add unit coverage for
_parse_parameter(..., bool)with common string/non-string inputs and invalid string handling. - Add an end-to-end
kernel.invoketest verifying"false"and"true"string arguments reach the function asFalse/True.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| python/semantic_kernel/functions/kernel_function_from_method.py | Adds explicit string-to-bool parsing branch to avoid incorrect coercion from non-empty strings. |
| python/tests/unit/functions/test_kernel_function_from_method.py | Adds unit + end-to-end tests to verify correct boolean coercion and invalid-string error behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| def test_parse_invalid_bool_string_raises_exception(get_custom_type_function_pydantic): | ||
| func = get_custom_type_function_pydantic | ||
| with pytest.raises(FunctionExecutionException, match=r"Cannot parse string 'yes' as bool."): |
|
@microsoft-github-policy-service agree |
Motivation and Context
When a kernel function declares a
boolparameter and the incomingKernelArgumentsvalue is a string — which is common when an LLM tool call quotes booleans as JSON strings, when arguments flow through the template engine (which passes strings), or when values originate from env/CLI/form input —KernelFunctionFromMethod._parse_parameterfalls through to the genericparam_type(value)fallback. Sincebool("false")isTruefor any non-empty string,enabled="false"silently runs the function withenabled=True. No error, no warning — the flag is simply inverted.Repro on current
main:This is also a cross-SDK inconsistency: the .NET SDK marshals the same case correctly (
"false"->falseviaTypeConverter/bool.Parsesemantics).Description
Adds an explicit branch in
_parse_parameterforparam_type is boolwith a string value, before the generic fallback:"true"/"1"->True,"false"/"0"->False(case-insensitive, whitespace-stripped)FunctionExecutionException(f"Cannot parse string '{value}' as bool.")instead of silently returningTruebool,int) and all other parameter types keep the existing behaviorTests added in
test_kernel_function_from_method.py:_parse_parametercoverage for"true"/"True"/"1"/"false"/"False"/"FALSE"/"0"/" false "plus non-stringTrue/False/1/0passthrough"yes") asserting the raisedFunctionExecutionExceptionkernel.invoketest showingenabled="false"now reaches the function asFalseAll of these fail on
mainwithout the fix (the end-to-end test getsenabled=True) and pass with it.uv run --frozen pytest tests/unit/functions tests/unit/kernelpasses (301 tests, including the 50 in the touched test file), andruff check,ruff format --check, andmypyare clean on the touched files.Contribution Checklist