Skip to content

.Net Fix - Include role definition for each StreamingChatMessageContent - #6957

Merged
Chris (crickman) merged 5 commits into
mainfrom
fix_streaming_message_role
Jun 25, 2024
Merged

.Net Fix - Include role definition for each StreamingChatMessageContent#6957
Chris (crickman) merged 5 commits into
mainfrom
fix_streaming_message_role

Conversation

@crickman

@crickman Chris (crickman) commented Jun 25, 2024

Copy link
Copy Markdown
Contributor

Motivation and Context

StreamingChatMessageContent.AuthorRole always null

Description

Logic already exists to capture role, but it was not being passed to each and every instance of StreamingChatMessageContent. Other properties, such as ModelId are included with each and every instance.

Updated integration test.

Issue: #6952

Contribution Checklist

@crickman Chris (crickman) added bug Something isn't working .NET Issue or Pull requests regarding .NET code ai connector Anything related to AI connectors labels Jun 25, 2024
@crickman Chris (crickman) self-assigned this Jun 25, 2024
@crickman
Chris (crickman) requested a review from a team as a code owner June 25, 2024 18:05
@markwallace-microsoft Mark Wallace (markwallace-microsoft) added the kernel Issues or pull requests impacting the core kernel label Jun 25, 2024
@crickman Chris (crickman) changed the title .Net Fix - Include role definition for StreamingChatMessageContent .Net Fix - Include role definition for each StreamingChatMessageContent Jun 25, 2024
Comment thread dotnet/src/Connectors/Connectors.OpenAI/AzureSdk/ClientCore.cs
@crickman
Chris (crickman) added this pull request to the merge queue Jun 25, 2024
Merged via the queue into main with commit 8f0a402 Jun 25, 2024
@crickman
Chris (crickman) deleted the fix_streaming_message_role branch June 25, 2024 22:06
LudoCorporateShark pushed a commit to LudoCorporateShark/semantic-kernel that referenced this pull request Aug 25, 2024
…nt (microsoft#6957)

### Motivation and Context
<!-- Thank you for your contribution to the semantic-kernel repo!
Please help reviewers and future users, providing the following
information:
  1. Why is this change required?
  2. What problem does it solve?
  3. What scenario does it contribute to?
  4. If it fixes an open issue, please link to the issue here.
-->

`StreamingChatMessageContent.AuthorRole` always `null`

### Description
<!-- Describe your changes, the overall approach, the underlying design.
These notes will help understanding how your code works. Thanks! -->

Logic already exists to capture role, but it was not being passed to
each and every instance of StreamingChatMessageContent. Other
properties, such as `ModelId` are included with each and every instance.

Updated integration test.

Issue: microsoft#6952

### Contribution Checklist

<!-- Before submitting this PR, please make sure: -->

- [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 😄
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 bug Something isn't working kernel Issues or pull requests impacting the core kernel .NET Issue or Pull requests regarding .NET code

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

.Net Bug: IChatCompletionService produces StreamingChatMessageContent without role assignment

4 participants