-
Notifications
You must be signed in to change notification settings - Fork 1k
.NET: Executor source gen for workflow executor routing #3131
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
0ed461a to
46bb208
Compare
46bb208 to
97c7f78
Compare
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 introduces source generation for workflow executors, enabling automatic generation of route configuration code for classes decorated with [MessageHandler] attributes. The implementation includes a Roslyn incremental source generator, supporting attributes, comprehensive test infrastructure, and detailed diagnostic reporting.
Changes:
- Adds
Microsoft.Agents.AI.Workflows.Generatorsproject with incremental source generator - Introduces three new attributes:
MessageHandlerAttribute,SendsMessageAttribute, andYieldsMessageAttribute - Creates comprehensive unit test project with 20+ test cases covering various scenarios
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| Microsoft.Agents.AI.Workflows.Generators.csproj | New source generator project targeting netstandard2.0 |
| ExecutorRouteGenerator.cs | Main incremental generator implementation using ForAttributeWithMetadataName |
| SemanticAnalyzer.cs | Two-phase semantic analysis for method and class validation |
| SourceBuilder.cs | Code generation for ConfigureRoutes and protocol type methods |
| DiagnosticDescriptors.cs | Seven diagnostic descriptors for validation errors |
| Models/*.cs | Value-equatable models for incremental generator caching |
| MessageHandlerAttribute.cs | Attribute to mark handler methods for route generation |
| SendsMessageAttribute.cs | Class-level attribute declaring sent message types |
| YieldsMessageAttribute.cs | Class-level attribute declaring yielded message types |
| Microsoft.Agents.AI.Workflows.Generators.UnitTests.csproj | Test project configuration |
| GeneratorTestHelper.cs | Helper utilities for testing source generators |
| ExecutorRouteGeneratorTests.cs | 20+ comprehensive test cases covering all scenarios |
| Microsoft.Agents.AI.Workflows.csproj | Updated to reference generator as analyzer |
| agent-framework-dotnet.slnx | Solution file updated with new projects |
| Directory.Packages.props | Added Microsoft.CodeAnalysis.Analyzers package version |
| Directory.Build.targets | Build configuration for handling incompatible TFMs |
| SkipIncompatibleBuild.targets | Targets to skip builds with incompatible frameworks |
dotnet/src/Microsoft.Agents.AI.Workflows.Generators/ExecutorRouteGenerator.cs
Outdated
Show resolved
Hide resolved
3930d93 to
d675f46
Compare
…uteGenerator.cs Co-authored-by: Copilot <[email protected]>
| internal static class SemanticAnalyzer | ||
| { | ||
| // Fully-qualified type names used for symbol comparison | ||
| private const string ExecutorTypeName = "Microsoft.Agents.AI.Workflows.Executor"; |
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.
[Minor] We may want to use typeof(<T>).FullName here, rather than hardcoding the strings, to help in refactor renames (though a lot of the tools are decent about finding the names in comments / strings, too)
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.
I tried to do this, but we can't reasonably reference the workflows project from the source gen project.
| var methodSyntax = context.TargetNode as MethodDeclarationSyntax; | ||
|
|
||
| // Extract class-level info (raw facts, no validation here) | ||
| var classKey = GetClassKey(classSymbol); |
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.
When type name is not available on the RHS, consider using the type rather than var, as it makes it easier for readers without an IDE to consume the code.
| /// </summary> | ||
| public static readonly DiagnosticDescriptor MissingWorkflowContext = Register(new( | ||
| id: "MAFGENWF001", | ||
| title: "Handler missing IWorkflowContext parameter", |
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.
We don't need to localize these strings, right?
| /// <summary> | ||
| /// Creates a placeholder result for invalid targets (e.g., attribute on non-method). | ||
| /// </summary> | ||
| private static MethodAnalysisResult CreateEmptyResult() |
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.
Consider moving this to MethodAnalysisResult as a static Empty() method/property, similar to ImmutableEquatableArray<T>.Empty
| public void Initialize(IncrementalGeneratorInitializationContext context) | ||
| { | ||
| // Step 1: Use ForAttributeWithMetadataName to efficiently find methods with [MessageHandler] attribute. For each method found, build a MethodAnalysisResult. | ||
| var methodAnalysisResults = context.SyntaxProvider |
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.
Rooting the analysis on the methods annotated by [MessageHandler] is likely simpler than starting with extends(Executor), but it has the drawback that if we only want to use SourceGeneration for the Send/Yield types, and manually create the routes, we ignore that Executor class;
We should also do a bit of perf work here, after we settle on the API surface (attributes, what gets generated / not).
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.
Agreed that we should update this to handle Send/Yield types even with manual routing, but I don't think we'd want to root on extends(Executor). Rooting on an attribute with ForAttributeWithMetadataName claims to be at least 99x faster.
| sb.AppendLine($"{indent}partial class {info.ClassName}{info.GenericParameters}"); | ||
| sb.AppendLine($"{indent}{{"); | ||
|
|
||
| var memberIndent = indent + " "; |
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.
We may want to consider using the AnalyzerConfigOptionsProvider mechanism to get the indent style / spacecount from .editorconfig, so that the generated code matches the general project style.
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.
Interesting idea. By default, these generated "files" are not written to actual files and even in VS/Code you have to look in the package dependency to see the virtual file. I'm not sure I see the value in trying to match spacecount etc.
| { | ||
| var builder = ImmutableArray.CreateBuilder<string>(); | ||
|
|
||
| foreach (var attr in classSymbol.GetAttributes()) |
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.
Does this always get iterated in the same order?
Since we are creating immutable arrays that are presumably used as some form of comparator somewhere, do we want/need to ensure this is permutation invariant?
| AppendHandlerGenericArgs(sb, handler); | ||
| sb.Append($"(this.{handler.MethodName})"); | ||
|
|
||
| if (isLast) |
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.
If you pull this out of the loop you can avoid the conditional and just do sb.AppendLine(";");.
| generated.Should().Contain(".AddHandler<int>(this.HandleFromFile2)"); | ||
| } | ||
|
|
||
| #endregion |
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.
We should also test that having class-level Send/Yield annotations mixed between multiple parts of a partial class works correctly
| /// </summary> | ||
| /// <param name="type">The type of message that the executor may yield.</param> | ||
| /// <exception cref="ArgumentNullException"><paramref name="type"/> is <see langword="null"/>.</exception> | ||
| public YieldsMessageAttribute(Type type) |
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.
Consider renaming this to YieldsOutput to be consistent with the API on IWorkflowContext
This pull request introduces a new Roslyn source generator project for the Microsoft Agent Framework Workflows, enabling compile-time route configuration for executor classes using the
[MessageHandler]attribute. It includes the generator implementation, diagnostics, build configuration, and integration into the solution and dependency management.Follow-up PRs will update samples and docs to use this new pattern and obsolete the ReflectingExecutor
New Source Generator Project:
Microsoft.Agents.AI.Workflows.Generatorstargetingnetstandard2.0, with all necessary build and NuGet packaging settings, and Roslyn dependencies for source generator development. [1] [2]Source Generator Implementation:
ExecutorRouteGeneratorclass, a Roslyn incremental source generator that scans for[MessageHandler]methods and generates partial classes to configure workflow routes at compile time.SourceBuilderutility to generate the actual C# code for route configuration, including overrides forConfigureRoutes,ConfigureSentTypes, andConfigureYieldTypesas needed.Diagnostics and Analysis:
DiagnosticDescriptorsto define and register custom diagnostics for common usage errors (e.g., missing parameters, invalid return types, non-partial classes, etc.) in handler methods and executor classes.Solution and Dependency Integration:
agent-framework-dotnet.slnx). [1] [2]Directory.Packages.propsto includeMicrosoft.CodeAnalysis.Analyzersas a dependency for code analysis support.### Motivation and ContextContribution Checklist