Skip to content

Python: Preserve auto function call text#14019

Open
pragnyanramtha wants to merge 2 commits into
microsoft:mainfrom
pragnyanramtha:codex/python-preserve-auto-invoke-text
Open

Python: Preserve auto function call text#14019
pragnyanramtha wants to merge 2 commits into
microsoft:mainfrom
pragnyanramtha:codex/python-preserve-auto-invoke-text

Conversation

@pragnyanramtha
Copy link
Copy Markdown

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, ChatCompletionClientBase adds that assistant message to ChatHistory and 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

  • Track assistant messages that produced tool calls during the non-streaming auto-invoke loop.
  • Prepend any text items from those intermediate tool-call messages to the final returned response.
  • Preserve the existing return path when intermediate tool-call messages contain no text.
  • Add provider-independent regression coverage for ChatCompletionClientBase.

Validation

From python/:

  • uv run --python 3.12 pytest tests/unit/connectors/ai/test_chat_completion_client_base.py -q
  • uv run --python 3.12 pytest tests/unit/connectors/ai/open_ai/services/test_openai_chat_completion_base.py -q
  • uv run --python 3.12 ruff check semantic_kernel/connectors/ai/chat_completion_client_base.py tests/unit/connectors/ai/test_chat_completion_client_base.py
  • uv 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.py
  • git diff --check

Contribution Checklist

  • The code builds clean without any errors or warnings
  • The PR follows the SK Contribution Guidelines
  • The code follows the Python coding conventions
  • All unit tests pass, and I have added new tests where possible
  • I didn't break anyone 😄

@moonbox3 moonbox3 added the python Pull requests for the Python Semantic Kernel label May 16, 2026
@pragnyanramtha pragnyanramtha marked this pull request as ready for review May 16, 2026 23:38
@pragnyanramtha pragnyanramtha requested a review from a team as a code owner May 16, 2026 23:38
Copilot AI review requested due to automatic review settings May 16, 2026 23:38
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

@github-actions github-actions Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 else clause 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_responses where 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

Comment thread python/tests/unit/connectors/ai/test_chat_completion_client_base.py
Comment thread python/semantic_kernel/connectors/ai/chat_completion_client_base.py
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

python Pull requests for the Python Semantic Kernel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants