Skip to content

Commit 32c3469

Browse files
authored
Python: Fix default result_parser arity on kernel function strategies (#14595)
### Motivation and Context `KernelFunctionSelectionStrategy` and `KernelFunctionTerminationStrategy` declare optional `result_parser` fields whose defaults are zero-argument lambdas, but `select_agent` / `should_agent_terminate` always call `self.result_parser(result)` with one argument. Using either strategy without a custom parser therefore always raises `TypeError`. Fixes #14589 ### Description Update the defaults so they accept the function result: - **Selection:** `lambda result: str(result).strip()` — use the function output as the agent name (aligned with the .NET default that reads a string from the result). - **Termination:** `lambda result: str(result).strip().lower() == "true"` — treat a case-insensitive `"true"` as terminate. For termination, .NET’s default is `(_) => true` (always terminate). I followed the issue’s suggestion instead, because an always-true default would ignore the kernel function output and stop the chat on the first check. Happy to switch to `lambda _: True` for .NET parity if maintainers prefer that. Also adds regression tests that construct both strategies without a custom `result_parser`. ### Contribution Checklist - [x] The code builds clean without any errors or warnings - [x] The PR follows the [SK Contribution Guidelines](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md) and the [pre-submission formatting script](https://github.com/microsoft/semantic-kernel/blob/main/CONTRIBUTING.md#development-scripts) raises no violations - [x] All unit tests pass, and I have added new tests where possible - [x] I didn't break anyone 😄 Co-authored-by: Li-john1021 <li-john1021@users.noreply.github.com>
1 parent bce5801 commit 32c3469

4 files changed

Lines changed: 59 additions & 2 deletions

File tree

‎python/semantic_kernel/agents/strategies/selection/kernel_function_selection_strategy.py‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@ class KernelFunctionSelectionStrategy(SelectionStrategy):
4141
arguments: KernelArguments | None = None
4242
function: KernelFunction
4343
kernel: Kernel
44-
result_parser: Callable[..., str] = Field(default_factory=lambda: (lambda: ""))
44+
result_parser: Callable[..., str] = Field(default_factory=lambda: (lambda result: str(result).strip()))
4545
history_reducer: ChatHistoryReducer | None = None
4646

4747
@override

‎python/semantic_kernel/agents/strategies/termination/kernel_function_termination_strategy.py‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,9 @@ class KernelFunctionTerminationStrategy(TerminationStrategy):
3333
arguments: KernelArguments | None = None
3434
function: KernelFunction
3535
kernel: Kernel
36-
result_parser: Callable[..., bool] = Field(default_factory=lambda: (lambda: True))
36+
result_parser: Callable[..., bool] = Field(
37+
default_factory=lambda: (lambda result: str(result).strip().lower() == "true")
38+
)
3739
history_reducer: ChatHistoryReducer | None = None
3840

3941
async def should_agent_terminate(

‎python/tests/unit/agents/test_group_chat_strategies/test_kernel_function_selection_strategy.py‎

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -104,3 +104,20 @@ async def async_result_parser(result):
104104

105105
assert selected_agent.name == "Agent_2"
106106
mock_function.invoke.assert_awaited_once()
107+
108+
109+
async def test_kernel_function_selection_default_result_parser(agents):
110+
"""Default result_parser must accept the function result (regression for #14589)."""
111+
history = [MagicMock(spec=ChatMessageContent)]
112+
mock_function = AsyncMock(spec=KernelFunction)
113+
mock_result = MagicMock()
114+
mock_result.__str__.return_value = "Agent_1"
115+
mock_function.invoke.return_value = mock_result
116+
mock_kernel = MagicMock(spec=Kernel)
117+
118+
strategy = KernelFunctionSelectionStrategy(function=mock_function, kernel=mock_kernel)
119+
120+
selected_agent = await strategy.next(agents, history)
121+
122+
assert selected_agent.name == "Agent_1"
123+
mock_function.invoke.assert_awaited_once()

‎python/tests/unit/agents/test_group_chat_strategies/test_kernel_function_termination_strategy.py‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,3 +112,41 @@ async def mock_result_parser(result):
112112

113113
assert result is True
114114
mock_function.invoke.assert_awaited_once()
115+
116+
117+
async def test_should_agent_terminate_default_result_parser_true():
118+
"""Default result_parser must accept the function result (regression for #14589)."""
119+
agent = MockAgent(id="test-agent-id")
120+
history = [MagicMock(spec=ChatMessageContent)]
121+
122+
mock_function = AsyncMock(spec=KernelFunction)
123+
mock_result = MagicMock()
124+
mock_result.__str__.return_value = "true"
125+
mock_function.invoke.return_value = mock_result
126+
mock_kernel = MagicMock(spec=Kernel)
127+
128+
strategy = KernelFunctionTerminationStrategy(agents=[agent], function=mock_function, kernel=mock_kernel)
129+
130+
result = await strategy.should_agent_terminate(agent, history)
131+
132+
assert result is True
133+
mock_function.invoke.assert_awaited_once()
134+
135+
136+
async def test_should_agent_terminate_default_result_parser_false():
137+
"""Default result_parser treats non-true strings as continue."""
138+
agent = MockAgent(id="test-agent-id")
139+
history = [MagicMock(spec=ChatMessageContent)]
140+
141+
mock_function = AsyncMock(spec=KernelFunction)
142+
mock_result = MagicMock()
143+
mock_result.__str__.return_value = "no"
144+
mock_function.invoke.return_value = mock_result
145+
mock_kernel = MagicMock(spec=Kernel)
146+
147+
strategy = KernelFunctionTerminationStrategy(agents=[agent], function=mock_function, kernel=mock_kernel)
148+
149+
result = await strategy.should_agent_terminate(agent, history)
150+
151+
assert result is False
152+
mock_function.invoke.assert_awaited_once()

0 commit comments

Comments
 (0)