diff --git a/dotnet/samples/GettingStarted/Workflows/Concurrent/Concurrent/Program.cs b/dotnet/samples/GettingStarted/Workflows/Concurrent/Concurrent/Program.cs index c839149d6c..e5373554c3 100644 --- a/dotnet/samples/GettingStarted/Workflows/Concurrent/Concurrent/Program.cs +++ b/dotnet/samples/GettingStarted/Workflows/Concurrent/Concurrent/Program.cs @@ -72,8 +72,8 @@ private static async Task Main() /// /// Executor that starts the concurrent processing by sending messages to the agents. /// -internal sealed class ConcurrentStartExecutor() : - Executor("ConcurrentStartExecutor") +internal sealed partial class ConcurrentStartExecutor() : + Executor("ConcurrentStartExecutor") { /// /// Starts the concurrent processing by sending messages to the agents. @@ -83,7 +83,8 @@ internal sealed class ConcurrentStartExecutor() : /// The to monitor for cancellation requests. /// The default is . /// A task representing the asynchronous operation - public override async ValueTask HandleAsync(string message, IWorkflowContext context, CancellationToken cancellationToken = default) + [MessageHandler] + public async ValueTask HandleAsync(string message, IWorkflowContext context, CancellationToken cancellationToken = default) { // Broadcast the message to all connected agents. Receiving agents will queue // the message but will not start processing until they receive a turn token. diff --git a/dotnet/samples/GettingStarted/Workflows/Directory.Build.props b/dotnet/samples/GettingStarted/Workflows/Directory.Build.props new file mode 100644 index 0000000000..8ad5839332 --- /dev/null +++ b/dotnet/samples/GettingStarted/Workflows/Directory.Build.props @@ -0,0 +1,13 @@ + + + + + + + + + + diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Models/EquatableArray.cs b/dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Models/EquatableArray.cs new file mode 100644 index 0000000000..91720ac809 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Models/EquatableArray.cs @@ -0,0 +1,121 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using System.Collections; +using System.Collections.Generic; +using System.Collections.Immutable; + +namespace Microsoft.Agents.AI.Workflows.Generators.Models; + +/// +/// A wrapper around that provides value-based equality. +/// This is necessary for incremental generator caching since ImmutableArray uses reference equality. +/// +/// +/// Creates a new from an . +/// +internal readonly struct EquatableArray(ImmutableArray array) : IEquatable>, IEnumerable + where T : IEquatable +{ + private readonly ImmutableArray _array = array.IsDefault ? ImmutableArray.Empty : array; + + /// + /// Gets the underlying array. + /// + public ImmutableArray AsImmutableArray() => this._array; + + /// + /// Gets the number of elements in the array. + /// + public int Length => this._array.Length; + + /// + /// Gets the element at the specified index. + /// + public T this[int index] => this._array[index]; + + /// + /// Gets whether the array is empty. + /// + public bool IsEmpty => this._array.IsEmpty; + + /// + public bool Equals(EquatableArray other) + { + if (this._array.Length != other._array.Length) + { + return false; + } + + for (int i = 0; i < this._array.Length; i++) + { + if (!this._array[i].Equals(other._array[i])) + { + return false; + } + } + + return true; + } + + /// + public override bool Equals(object? obj) + { + return obj is EquatableArray other && this.Equals(other); + } + + /// + public override int GetHashCode() + { + if (this._array.IsEmpty) + { + return 0; + } + + var hashCode = 17; + foreach (var item in this._array) + { + hashCode = hashCode * 31 + (item?.GetHashCode() ?? 0); + } + + return hashCode; + } + + /// + public IEnumerator GetEnumerator() + { + return ((IEnumerable)this._array).GetEnumerator(); + } + + /// + IEnumerator IEnumerable.GetEnumerator() + { + return this.GetEnumerator(); + } + + /// + /// Equality operator. + /// + public static bool operator ==(EquatableArray left, EquatableArray right) + { + return left.Equals(right); + } + + /// + /// Inequality operator. + /// + public static bool operator !=(EquatableArray left, EquatableArray right) + { + return !left.Equals(right); + } + + /// + /// Creates an empty . + /// + public static EquatableArray Empty => new(ImmutableArray.Empty); + + /// + /// Implicit conversion from . + /// + public static implicit operator EquatableArray(ImmutableArray array) => new(array); +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs new file mode 100644 index 0000000000..82ca9106b7 --- /dev/null +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs @@ -0,0 +1,49 @@ +// Copyright (c) Microsoft. All rights reserved. + +using System; +using Microsoft.Shared.Diagnostics; + +namespace Microsoft.Agents.AI.Workflows; + +/// +/// Declares that an executor may yield messages of the specified type as workflow outputs. +/// +/// +/// +/// Apply this attribute to an class to declare the types of messages +/// it may yield via . This information is used +/// for protocol validation and documentation. +/// +/// +/// This attribute can be applied multiple times to declare multiple output types. +/// It is inherited by derived classes, allowing base executors to declare common output types. +/// +/// +/// +/// +/// [YieldsMessage(typeof(FinalResult))] +/// [YieldsMessage(typeof(StreamChunk))] +/// public partial class MyExecutor : Executor +/// { +/// // ... +/// } +/// +/// +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] +public sealed class YieldsMessageAttribute : Attribute +{ + /// + /// Gets the type of message that the executor may yield. + /// + public Type Type { get; } + + /// + /// Initializes a new instance of the class. + /// + /// The type of message that the executor may yield. + /// is . + public YieldsMessageAttribute(Type type) + { + this.Type = Throw.IfNull(type); + } +} diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Executor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Executor.cs index b85426f438..ba9cbac4e1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Executor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Executor.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Internal use of obsolete types for backward compatibility + using System; using System.Collections.Generic; using System.Diagnostics; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs index 3b18379907..fe1a777859 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Threading; using System.Threading.Tasks; @@ -9,6 +10,12 @@ namespace Microsoft.Agents.AI.Workflows.Reflection; /// A message handler interface for handling messages of type . /// /// +/// +/// This interface is obsolete. Use the on methods in a partial class +/// deriving from instead. +/// +[Obsolete("Use [MessageHandler] attribute on methods in a partial class deriving from Executor. " + + "This interface will be removed in a future version.")] public interface IMessageHandler { /// @@ -28,6 +35,12 @@ public interface IMessageHandler /// /// The type of message to handle. /// The type of result returned after handling the message. +/// +/// This interface is obsolete. Use the on methods in a partial class +/// deriving from instead. +/// +[Obsolete("Use [MessageHandler] attribute on methods in a partial class deriving from Executor. " + + "This interface will be removed in a future version.")] public interface IMessageHandler { /// diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/MessageHandlerInfo.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/MessageHandlerInfo.cs index f63a43b4a8..f655c27cd4 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/MessageHandlerInfo.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/MessageHandlerInfo.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Internal use of obsolete types for backward compatibility + using System; using System.Diagnostics; using System.Diagnostics.CodeAnalysis; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs index d96f9319f4..f4dcf1291f 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs @@ -1,5 +1,6 @@ // Copyright (c) Microsoft. All rights reserved. +using System; using System.Diagnostics.CodeAnalysis; namespace Microsoft.Agents.AI.Workflows.Reflection; @@ -10,6 +11,12 @@ namespace Microsoft.Agents.AI.Workflows.Reflection; /// The actual type of the . /// This is used to reflectively discover handlers for messages without violating ILTrim requirements. /// +/// +/// This type is obsolete. Use the on methods in a partial class +/// deriving from instead. +/// +[Obsolete("Use [MessageHandler] attribute on methods in a partial class deriving from Executor. " + + "This type will be removed in a future version.")] public class ReflectingExecutor< [DynamicallyAccessedMembers( ReflectionDemands.RuntimeInterfaceDiscoveryAndInvocation) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/RouteBuilderExtensions.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/RouteBuilderExtensions.cs index f25f896db9..d554138f1e 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/RouteBuilderExtensions.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/RouteBuilderExtensions.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Internal use of obsolete types for backward compatibility + using System; using System.Collections.Generic; using System.Diagnostics; diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/StatefulExecutor.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/StatefulExecutor.cs index 12079289a4..234958a98a 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/StatefulExecutor.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/StatefulExecutor.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Internal use of obsolete types for backward compatibility + using System; using System.Threading; using System.Threading.Tasks; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ReflectionSmokeTest.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ReflectionSmokeTest.cs index ccf3f7bc8b..55aefa0133 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ReflectionSmokeTest.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/ReflectionSmokeTest.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Testing legacy reflection-based pattern + using System; using System.Threading; using System.Threading.Tasks; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/01_Simple_Workflow_Sequential.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/01_Simple_Workflow_Sequential.cs index c6d33e13d7..5af52874f6 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/01_Simple_Workflow_Sequential.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/01_Simple_Workflow_Sequential.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Testing legacy reflection-based pattern + using System; using System.IO; using System.Linq; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/02_Simple_Workflow_Condition.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/02_Simple_Workflow_Condition.cs index 9ee50ae3fb..d44b0babcd 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/02_Simple_Workflow_Condition.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/02_Simple_Workflow_Condition.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Testing legacy reflection-based pattern + using System; using System.IO; using System.Linq; diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/03_Simple_Workflow_Loop.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/03_Simple_Workflow_Loop.cs index 62ba2a8a68..61e063df32 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/03_Simple_Workflow_Loop.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/03_Simple_Workflow_Loop.cs @@ -1,5 +1,7 @@ // Copyright (c) Microsoft. All rights reserved. +#pragma warning disable CS0618 // Type or member is obsolete - Testing legacy reflection-based pattern + using System; using System.IO; using System.Threading; diff --git a/dotnet/wf-code-gen-impact.md b/dotnet/wf-code-gen-impact.md new file mode 100644 index 0000000000..b49c8c0594 --- /dev/null +++ b/dotnet/wf-code-gen-impact.md @@ -0,0 +1,257 @@ +# Source Generator for Workflow Executors: Rationale and Impact + +## Overview + +The Microsoft Agents AI Workflows framework has introduced a Roslyn source generator (`Microsoft.Agents.AI.Workflows.Generators`) that replaces the previous reflection-based approach for discovering and registering message handlers. This document explains why this change was made, what benefits it provides, and how it impacts framework users. + +## Why Move from Reflection to Code Generation? + +### The Previous Approach: `ReflectingExecutor` + +Previously, executors that needed automatic handler discovery inherited from `ReflectingExecutor` and implemented marker interfaces like `IMessageHandler`: + +```csharp +// Old approach - reflection-based +public class MyExecutor : ReflectingExecutor, + IMessageHandler, + IMessageHandler +{ + public ValueTask HandleAsync(QueryMessage msg, IWorkflowContext ctx, CancellationToken ct) + { + // Handle query + } + + public ValueTask HandleAsync(CommandMessage msg, IWorkflowContext ctx, CancellationToken ct) + { + // Handle command and return result + } +} +``` + +This approach had several limitations: + +1. **Runtime overhead**: Handler discovery happened at runtime via reflection, adding latency to executor initialization +2. **No AOT compatibility**: Reflection-based discovery doesn't work with Native AOT compilation +3. **Redundant declarations**: The interface list duplicated information already present in method signatures +4. **Limited metadata**: No clean way to declare yield/send types for protocol validation +5. **Hidden errors**: Invalid handler signatures weren't caught until runtime + +### The New Approach: `[MessageHandler]` Attribute + +The source generator enables a cleaner, attribute-based pattern: + +```csharp +// New approach - source generated +[SendsMessage(typeof(PollToken))] +public partial class MyExecutor : Executor +{ + [MessageHandler] + private ValueTask HandleQueryAsync(QueryMessage msg, IWorkflowContext ctx, CancellationToken ct) + { + // Handle query + } + + [MessageHandler(Yield = [typeof(StreamChunk)], Send = [typeof(InternalMessage)])] + private ValueTask HandleCommandAsync(CommandMessage msg, IWorkflowContext ctx, CancellationToken ct) + { + // Handle command and return result + } +} +``` + +The generator produces a partial class with `ConfigureRoutes()`, `ConfigureSentTypes()`, and `ConfigureYieldTypes()` implementations at compile time. + +## What's Better About Code Generation? + +### 1. Compile-Time Validation + +Invalid handler signatures are caught during compilation, not at runtime: + +```csharp +[MessageHandler] +private void InvalidHandler(string msg) // Error WFGEN005: Missing IWorkflowContext parameter +{ +} +``` + +Diagnostic errors include: +- `WFGEN001`: Handler missing `IWorkflowContext` parameter +- `WFGEN002`: Invalid return type (must be `void`, `ValueTask`, or `ValueTask`) +- `WFGEN003`: Executor class must be `partial` +- `WFGEN004`: `[MessageHandler]` on non-Executor class +- `WFGEN005`: Insufficient parameters +- `WFGEN006`: `ConfigureRoutes` already manually defined + +### 2. Zero Runtime Reflection + +All handler registration happens at compile time. The generated code is simple, direct method calls: + +```csharp +// Generated code +protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) +{ + return routeBuilder + .AddHandler(this.HandleQueryAsync) + .AddHandler(this.HandleCommandAsync); +} +``` + +This eliminates: +- Reflection overhead during initialization +- Assembly scanning +- Dynamic delegate creation + +### 3. Native AOT Compatibility + +Because there's no runtime reflection, executors work seamlessly with .NET Native AOT compilation. This enables: +- Faster startup times +- Smaller deployment sizes +- Deployment to environments that don't support JIT compilation + +### 4. Explicit Protocol Metadata + +The `Yield` and `Send` properties on `[MessageHandler]` plus class-level `[SendsMessage]` and `[YieldsMessage]` attributes provide explicit protocol documentation: + +```csharp +[SendsMessage(typeof(PollToken))] // This executor sends PollToken messages +[YieldsMessage(typeof(FinalResult))] // This executor yields FinalResult to workflow output +public partial class MyExecutor : Executor +{ + [MessageHandler( + Yield = [typeof(StreamChunk)], // This handler yields StreamChunk + Send = [typeof(InternalQuery)])] // This handler sends InternalQuery + private ValueTask HandleAsync(Request req, IWorkflowContext ctx) { ... } +} +``` + +This metadata enables: +- Static protocol validation +- Better IDE tooling and documentation +- Clearer code intent + +### 5. Handler Accessibility Freedom + +Handlers can be `private`, `protected`, `internal`, or `public`. The old interface-based approach required public methods. Now you can encapsulate handler implementations: + +```csharp +public partial class MyExecutor : Executor +{ + [MessageHandler] + private ValueTask HandleInternalAsync(InternalMessage msg, IWorkflowContext ctx) + { + // Private handler - implementation detail + } +} +``` + +### 6. Cleaner Inheritance + +The generator properly handles inheritance chains, calling `base.ConfigureRoutes()` when appropriate: + +```csharp +public partial class DerivedExecutor : BaseExecutor +{ + [MessageHandler] + private ValueTask HandleDerivedAsync(DerivedMessage msg, IWorkflowContext ctx) { ... } +} + +// Generated: +protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) +{ + routeBuilder = base.ConfigureRoutes(routeBuilder); // Preserves base handlers + return routeBuilder + .AddHandler(this.HandleDerivedAsync); +} +``` + +## New Capabilities Enabled + +### 1. Static Workflow Analysis + +With explicit yield/send metadata, tools can analyze workflow graphs at compile time: +- Validate that all message types have handlers +- Detect unreachable executors +- Generate workflow documentation + +### 2. Trimming-Safe Deployments + +The generated code contains no reflection, making it fully compatible with IL trimming. This reduces deployment size significantly for serverless and edge scenarios. + +### 3. Better IDE Experience + +Because the generator runs in the IDE, you get: +- Immediate feedback on handler signature errors +- IntelliSense for generated methods +- Go-to-definition on generated code + +### 4. Protocol Documentation Generation + +The explicit type metadata can be used to generate: +- API documentation +- OpenAPI/Swagger specs for workflow endpoints +- Visual workflow diagrams + +## Impact on Framework Users + +### Migration Path + +Existing code using `ReflectingExecutor` continues to work but is marked `[Obsolete]`. To migrate: + +1. Change base class from `ReflectingExecutor` to `Executor` +2. Add `partial` modifier to the class +3. Replace `IMessageHandler` interfaces with `[MessageHandler]` attributes +4. Optionally add `Yield`/`Send` metadata for protocol validation + +**Before:** +```csharp +public class MyExecutor : ReflectingExecutor, IMessageHandler +{ + public ValueTask HandleAsync(Query q, IWorkflowContext ctx, CancellationToken ct) { ... } +} +``` + +**After:** +```csharp +public partial class MyExecutor : Executor +{ + [MessageHandler] + private ValueTask HandleQueryAsync(Query q, IWorkflowContext ctx, CancellationToken ct) { ... } +} +``` + +### Breaking Changes + +- Classes using `[MessageHandler]` **must** be `partial` +- Handler methods must have at least 2 parameters: `(TMessage, IWorkflowContext)` +- Return type must be `void`, `ValueTask`, or `ValueTask` + +### Performance Improvements + +Users can expect: +- **Faster executor initialization**: No reflection overhead +- **Reduced memory allocation**: No dynamic delegate creation +- **AOT deployment support**: Full Native AOT compatibility +- **Smaller trimmed deployments**: No reflection metadata preserved + +### NuGet Package + +The generator is distributed as a separate NuGet package (`Microsoft.Agents.AI.Workflows.Generators`) that's automatically referenced by the main Workflows package. It's packaged as an analyzer, so it: +- Runs automatically during build +- Requires no additional configuration +- Works in all IDEs that support Roslyn analyzers + +## Summary + +The move from reflection to source generation represents a significant improvement in the Workflows framework: + +| Aspect | Reflection (Old) | Source Generator (New) | +|--------|------------------|------------------------| +| Handler discovery | Runtime | Compile-time | +| Error detection | Runtime exceptions | Compiler errors | +| AOT support | No | Yes | +| Trimming support | Limited | Full | +| Protocol metadata | Implicit | Explicit | +| Handler visibility | Public only | Any | +| Initialization speed | Slower | Faster | + +The source generator approach aligns with modern .NET best practices and positions the framework for future scenarios including edge computing, serverless, and mobile deployments where AOT compilation and minimal footprint are essential. diff --git a/dotnet/wf-source-gen-bp.md b/dotnet/wf-source-gen-bp.md new file mode 100644 index 0000000000..c0f3d25892 --- /dev/null +++ b/dotnet/wf-source-gen-bp.md @@ -0,0 +1,439 @@ +# Source Generator Best Practices Review + +This document reviews the Workflow Executor Route Source Generator implementation against the official Roslyn Source Generator Cookbook best practices from the dotnet/roslyn repository. + +## Reference Documentation + +- [Source Generators Cookbook](https://github.com/dotnet/roslyn/blob/main/docs/features/source-generators.cookbook.md) +- [Incremental Generators Cookbook](https://github.com/dotnet/roslyn/blob/main/docs/features/incremental-generators.cookbook.md) + +--- + +## Executive Summary + +| Category | Status | Priority | +|----------|--------|----------| +| Generator Type | PASS | - | +| Attribute-Based Detection | FAIL | HIGH | +| Model Value Equality | FAIL | HIGH | +| Collection Equality | FAIL | HIGH | +| Symbol/SyntaxNode Storage | PASS | - | +| Code Generation Approach | PASS | - | +| Diagnostics | PASS | - | +| Pipeline Efficiency | FAIL | MEDIUM | +| CancellationToken Handling | PARTIAL | LOW | + +**Overall Assessment**: The generator follows several best practices but has critical performance issues that should be addressed before production use. The most significant issue is not using `ForAttributeWithMetadataName`, which the Roslyn team states is "at least 99x more efficient" than `CreateSyntaxProvider`. + +--- + +## Detailed Analysis + +### 1. Generator Interface Selection + +**Best Practice**: Use `IIncrementalGenerator` instead of the deprecated `ISourceGenerator`. + +**Our Implementation**: PASS + +```csharp +// ExecutorRouteGenerator.cs:19 +public sealed class ExecutorRouteGenerator : IIncrementalGenerator +``` + +The generator correctly implements `IIncrementalGenerator`, the recommended interface for new generators. + +--- + +### 2. Attribute-Based Detection with ForAttributeWithMetadataName + +**Best Practice**: Use `ForAttributeWithMetadataName()` for attribute-based discovery. + +> "This utility method is at least 99x more efficient than `SyntaxProvider.CreateSyntaxProvider`, and in many cases even more efficient." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: FAIL (HIGH PRIORITY) + +```csharp +// ExecutorRouteGenerator.cs:25-30 +var executorCandidates = context.SyntaxProvider + .CreateSyntaxProvider( + predicate: static (node, _) => SyntaxDetector.IsExecutorCandidate(node), + transform: static (ctx, ct) => SemanticAnalyzer.Analyze(ctx, ct, out _)) +``` + +**Problem**: We use `CreateSyntaxProvider` with manual attribute detection in `SyntaxDetector`. This requires the generator to examine every syntax node in the compilation, whereas `ForAttributeWithMetadataName` uses the compiler's built-in attribute index for O(1) lookup. + +**Recommended Fix**: + +```csharp +var executorCandidates = context.SyntaxProvider + .ForAttributeWithMetadataName( + fullyQualifiedMetadataName: "Microsoft.Agents.AI.Workflows.MessageHandlerAttribute", + predicate: static (node, _) => node is MethodDeclarationSyntax, + transform: static (ctx, ct) => AnalyzeMethodWithAttribute(ctx, ct)) + .Collect() + .SelectMany((methods, _) => GroupByContainingClass(methods)); +``` + +**Impact**: Current approach causes IDE lag on every keystroke in large projects. + +--- + +### 3. Model Value Equality (Records vs Classes) + +**Best Practice**: Use `record` types for pipeline models to get automatic value equality. + +> "Use `record`s, rather than `class`es, so that value equality is generated for you." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: FAIL (HIGH PRIORITY) + +```csharp +// HandlerInfo.cs:28 +internal sealed class HandlerInfo { ... } + +// ExecutorInfo.cs:10 +internal sealed class ExecutorInfo { ... } +``` + +**Problem**: Both `HandlerInfo` and `ExecutorInfo` are `sealed class` types, which use reference equality by default. The incremental generator caches results based on equality comparison—when the model equals the previous run's model, regeneration is skipped. With reference equality, every analysis produces a "new" object, defeating caching entirely. + +**Recommended Fix**: + +```csharp +// HandlerInfo.cs +internal sealed record HandlerInfo( + string MethodName, + string InputTypeName, + string? OutputTypeName, + HandlerSignatureKind SignatureKind, + bool HasCancellationToken, + EquatableArray? YieldTypes, + EquatableArray? SendTypes); + +// ExecutorInfo.cs +internal sealed record ExecutorInfo( + string? Namespace, + string ClassName, + string? GenericParameters, + bool IsNested, + string ContainingTypeChain, + bool BaseHasConfigureRoutes, + EquatableArray Handlers, + EquatableArray ClassSendTypes, + EquatableArray ClassYieldTypes); +``` + +**Impact**: Without value equality, the generator regenerates code on every compilation even when nothing changed. + +--- + +### 4. Collection Equality + +**Best Practice**: Use custom equatable wrappers for collections since `ImmutableArray` uses reference equality. + +> "Arrays, `ImmutableArray`, and `List` use reference equality by default. Wrap collections with custom types implementing value-based equality." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: FAIL (HIGH PRIORITY) + +```csharp +// ExecutorInfo.cs:46 +public ImmutableArray Handlers { get; } + +// HandlerInfo.cs:58-63 +public ImmutableArray? YieldTypes { get; } +public ImmutableArray? SendTypes { get; } +``` + +**Problem**: `ImmutableArray` compares by reference, not by contents. Two arrays with identical elements are considered unequal, breaking incremental caching. + +**Recommended Fix**: Create an `EquatableArray` wrapper: + +```csharp +internal readonly struct EquatableArray : IEquatable>, IEnumerable + where T : IEquatable +{ + private readonly ImmutableArray _array; + + public EquatableArray(ImmutableArray array) => _array = array; + + public bool Equals(EquatableArray other) + { + if (_array.Length != other._array.Length) return false; + for (int i = 0; i < _array.Length; i++) + { + if (!_array[i].Equals(other._array[i])) return false; + } + return true; + } + + public override int GetHashCode() + { + var hash = new HashCode(); + foreach (var item in _array) hash.Add(item); + return hash.ToHashCode(); + } + + // ... IEnumerable implementation +} +``` + +**Impact**: Same as model equality—caching is completely broken for handlers and type arrays. + +--- + +### 5. Symbol and SyntaxNode Storage + +**Best Practice**: Never store `ISymbol` or `SyntaxNode` in pipeline models. + +> "Storing `ISymbol` references blocks garbage collection and roots old compilations unnecessarily. Extract only the information you need—typically string representations work well—into your equatable models." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: PASS + +The models correctly store only primitive types and strings: + +```csharp +// HandlerInfo.cs - stores strings, not symbols +public string MethodName { get; } +public string InputTypeName { get; } +public string? OutputTypeName { get; } + +// ExecutorInfo.cs - stores strings, not symbols +public string? Namespace { get; } +public string ClassName { get; } +``` + +The `SemanticAnalyzer` correctly extracts string representations from symbols: + +```csharp +// SemanticAnalyzer.cs:300-301 +var inputType = methodSymbol.Parameters[0].Type; +var inputTypeName = inputType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); +``` + +--- + +### 6. Code Generation Approach + +**Best Practice**: Use `StringBuilder` for code generation, not `SyntaxNode` construction. + +> "Avoid constructing `SyntaxNode`s for output; they're complex to format correctly and `NormalizeWhitespace()` is expensive. Instead, use a `StringBuilder` wrapper that tracks indentation levels." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: PASS + +```csharp +// SourceBuilder.cs:17-19 +public static string Generate(ExecutorInfo info) +{ + var sb = new StringBuilder(); +``` + +The `SourceBuilder` correctly uses `StringBuilder` with manual indentation tracking. + +--- + +### 7. Diagnostic Reporting + +**Best Practice**: Use `ReportDiagnostic` for surfacing issues to users. + +**Our Implementation**: PASS + +```csharp +// ExecutorRouteGenerator.cs:44-50 +context.RegisterSourceOutput(diagnosticsProvider, static (ctx, diagnostics) => +{ + foreach (var diagnostic in diagnostics) + { + ctx.ReportDiagnostic(diagnostic); + } +}); +``` + +Diagnostics are well-defined with appropriate severities: + +| ID | Severity | Description | +|----|----------|-------------| +| WFGEN001 | Error | Missing IWorkflowContext parameter | +| WFGEN002 | Error | Invalid return type | +| WFGEN003 | Error | Class must be partial | +| WFGEN004 | Warning | Not an Executor | +| WFGEN005 | Error | Insufficient parameters | +| WFGEN006 | Info | ConfigureRoutes already defined | +| WFGEN007 | Error | Handler cannot be static | + +--- + +### 8. Pipeline Efficiency + +**Best Practice**: Avoid duplicate work in the pipeline. + +**Our Implementation**: FAIL (MEDIUM PRIORITY) + +```csharp +// ExecutorRouteGenerator.cs:25-41 +// Pipeline 1: Get executor candidates +var executorCandidates = context.SyntaxProvider + .CreateSyntaxProvider( + predicate: static (node, _) => SyntaxDetector.IsExecutorCandidate(node), + transform: static (ctx, ct) => SemanticAnalyzer.Analyze(ctx, ct, out _)) + ... + +// Pipeline 2: Get diagnostics (duplicates the same work!) +var diagnosticsProvider = context.SyntaxProvider + .CreateSyntaxProvider( + predicate: static (node, _) => SyntaxDetector.IsExecutorCandidate(node), + transform: static (ctx, ct) => + { + SemanticAnalyzer.Analyze(ctx, ct, out var diagnostics); + return diagnostics; + }) +``` + +**Problem**: The same syntax detection and semantic analysis runs twice—once for extracting `ExecutorInfo` and once for extracting diagnostics. + +**Recommended Fix**: Return both in a single pipeline: + +```csharp +var analysisResults = context.SyntaxProvider + .ForAttributeWithMetadataName(...) + .Select((ctx, ct) => { + var info = SemanticAnalyzer.Analyze(ctx, ct, out var diagnostics); + return (Info: info, Diagnostics: diagnostics); + }); + +// Split for different outputs +context.RegisterSourceOutput( + analysisResults.Where(r => r.Info != null).Select((r, _) => r.Info!), + GenerateSource); + +context.RegisterSourceOutput( + analysisResults.Where(r => r.Diagnostics.Length > 0).Select((r, _) => r.Diagnostics), + ReportDiagnostics); +``` + +--- + +### 9. Base Type Chain Scanning + +**Best Practice**: Avoid scanning indirect type relationships when possible. + +> "Never scan for types that indirectly implement interfaces, inherit from base types, or acquire attributes through inheritance hierarchies. This pattern forces the generator to inspect every type's `AllInterfaces` or base-type chain on every keystroke." +> — Roslyn Incremental Generators Cookbook + +**Our Implementation**: PARTIAL CONCERN + +```csharp +// SemanticAnalyzer.cs:126-141 +private static bool DerivesFromExecutor(INamedTypeSymbol classSymbol) +{ + var current = classSymbol.BaseType; + while (current != null) + { + var fullName = current.OriginalDefinition.ToDisplayString(); + if (fullName == ExecutorTypeName || fullName.StartsWith(ExecutorTypeName + "<", ...)) + { + return true; + } + current = current.BaseType; + } + return false; +} +``` + +**Analysis**: We do walk the base type chain, but this only happens after attribute filtering (classes must have `[MessageHandler]` methods). Since this is targeted to specific candidates rather than scanning all types, the performance impact is acceptable. However, if we switch to `ForAttributeWithMetadataName`, the attribute is on methods, so we'd need to check the containing class's base types—which is still targeted. + +--- + +### 10. CancellationToken Handling + +**Best Practice**: Respect `CancellationToken` in long-running operations. + +**Our Implementation**: PARTIAL (LOW PRIORITY) + +The `CancellationToken` is passed through to semantic model calls: + +```csharp +// SemanticAnalyzer.cs:46 +var classSymbol = semanticModel.GetDeclaredSymbol(classDecl, cancellationToken); +``` + +However, there are no explicit `cancellationToken.ThrowIfCancellationRequested()` calls in loops like `AnalyzeHandlers`. For most compilations this is fine, but very large classes with many handlers might benefit from periodic checks. + +--- + +### 11. File Naming Convention + +**Best Practice**: Use descriptive generated file names with `.g.cs` suffix. + +**Our Implementation**: PASS + +```csharp +// ExecutorRouteGenerator.cs:62-91 +private static string GetHintName(ExecutorInfo info) +{ + // Produces: "Namespace.ClassName.g.cs" or "Namespace.Outer.Inner.ClassName.g.cs" + ... + sb.Append(".g.cs"); + return sb.ToString(); +} +``` + +--- + +## Recommended Action Plan + +### High Priority (Performance Critical) + +1. **Switch to `ForAttributeWithMetadataName`** + - Estimated impact: 99x+ performance improvement for attribute detection + - Requires restructuring the pipeline to collect methods then group by class + +2. **Convert models to records** + - Change `HandlerInfo` and `ExecutorInfo` from `sealed class` to `sealed record` + - Enables automatic value equality for incremental caching + +3. **Implement `EquatableArray`** + - Create wrapper struct with value-based equality + - Replace all `ImmutableArray` usages in models + +### Medium Priority (Efficiency) + +4. **Eliminate duplicate pipeline execution** + - Combine info extraction and diagnostic collection into single pipeline + - Split outputs using `Where` and `Select` + +### Low Priority (Polish) + +5. **Add periodic cancellation checks** + - Add `ThrowIfCancellationRequested()` in handler analysis loop + - Only needed for extremely large classes + +--- + +## Compliance Matrix + +| Best Practice | Cookbook Reference | Status | Fix Required | +|--------------|-------------------|--------|--------------| +| Use IIncrementalGenerator | Main cookbook | PASS | No | +| Use ForAttributeWithMetadataName | Incremental cookbook | FAIL | Yes (High) | +| Use records for models | Incremental cookbook | FAIL | Yes (High) | +| Implement collection equality | Incremental cookbook | FAIL | Yes (High) | +| Don't store ISymbol/SyntaxNode | Incremental cookbook | PASS | No | +| Use StringBuilder for codegen | Incremental cookbook | PASS | No | +| Report diagnostics properly | Main cookbook | PASS | No | +| Avoid duplicate pipeline work | Incremental cookbook | FAIL | Yes (Medium) | +| Respect CancellationToken | Main cookbook | PARTIAL | Optional | +| Use .g.cs file suffix | Main cookbook | PASS | No | +| Additive-only generation | Main cookbook | PASS | No | +| No language feature emulation | Main cookbook | PASS | No | + +--- + +## Conclusion + +The source generator implementation demonstrates solid understanding of Roslyn generator fundamentals—correct interface usage, proper diagnostic reporting, and appropriate code generation patterns. However, critical performance optimizations are missing that could cause significant IDE lag in production environments. + +The three high-priority fixes (ForAttributeWithMetadataName, record models, and EquatableArray) should be implemented before the generator is used in large codebases. These changes will enable proper incremental caching, reducing regeneration from "every keystroke" to "only when relevant code changes." diff --git a/dotnet/wf-source-gen-changes.md b/dotnet/wf-source-gen-changes.md new file mode 100644 index 0000000000..cc0aca5157 --- /dev/null +++ b/dotnet/wf-source-gen-changes.md @@ -0,0 +1,258 @@ +# Workflow Executor Route Source Generator - Implementation Summary + +This document summarizes all changes made to implement a Roslyn source generator that replaces the reflection-based `ReflectingExecutor` pattern with compile-time code generation using `[MessageHandler]` attributes. + +## Overview + +The source generator automatically discovers methods marked with `[MessageHandler]` and generates `ConfigureRoutes`, `ConfigureSentTypes`, and `ConfigureYieldTypes` method implementations at compile time. This improves AOT compatibility and eliminates the need for the CRTP (Curiously Recurring Template Pattern) used by `ReflectingExecutor`. + +## New Files Created + +### Attributes (3 files) + +| File | Purpose | +|------|---------| +| `src/Microsoft.Agents.AI.Workflows/Attributes/MessageHandlerAttribute.cs` | Marks methods as message handlers with optional `Yield` and `Send` type arrays | +| `src/Microsoft.Agents.AI.Workflows/Attributes/SendsMessageAttribute.cs` | Class-level attribute declaring message types an executor may send | +| `src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs` | Class-level attribute declaring output types an executor may yield | + +### Source Generator Project (8 files) + +| File | Purpose | +|------|---------| +| `src/Microsoft.Agents.AI.Workflows.Generators/Microsoft.Agents.AI.Workflows.Generators.csproj` | Project file targeting netstandard2.0 with Roslyn component settings | +| `src/Microsoft.Agents.AI.Workflows.Generators/ExecutorRouteGenerator.cs` | Main incremental generator implementing `IIncrementalGenerator` | +| `src/Microsoft.Agents.AI.Workflows.Generators/Models/HandlerInfo.cs` | Data model for handler method information | +| `src/Microsoft.Agents.AI.Workflows.Generators/Models/ExecutorInfo.cs` | Data model for executor class information | +| `src/Microsoft.Agents.AI.Workflows.Generators/Analysis/SyntaxDetector.cs` | Fast syntax-level candidate detection | +| `src/Microsoft.Agents.AI.Workflows.Generators/Analysis/SemanticAnalyzer.cs` | Semantic validation and type extraction | +| `src/Microsoft.Agents.AI.Workflows.Generators/Generation/SourceBuilder.cs` | Code generation logic | +| `src/Microsoft.Agents.AI.Workflows.Generators/Diagnostics/DiagnosticDescriptors.cs` | Analyzer diagnostic definitions | + +## Files Modified + +### Project Files + +| File | Changes | +|------|---------| +| `src/Microsoft.Agents.AI.Workflows/Microsoft.Agents.AI.Workflows.csproj` | Added generator project reference and `InternalsVisibleTo` for generator tests | +| `Directory.Packages.props` | Added `Microsoft.CodeAnalysis.Analyzers` version 3.11.0 | +| `agent-framework-dotnet.slnx` | Added generator project to solution | + +### Obsolete Annotations + +| File | Changes | +|------|---------| +| `src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs` | Added `[Obsolete]` attribute with migration guidance | +| `src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs` | Added `[Obsolete]` to both `IMessageHandler` and `IMessageHandler` interfaces | + +### Pragma Suppressions for Internal Obsolete Usage + +| File | Changes | +|------|---------| +| `src/Microsoft.Agents.AI.Workflows/Executor.cs` | Added `#pragma warning disable CS0618` | +| `src/Microsoft.Agents.AI.Workflows/StatefulExecutor.cs` | Added `#pragma warning disable CS0618` | +| `src/Microsoft.Agents.AI.Workflows/Reflection/RouteBuilderExtensions.cs` | Added `#pragma warning disable CS0618` | +| `src/Microsoft.Agents.AI.Workflows/Reflection/MessageHandlerInfo.cs` | Added `#pragma warning disable CS0618` | + +### Test File Pragma Suppressions + +| File | Changes | +|------|---------| +| `tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/01_Simple_Workflow_Sequential.cs` | Added `#pragma warning disable CS0618` for legacy pattern testing | +| `tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/02_Simple_Workflow_Condition.cs` | Added `#pragma warning disable CS0618` for legacy pattern testing | +| `tests/Microsoft.Agents.AI.Workflows.UnitTests/Sample/03_Simple_Workflow_Loop.cs` | Added `#pragma warning disable CS0618` for legacy pattern testing | +| `tests/Microsoft.Agents.AI.Workflows.UnitTests/ReflectionSmokeTest.cs` | Added `#pragma warning disable CS0618` for legacy pattern testing | + +## Attribute Definitions + +### MessageHandlerAttribute + +```csharp +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = false)] +public sealed class MessageHandlerAttribute : Attribute +{ + public Type[]? Yield { get; set; } // Types yielded as workflow outputs + public Type[]? Send { get; set; } // Types sent to other executors +} +``` + +### SendsMessageAttribute + +```csharp +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] +public sealed class SendsMessageAttribute : Attribute +{ + public Type Type { get; } + public SendsMessageAttribute(Type type) => this.Type = Throw.IfNull(type); +} +``` + +### YieldsMessageAttribute + +```csharp +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] +public sealed class YieldsMessageAttribute : Attribute +{ + public Type Type { get; } + public YieldsMessageAttribute(Type type) => this.Type = Throw.IfNull(type); +} +``` + +## Diagnostic Rules + +| ID | Severity | Description | +|----|----------|-------------| +| `WFGEN001` | Error | Handler method must have at least 2 parameters (message and IWorkflowContext) | +| `WFGEN002` | Error | Handler method's second parameter must be IWorkflowContext | +| `WFGEN003` | Error | Handler method must return void, ValueTask, or ValueTask | +| `WFGEN004` | Error | Executor class with [MessageHandler] methods must be declared as partial | +| `WFGEN005` | Warning | [MessageHandler] attribute on method in non-Executor class (ignored) | +| `WFGEN006` | Info | ConfigureRoutes already defined manually, [MessageHandler] methods ignored | +| `WFGEN007` | Error | Handler method's third parameter (if present) must be CancellationToken | + +## Handler Signature Support + +The generator supports the following method signatures: + +| Return Type | Parameters | Generated Call | +|-------------|------------|----------------| +| `void` | `(TMessage, IWorkflowContext)` | `AddHandler(this.Method)` | +| `void` | `(TMessage, IWorkflowContext, CancellationToken)` | `AddHandler(this.Method)` | +| `ValueTask` | `(TMessage, IWorkflowContext)` | `AddHandler(this.Method)` | +| `ValueTask` | `(TMessage, IWorkflowContext, CancellationToken)` | `AddHandler(this.Method)` | +| `TResult` | `(TMessage, IWorkflowContext)` | `AddHandler(this.Method)` | +| `TResult` | `(TMessage, IWorkflowContext, CancellationToken)` | `AddHandler(this.Method)` | +| `ValueTask` | `(TMessage, IWorkflowContext)` | `AddHandler(this.Method)` | +| `ValueTask` | `(TMessage, IWorkflowContext, CancellationToken)` | `AddHandler(this.Method)` | + +## Generated Code Example + +### Input (User Code) + +```csharp +[SendsMessage(typeof(PollToken))] +public partial class MyChatExecutor : Executor +{ + [MessageHandler] + private async ValueTask HandleQueryAsync( + ChatQuery query, IWorkflowContext ctx, CancellationToken ct) + { + return new ChatResponse(...); + } + + [MessageHandler(Yield = new[] { typeof(StreamChunk) }, Send = new[] { typeof(InternalMessage) })] + private void HandleStream(StreamRequest req, IWorkflowContext ctx) + { + // Handler implementation + } +} +``` + +### Output (Generated Code) + +```csharp +// +#nullable enable + +namespace MyNamespace; + +partial class MyChatExecutor +{ + protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) + { + return routeBuilder + .AddHandler(this.HandleQueryAsync) + .AddHandler(this.HandleStream); + } + + protected override ISet ConfigureSentTypes() + { + var types = base.ConfigureSentTypes(); + types.Add(typeof(PollToken)); + types.Add(typeof(InternalMessage)); + return types; + } + + protected override ISet ConfigureYieldTypes() + { + var types = base.ConfigureYieldTypes(); + types.Add(typeof(ChatResponse)); + types.Add(typeof(StreamChunk)); + return types; + } +} +``` + +## Build Issues Resolved + +### 1. NU1008 - Central Package Management +Package references in the generator project had inline versions, which conflicts with central package management. Fixed by removing `Version` attributes from `PackageReference` items. + +### 2. RS2008 - Analyzer Release Tracking +Roslyn requires analyzer release tracking documentation. Fixed by adding `$(NoWarn);RS2008` to the generator project. + +### 3. CA1068 - CancellationToken Parameter Order +Method parameters were in wrong order. Fixed by reordering `CancellationToken` to be last. + +### 4. RCS1146 - Conditional Access +Used null check with `&&` instead of `?.` operator. Fixed by using conditional access. + +### 5. CA1310 - StringComparison +`StartsWith(string)` calls without `StringComparison`. Fixed by adding `StringComparison.Ordinal`. + +### 6. CS0103 - Missing Using Directive +Missing `using System;` in SemanticAnalyzer.cs. Fixed by adding the using directive. + +### 7. CS0618 - Obsolete Warnings as Errors +Internal uses of obsolete types caused build failures (TreatWarningsAsErrors). Fixed by adding `#pragma warning disable CS0618` to affected internal files and test files. + +### 8. NU1109 - Package Version Conflict +`Microsoft.CodeAnalysis.Analyzers` 3.3.4 conflicts with `Microsoft.CodeAnalysis.CSharp` 4.14.0 which requires >= 3.11.0. Fixed by updating version to 3.11.0 in `Directory.Packages.props`. + +### 9. RS1041 - Wrong Target Framework for Analyzer +The generator was being multi-targeted due to inherited `TargetFrameworks` from `Directory.Build.props`. Fixed by clearing `TargetFrameworks` and only setting `TargetFramework` to `netstandard2.0`. + +## Migration Guide + +### Before (Reflection-based) + +```csharp +public class MyExecutor : ReflectingExecutor, IMessageHandler +{ + public MyExecutor() : base("MyExecutor") { } + + public ValueTask HandleAsync(MyMessage message, IWorkflowContext context, CancellationToken ct) + { + // Handler implementation + } +} +``` + +### After (Source Generator) + +```csharp +public partial class MyExecutor : Executor +{ + public MyExecutor() : base("MyExecutor") { } + + [MessageHandler] + private ValueTask HandleAsync(MyMessage message, IWorkflowContext context, CancellationToken ct) + { + // Handler implementation + } +} +``` + +Key migration steps: +1. Change base class from `ReflectingExecutor` to `Executor` +2. Add `partial` modifier to the class +3. Remove `IMessageHandler` interface implementations +4. Add `[MessageHandler]` attribute to handler methods +5. Handler methods can now be any accessibility (private, protected, internal, public) + +## Future Work + +- Create comprehensive unit tests for the source generator +- Add integration tests verifying generated routes match reflection-discovered routes +- Consider adding IDE quick-fix for migrating from `ReflectingExecutor` pattern diff --git a/wf-source-gen-plan.md b/wf-source-gen-plan.md new file mode 100644 index 0000000000..e936b538b2 --- /dev/null +++ b/wf-source-gen-plan.md @@ -0,0 +1,293 @@ +# Roslyn Source Generator for Workflow Executor Routes + +## Overview + +Replace the reflection-based `ReflectingExecutor` pattern with a compile-time source generator that discovers `[MessageHandler]` attributed methods and generates `ConfigureRoutes`, `ConfigureSentTypes`, and `ConfigureYieldTypes` implementations. + +## Design Decisions (Confirmed) + +- **Attribute syntax**: Inline properties on `[MessageHandler(Yield=[...], Send=[...])]` +- **Class-level attributes**: Generate `ConfigureSentTypes()`/`ConfigureYieldTypes()` from `[SendsMessage]`/`[YieldsMessage]` +- **Migration**: Clean break - requires direct `Executor` inheritance (not `ReflectingExecutor`) +- **Handler accessibility**: Any (private, protected, internal, public) + +--- + +## Implementation Steps + +### Phase 1: Create Source Generator Project + +**1.1 Create project structure:** +``` +dotnet/src/Microsoft.Agents.AI.Workflows.Generators/ +├── Microsoft.Agents.AI.Workflows.Generators.csproj +├── ExecutorRouteGenerator.cs # Main incremental generator +├── Models/ +│ ├── ExecutorInfo.cs # Data model for executor analysis +│ └── HandlerInfo.cs # Data model for handler methods +├── Analysis/ +│ ├── SyntaxDetector.cs # Syntax-based candidate detection +│ └── SemanticAnalyzer.cs # Semantic model analysis +├── Generation/ +│ └── SourceBuilder.cs # Code generation logic +└── Diagnostics/ + └── DiagnosticDescriptors.cs # Analyzer diagnostics +``` + +**1.2 Project file configuration:** +- Target `netstandard2.0` +- Reference `Microsoft.CodeAnalysis.CSharp` 4.8.0+ +- Set `IsRoslynComponent=true`, `EnforceExtendedAnalyzerRules=true` +- Package as analyzer in `analyzers/dotnet/cs` + +### Phase 2: Define Attributes + +**2.1 Create `MessageHandlerAttribute`:** +``` +dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/MessageHandlerAttribute.cs +``` +```csharp +[AttributeUsage(AttributeTargets.Method, AllowMultiple = false, Inherited = false)] +public sealed class MessageHandlerAttribute : Attribute +{ + public Type[]? Yield { get; set; } // Types yielded as workflow outputs + public Type[]? Send { get; set; } // Types sent to other executors +} +``` + +**2.2 Create `SendsMessageAttribute`:** +``` +dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/SendsMessageAttribute.cs +``` +```csharp +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] +public sealed class SendsMessageAttribute : Attribute +{ + public Type Type { get; } + public SendsMessageAttribute(Type type) => this.Type = type; +} +``` + +**2.3 Create `YieldsMessageAttribute`:** +``` +dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs +``` +```csharp +[AttributeUsage(AttributeTargets.Class, AllowMultiple = true, Inherited = true)] +public sealed class YieldsMessageAttribute : Attribute +{ + public Type Type { get; } + public YieldsMessageAttribute(Type type) => this.Type = type; +} +``` + +### Phase 3: Implement Source Generator + +**3.1 Detection criteria (syntax level):** +- Class has `partial` modifier +- Class has at least one method with `[MessageHandler]` attribute + +**3.2 Validation criteria (semantic level):** +- Class derives from `Executor` (directly or transitively) +- Class does NOT already define `ConfigureRoutes` with a body +- Handler method has valid signature: `(TMessage, IWorkflowContext[, CancellationToken])` +- Handler returns `void`, `ValueTask`, or `ValueTask` + +**3.3 Handler signature mapping:** + +| Method Signature | Generated AddHandler Call | +|-----------------|---------------------------| +| `void Handler(T, IWorkflowContext)` | `AddHandler(this.Handler)` | +| `void Handler(T, IWorkflowContext, CT)` | `AddHandler(this.Handler)` | +| `ValueTask Handler(T, IWorkflowContext)` | `AddHandler(this.Handler)` | +| `ValueTask Handler(T, IWorkflowContext, CT)` | `AddHandler(this.Handler)` | +| `TResult Handler(T, IWorkflowContext)` | `AddHandler(this.Handler)` | +| `ValueTask Handler(T, IWorkflowContext, CT)` | `AddHandler(this.Handler)` | + +**3.4 Generated code structure:** +```csharp +// +#nullable enable + +namespace MyNamespace; + +partial class MyExecutor +{ + protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) + { + // Call base if inheriting from another executor with routes + // routeBuilder = base.ConfigureRoutes(routeBuilder); + + return routeBuilder + .AddHandler(this.Handler1) + .AddHandler(this.Handler2); + } + + protected override ISet ConfigureSentTypes() + { + var types = base.ConfigureSentTypes(); + types.Add(typeof(SentType1)); + return types; + } + + protected override ISet ConfigureYieldTypes() + { + var types = base.ConfigureYieldTypes(); + types.Add(typeof(YieldType1)); + return types; + } +} +``` + +**3.5 Inheritance handling:** + +| Scenario | Generated `ConfigureRoutes` | +|----------|----------------------------| +| Directly extends `Executor` | No base call (abstract) | +| Extends executor with `[MessageHandler]` methods | `routeBuilder = base.ConfigureRoutes(routeBuilder);` | +| Extends executor with manual `ConfigureRoutes` | `routeBuilder = base.ConfigureRoutes(routeBuilder);` | + +### Phase 4: Analyzer Diagnostics + +| ID | Severity | Condition | +|----|----------|-----------| +| `WFGEN001` | Error | Handler missing `IWorkflowContext` parameter | +| `WFGEN002` | Error | Handler has invalid return type | +| `WFGEN003` | Error | Executor with `[MessageHandler]` must be `partial` | +| `WFGEN004` | Warning | `[MessageHandler]` on non-Executor class | +| `WFGEN005` | Error | Handler has fewer than 2 parameters | +| `WFGEN006` | Info | `ConfigureRoutes` already defined, handlers ignored | + +### Phase 5: Integration & Migration + +**5.1 Wire generator to main project:** +```xml + + + + +``` + +**5.2 Mark `ReflectingExecutor` obsolete:** +```csharp +[Obsolete("Use [MessageHandler] attribute on methods in a partial class deriving from Executor. " + + "See migration guide. This type will be removed in v1.0.", error: false)] +public class ReflectingExecutor : Executor ... +``` + +**5.3 Mark `IMessageHandler` interfaces obsolete:** +```csharp +[Obsolete("Use [MessageHandler] attribute instead.")] +public interface IMessageHandler { ... } +``` + +### Phase 6: Testing + +**6.1 Generator unit tests:** +``` +dotnet/tests/Microsoft.Agents.AI.Workflows.Generators.UnitTests/ +├── ExecutorRouteGeneratorTests.cs +├── SyntaxDetectorTests.cs +├── SemanticAnalyzerTests.cs +└── TestHelpers/ + └── GeneratorTestHelper.cs +``` + +Test cases: +- Simple single handler +- Multiple handlers on one class +- Handlers with different signatures (void, ValueTask, ValueTask) +- Nested classes +- Generic executors +- Inheritance chains (Executor -> CustomBase -> Concrete) +- Class-level `[SendsMessage]`/`[YieldsMessage]` attributes +- Manual `ConfigureRoutes` present (should skip generation) +- Invalid signatures (should produce diagnostics) + +**6.2 Integration tests:** +- Port existing `ReflectingExecutor` test cases to use `[MessageHandler]` +- Verify generated routes match reflection-discovered routes + +--- + +## Files to Create + +| Path | Purpose | +|------|---------| +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Microsoft.Agents.AI.Workflows.Generators.csproj` | Generator project | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/ExecutorRouteGenerator.cs` | Main generator | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Models/ExecutorInfo.cs` | Data model | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Models/HandlerInfo.cs` | Data model | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Analysis/SyntaxDetector.cs` | Syntax analysis | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Analysis/SemanticAnalyzer.cs` | Semantic analysis | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Generation/SourceBuilder.cs` | Code gen | +| `dotnet/src/Microsoft.Agents.AI.Workflows.Generators/Diagnostics/DiagnosticDescriptors.cs` | Diagnostics | +| `dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/MessageHandlerAttribute.cs` | Handler attribute | +| `dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/SendsMessageAttribute.cs` | Class-level send | +| `dotnet/src/Microsoft.Agents.AI.Workflows/Attributes/YieldsMessageAttribute.cs` | Class-level yield | +| `dotnet/tests/Microsoft.Agents.AI.Workflows.Generators.UnitTests/*.cs` | Generator tests | + +## Files to Modify + +| Path | Changes | +|------|---------| +| `dotnet/src/Microsoft.Agents.AI.Workflows/Microsoft.Agents.AI.Workflows.csproj` | Add generator reference | +| `dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/ReflectingExecutor.cs` | Add `[Obsolete]` | +| `dotnet/src/Microsoft.Agents.AI.Workflows/Reflection/IMessageHandler.cs` | Add `[Obsolete]` | +| `dotnet/Microsoft.Agents.sln` | Add new projects | + +--- + +## Example Usage (End State) + +```csharp +[SendsMessage(typeof(PollToken))] +public partial class MyChatExecutor : ChatProtocolExecutor +{ + [MessageHandler] + private async ValueTask HandleQueryAsync( + ChatQuery query, IWorkflowContext ctx, CancellationToken ct) + { + // Return type automatically inferred as output + return new ChatResponse(...); + } + + [MessageHandler(Yield = [typeof(StreamChunk)], Send = [typeof(InternalMessage)])] + private void HandleStream(StreamRequest req, IWorkflowContext ctx) + { + // Explicit Yield/Send for complex handlers + } +} +``` + +Generated: +```csharp +partial class MyChatExecutor +{ + protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) + { + routeBuilder = base.ConfigureRoutes(routeBuilder); + return routeBuilder + .AddHandler(this.HandleQueryAsync) + .AddHandler(this.HandleStream); + } + + protected override ISet ConfigureSentTypes() + { + var types = base.ConfigureSentTypes(); + types.Add(typeof(PollToken)); + types.Add(typeof(InternalMessage)); // From handler attribute + return types; + } + + protected override ISet ConfigureYieldTypes() + { + var types = base.ConfigureYieldTypes(); + types.Add(typeof(ChatResponse)); // From return type + types.Add(typeof(StreamChunk)); // From handler attribute + return types; + } +} +```