-
Notifications
You must be signed in to change notification settings - Fork 451
.NET: AIAgentHostExecutor to use ToAgentRunResponse #1439
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
base: main
Are you sure you want to change the base?
.NET: AIAgentHostExecutor to use ToAgentRunResponse #1439
Conversation
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 refactors the AIAgentHostExecutor
to use the ToAgentRunResponse
extension method for processing streaming agent updates. The change simplifies the complex manual aggregation logic that was previously used to assemble streaming updates into complete messages.
- Replaces manual message aggregation with a standardized extension method approach
- Simplifies streaming update processing by collecting all updates before transforming them
- Updates corresponding unit tests to reflect the simplified message handling structure
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
File | Description |
---|---|
AIAgentHostExecutor.cs | Refactored streaming message processing to use ToAgentRunResponse extension method, removing complex manual aggregation logic |
SpecializedExecutorSmokeTests.cs | Updated test structure to match simplified message handling, removing detailed content splitting verification |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
||
currentStreamingMessage = null; | ||
} | ||
await context.SendMessageAsync(updates.ToAgentRunResponse().Messages, cancellationToken: cancellationToken).ConfigureAwait(false); |
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.
Note that the new M.E.AI build that should be out tomorrow updates the logic used by this to also factor in AuthorName for message boundary detection, which should help in the case where message ID isn't set but AuthorName is (and AuthorName is always set when using ChatClientAgent).
This changes the logic of the resulting joined messages - they lose the initial splits as sent out by the agent in individual updates. We were keeping them around deliberately: I recall that fixing a bug that @crickman had with Declarative Workflows at one point (though the details elude me now). Are we sure this behaviour change will not cause problems? |
Can you elaborate on this? |
Looks like I had outdated info - it used to be that when the AgentRunResponses were merged using the existing method in the AgentFramework they would merge the multiple updates into a single ChatMessage. But it looks like now that is no longer the logic going on. |
Motivation and Context
Addressed #1137
Description
Use the
ToAgentRunResponse
extension method to assemble streaming updates into fully-formed updates. This will avoid unintended bugs caused by differences between underlying clients/services driving the agents.Contribution Checklist