- 
                Notifications
    You must be signed in to change notification settings 
- Fork 639
          .NET: IHostedAgentBuilder overloads for workflows; simplify workflow extensions
          #1731
        
          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
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 simplifies workflow extension methods and adds overloads for hosting extensions to accept IHostedAgentBuilder parameters. The changes eliminate the AddConcurrentWorkflow and AddSequentialWorkflow convenience methods in favor of explicit workflow configuration, while enabling callers to pass agent builders directly to mapping methods instead of repeating agent names.
Key changes:
- Removed AddConcurrentWorkflowandAddSequentialWorkflowextension methods
- Added IHostedAgentBuilderoverloads toMapA2A,MapOpenAIChatCompletions, andMapOpenAIResponses
- Updated sample code to use explicit workflow configuration and new builder-based mapping methods
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description | 
|---|---|
| HostApplicationBuilderWorkflowExtensionsTests.cs | Removed unit tests for deleted AddConcurrentWorkflowandAddSequentialWorkflowmethods | 
| HostApplicationBuilderWorkflowExtensions.cs | Deleted AddConcurrentWorkflowandAddSequentialWorkflowmethods and removed unusedSystem.Collections.Genericimport | 
| EndpointRouteBuilderExtensions.Responses.cs | Added IHostedAgentBuilderoverloads forMapOpenAIResponsesmethod | 
| EndpointRouteBuilderExtensions.ChatCompletions.cs | Added IHostedAgentBuilderoverload forMapOpenAIChatCompletionsand changed return type toIEndpointConventionBuilder | 
| EndpointRouteBuilderExtensions.cs | Added four new MapA2Aoverloads acceptingIHostedAgentBuilder | 
| Program.cs | Updated sample to use explicit workflow configuration and new builder-based mapping methods | 
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.
Looks ok to me, though for the builder stuff I defer to @ReubenBond
| builder.AddConcurrentWorkflow("science-concurrent-workflow", [chemistryAgent, mathsAgent, literatureAgent]).AddAsAIAgent(); | ||
| var scienceSequentialWorkflow = builder.AddWorkflow("science-sequential-workflow", (sp, key) => | ||
| { | ||
| List<IHostedAgentBuilder> usedAgents = [chemistryAgent, mathsAgent, literatureAgent]; | 
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.
FWIW, it's still counterintuitive to me that something called "Agent" and that's returned from a method called "AddAIAgent" is actually a builder. I understand why it is, but maybe there are things we could do to ease this.
2 intentions here:
AddConcurrentWorkflowandAddSequentialWorkflowas per .NET: Add DevUI package for .NET #1603 (comment)IHostedAgentBuilder- this avoids need of copying the name of agent in registration+Map... methods.