Python: Preserve auto function call text#14019
Conversation
There was a problem hiding this comment.
Automated Code Review
Reviewers: 4 | Confidence: 92%
✓ Correctness
The PR correctly preserves intermediate assistant text during non-streaming auto function invocation. The implementation tracks function-call messages, extracts their text items, and prepends them to the final response using deep copies to avoid mutation. Both normal-exit and max-retries paths are covered. The early-termination path (result.terminate) is intentionally excluded since it returns function results, not model text. The test coverage is well-structured and validates both the text-present and text-absent intermediate message cases.
✓ Security Reliability
Clean, well-scoped change that preserves intermediate assistant text during non-streaming auto function invocation. The new _combine_auto_invoke_text_responses method follows the exact pattern established by ChatMessageContent.content setter (lines 207-229 of chat_message_content.py). Uses copy.deepcopy to avoid mutating original completions, which is safe for Pydantic v2 models. The terminate early-exit path (line 170) intentionally does not include the new combination logic since it returns merged function results rather than model completions. No injection risks, resource leaks, or unhandled failure modes identified.
✓ Test Coverage
The two new tests cover the primary happy path (intermediate text is prepended) and the no-op path (no intermediate text). However, three code paths modified by this PR lack test coverage: (1) the max-auto-invoke-attempts-exhausted branch (the
elseclause at diff line 174–175), (2) accumulation of text across multiple intermediate tool-call rounds, and (3) the insertion branch in_combine_auto_invoke_text_responseswhere the final completion has no TextContent item.
✓ Design Approach
The new helper fixes the normal auto-invoke exit paths, but it still misses the supported early-termination path. When an auto-function-invocation filter sets
context.terminate = True, non-streaming calls still return merged tool results only, so any assistant text that accompanied the tool call is still dropped.
Automated review by pragnyanramtha's agents
Motivation and Context
Addresses the Python response-text portion of #13420.
When non-streaming auto function invocation receives an assistant response that contains both user-visible text and tool calls,
ChatCompletionClientBaseadds that assistant message toChatHistoryand continues the tool loop. The final return value only contains the last model response, so text such as "I'll check that..." is visible in history but is not returned to the caller. Streaming already yields those chunks as they arrive.This PR does not change token usage metadata aggregation.
Description
ChatCompletionClientBase.Validation
From
python/:uv run --python 3.12 pytest tests/unit/connectors/ai/test_chat_completion_client_base.py -quv run --python 3.12 pytest tests/unit/connectors/ai/open_ai/services/test_openai_chat_completion_base.py -quv run --python 3.12 ruff check semantic_kernel/connectors/ai/chat_completion_client_base.py tests/unit/connectors/ai/test_chat_completion_client_base.pyuv run --python 3.12 ruff format --check semantic_kernel/connectors/ai/chat_completion_client_base.py tests/unit/connectors/ai/test_chat_completion_client_base.pygit diff --checkContribution Checklist