-
Notifications
You must be signed in to change notification settings - Fork 1k
.NET: Joslat fix sample issue #3270
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?
Conversation
…d mermaid, adds rendering of labels in edges
…wVisualizer.cs Co-authored-by: Copilot <[email protected]>
…acters that would break the diagram and enabling the previous signature so the API has backwards compatibility.
…gent-framework-official into joslat-fix-sample-issue
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 fixes issue #3269 in the MixedWorkflowAgentsAndExecutors sample by correcting a type mismatch between executors and the AIAgentHostExecutor. The AIAgentHostExecutor sends messages as List<ChatMessage>, but the downstream executors were expecting a single ChatMessage, causing runtime routing failures.
Changes:
- Fixed type signatures in
JailbreakSyncExecutorandFinalOutputExecutorto acceptList<ChatMessage>instead ofChatMessage - Added XML remarks documenting the type matching requirement
- Changed
ShowAgentThinkingfromfalsetotruefor better debugging visibility - Modified deployment name configuration to use a hardcoded constant
Comments suppressed due to low confidence (2)
dotnet/samples/GettingStarted/Workflows/_Foundational/07_MixedWorkflowAgentsAndExecutors/Program.cs:45
- The deployment name should be read from environment variables as per the sample code guidelines. All other samples in this repository use the pattern:
var deploymentName = Environment.GetEnvironmentVariable("AZURE_OPENAI_DEPLOYMENT_NAME") ?? "gpt-4o-mini";This ensures consistency across samples and allows users to configure their deployment name without modifying code. The hardcoded constant with a commented-out environment variable read deviates from established patterns.
var deploymentName = Environment.GetEnvironmentVariable("AZURE_OPENAI_DEPLOYMENT_NAME") ?? "gpt-4o-mini";
dotnet/samples/GettingStarted/Workflows/_Foundational/07_MixedWorkflowAgentsAndExecutors/Program.cs:46
- According to the sample code guidelines, "Prefer defining variables using types rather than var, to help users understand the types involved." The variable should be declared as
IChatClient chatClientinstead of usingvarto make the type explicit for users learning from this sample.
var chatClient = new AzureOpenAIClient(new Uri(endpoint), new AzureCliCredential()).GetChatClient(deploymentName).AsIChatClient();
Root Cause AnalysisBreaking Change Assessment: AIAgentHostExecutor Message OutputSummaryThe Root CauseCommit Details
What ChangedBefore (old behavior): // Individual ChatMessage objects were sent one by one as they were built from streaming updates
ChatMessage? currentStreamingMessage = null;
// ... streaming logic ...
await context.SendMessageAsync(currentStreamingMessage, cancellationToken: cancellationToken).ConfigureAwait(false);After (new behavior): // The entire Messages collection is sent at once
await context.SendMessageAsync(updates.ToAgentRunResponse().Messages, cancellationToken: cancellationToken).ConfigureAwait(false);
// or in non-streaming mode:
await context.SendMessageAsync(response.Messages, cancellationToken: cancellationToken).ConfigureAwait(false);Technical DetailsMessage Router BehaviorThe // From MessageRouter.RouteMessageAsync
if (this._typedHandlers.TryGetValue(message.GetType(), out MessageHandlerF? handler))
{
result = await handler(message, context, cancellationToken).ConfigureAwait(false);
}This means:
Type Mismatch
Impact on Sample CodeOriginal Code (broken)internal sealed class JailbreakSyncExecutor() : Executor<ChatMessage>("JailbreakSync")
{
public override async ValueTask HandleAsync(ChatMessage message, ...)Fixed Code (working)internal sealed class JailbreakSyncExecutor() : Executor<List<ChatMessage>>("JailbreakSync")
{
public override async ValueTask HandleAsync(List<ChatMessage> message, ...)Affected PatternsAny executor that:
Must now be changed to accept RecommendationWhen connecting a custom executor after an AI agent in a workflow, use:
Related Files Changed
Workaround Alternatives
|
Motivation and Context
Please help reviewers and future users, providing the following information:
Fixes issue .NET: Issue with Foundational Sample #3269
.NET: Issue with Foundational Sample #3269
.NET: Issue with Foundational Sample #3269
Description
Just some changes and adjustments These notes will help understanding how your code works. Thanks! -->
Contribution Checklist