Python: Handle empty chat message function results - #14375
Python: Handle empty chat message function results#14375Alperen (lprnmns) wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
The change is small, targeted to the reported IndexError, and includes a focused regression test covering the boundary case.
Pull request overview
This PR fixes a Python edge case where FunctionResultContent.from_function_call_content_and_result() could raise IndexError when a tool returned a valid but empty ChatMessageContent, by avoiding unsafe items[0] access and adding a regression test to lock in the behavior.
Changes:
- Remove
ChatMessageContent.items[0]inspection infrom_function_call_content_and_result()to preventIndexErroron empty messages. - Add a focused unit test asserting empty
ChatMessageContentresults become an empty function result while preservinginner_content.
File summaries
| File | Description |
|---|---|
| python/semantic_kernel/contents/function_result_content.py | Removes unsafe items[0] access in the ChatMessageContent conversion path to avoid IndexError for empty tool messages. |
| python/tests/unit/contents/test_function_result_content.py | Adds a regression test covering empty ChatMessageContent results and inner_content retention. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| elif isinstance(result, ChatMessageContent): | ||
| if isinstance(result.items[0], TextContent): | ||
| res = result.items[0].text | ||
| elif isinstance(result.items[0], ImageContent): | ||
| res = result.items[0].data_uri | ||
| elif isinstance(result.items[0], FunctionResultContent): | ||
| res = result.items[0].result | ||
| res = str(result) | ||
| else: |
There was a problem hiding this comment.
MAF Automated Review — Iteration 1
Result: No findings
Scope: full PR (1 commit(s)): dd0092796566
Model: claude-opus-4.8
Overview
The PR removes an item-inspection block from the ChatMessageContent branch of
FunctionResultContent.from_function_call_content_and_result() and lets res = str(result) stand alone, fixing an IndexError raised when a tool returns a
ChatMessageContent with an empty items list. The removed assignments were
provably dead code — each res = result.items[0]... was unconditionally
overwritten by the following res = str(result) line — so every non-empty input
produces byte-identical output before and after the change. The only observable
behavioral delta is that an empty message now yields an empty-string result
instead of crashing, which is confirmed by the added regression test and does not
cross or weaken any trust boundary. No public API, serialization, or provider
parity regression was found.
Reviewed the supplied pull-request change set across correctness, security/reliability, architecture, and failure behavior.
No publishable findings remained after source verification for this scope.
|
@microsoft-github-policy-service agree |
Motivation and Context
FunctionResultContent.from_function_call_content_and_result()raisesIndexErrorwhen a tool returns a valid emptyChatMessageContent, because it readsresult.items[0]before converting the message to text.This is distinct from #14359 / #14364, which handle an empty string passed through
ChatHistory.add_tool_message(); that change does not affect this factory path.Description
Remove the item inspection from the
ChatMessageContentbranch. Those extracted values were immediately overwritten bystr(result), so non-empty behavior is unchanged while an empty message now becomes an empty function result. A focused regression covers the empty result and retainedinner_content.Validation
uv run pytest -q tests/unit/contents/test_function_result_content.py --maxfail=1uv run pytest -q tests/unit/contents/test_chat_history.py tests/unit/kernel/test_kernel.py --maxfail=1uv run pytest -q tests/unit/contents --maxfail=1uv run pre-commit run --files semantic_kernel/contents/function_result_content.py tests/unit/contents/test_function_result_content.pygit diff --checkThe complete Python unit suite was attempted but could not collect because the optional
autogenextra is not installed; all relevant and package-level contents tests passed.This is backward compatible and changes no public API.