- Notifications
You must be signed in to change notification settings - Fork845
Update ToChatResponse{Async} to also factor in AuthorName#6910
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
Originally we were only factoring in Role. Then we also augmented it to factor in MessageId, since updates from different messages are by definition not part of the same message. Now it also makes sense to factor in AuthorName, as different text from different speakers / agents are also by definition not part of the same message.
stephentoub commentedOct 10, 2025
cc:@westey-m |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Pull Request Overview
This PR updates theToChatResponse andToChatResponseAsync methods to includeAuthorName as an additional boundary condition for determining when to create new chat messages from streaming updates. Previously, the methods only consideredRole andMessageId changes as triggers for new message boundaries. Now, changes inAuthorName will also dictate message boundaries, ensuring that text from different speakers/agents are properly separated into distinct messages.
Key changes:
- Enhanced boundary detection logic to include
AuthorNamecomparison alongside existingRoleandMessageIdchecks - Comprehensive test coverage for various
AuthorNameboundary scenarios - Updated existing tests to reflect the new behavior
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 9 comments.
| File | Description |
|---|---|
src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/ChatResponseExtensions.cs | Refactored message boundary detection logic to includeAuthorName comparison and added helper methods for clean boundary checking |
test/Libraries/Microsoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs | Added extensive test coverage forAuthorName boundary scenarios and updated existing tests to accommodate the new behavior |
src/Libraries/Microsoft.Extensions.AI.Abstractions/ChatCompletion/ChatResponseExtensions.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.cs OutdatedShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
...crosoft.Extensions.AI.Abstractions.Tests/ChatCompletion/ChatResponseUpdateExtensionsTests.csShow resolvedHide resolved
Uh oh!
There was an error while loading.Please reload this page.
3d0419c intodotnet:mainUh oh!
There was an error while loading.Please reload this page.
* Update ToChatResponse{Async} to also factor in AuthorNameOriginally we were only factoring in Role. Then we also augmented it to factor in MessageId, since updates from different messages are by definition not part of the same message. Now it also makes sense to factor in AuthorName, as different text from different speakers / agents are also by definition not part of the same message.* Fix useAsync inversion in tests* Update ToChatResponse{Async} to also factor in AuthorNameOriginally we were only factoring in Role. Then we also augmented it to factor in MessageId, since updates from different messages are by definition not part of the same message. Now it also makes sense to factor in AuthorName, as different text from different speakers / agents are also by definition not part of the same message.* Fix useAsync inversion in tests* Update ToChatResponse{Async} to also factor in AuthorNameOriginally we were only factoring in Role. Then we also augmented it to factor in MessageId, since updates from different messages are by definition not part of the same message. Now it also makes sense to factor in AuthorName, as different text from different speakers / agents are also by definition not part of the same message.* Fix useAsync inversion in tests
Uh oh!
There was an error while loading.Please reload this page.
Originally we were only factoring in Role. Then we also augmented it to factor in MessageId, since updates from different messages are by definition not part of the same message. Now it also makes sense to factor in AuthorName, as different text from different speakers / agents are also by definition not part of the same message.
Microsoft Reviewers:Open in CodeFlow