Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Python: Yield FunctionResultContent in streaming chat completion path. Update tests. #9974

Merged
merged 13 commits into from
Dec 18, 2024

Conversation

moonbox3
Copy link
Contributor

Motivation and Context

Currently, if using SK's Python streaming chat completion path, we only yield the following content types: StreamingChatMessageContent and FunctionCallContent. We are not yielding FunctionResultContent which is valuable for some use cases.

Description

This PR updates the code to yield FunctionResultContent if it exists in the streaming chat completion path. When we merge the function call result content together into a StreamingChatMessageContent type, we check if that message has items (which are of type FunctionResultContent) and if so, we yield them. The filter path still works because once they're yielded, we break out of the function calling loop.

We need to include the ai_model_id if it exists for the current PromptExecutionSettings because if performing a reduce operation to add two streaming chat message chunks together, the StreamingChatMessageContent that has the function results will break if the ai_model_id is not set (the error is thrown in the __add__ override for the StreamingChatMessageContent.

Some unit tests that cover function calling were also updated -- during the test, the test JSON function args were breaking in the json.loads call because they contained single quotes and not double. We're now sanitizing the args, just in case, so we don't break there.

This PR fixes:

Contribution Checklist

@moonbox3 moonbox3 added the ai connector Anything related to AI connectors label Dec 14, 2024
@moonbox3 moonbox3 self-assigned this Dec 14, 2024
@moonbox3 moonbox3 requested a review from a team as a code owner December 14, 2024 09:11
@markwallace-microsoft markwallace-microsoft added the python Pull requests for the Python Semantic Kernel label Dec 14, 2024
@markwallace-microsoft
Copy link
Member

markwallace-microsoft commented Dec 16, 2024

Python Test Coverage

Python Test Coverage Report •
FileStmtsMissCoverMissing
semantic_kernel
   kernel.py2024777%143, 154, 158, 308–311, 423, 437–480
semantic_kernel/connectors/ai
   chat_completion_client_base.py127298%401, 411
semantic_kernel/contents
   function_call_content.py101298%186, 214
semantic_kernel/functions
   kernel_function_from_prompt.py154795%167–168, 182, 203, 221, 241, 324
TOTAL16775184989% 

Python Unit Test Overview

Tests Skipped Failures Errors Time
2966 4 💤 0 ❌ 0 🔥 1m 17s ⏱️

@moonbox3 moonbox3 added this pull request to the merge queue Dec 18, 2024
Merged via the queue into microsoft:main with commit 16690ed Dec 18, 2024
25 checks passed
@moonbox3 moonbox3 deleted the yield-func-result-content branch December 18, 2024 01:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ai connector Anything related to AI connectors python Pull requests for the Python Semantic Kernel
Projects
Status: Sprint: Done
Development

Successfully merging this pull request may close these issues.

4 participants