-
Notifications
You must be signed in to change notification settings - Fork 966
Description
I was reviewing documentation of Workflows Core Concepts following the samples and noticed several concerns with the current APIs shown there.
The following list is composed of suggestions and questions intended to foster discussion, and there's no particular order in the items described.
-
Generic edge methods like
AddCase<T>,AddFanOutEdge<T>,AddEdge<T>box values, silently convert type mismatches todefaultinstead of throwing exceptions and lack type safety. I also don't see a way to ensure static type safety between executors. These APIs should be honest about their runtime behavior and useobject?instead. -
FanOutEdges "partitioner" parameter should be "discriminator" or "selector". Contrast it with
Partitioner.Createwhich actually partitions data. -
AddEdgeidempotent parameter only works when condition is null. What's the use case for bypassing the duplicate edge exception?WorkflowBuilder builder = new WorkflowBuilder(sourceExecutor) .AddEdge<string>(sourceExecutor, targetExecutor, condition: a => true, idempotent: false) .AddEdge<string>(sourceExecutor, targetExecutor, condition: a => true, idempotent: false); // duplicate builder.Build(); // no error
-
StreamAsynchas overloads without input parameters butRunAsyncdoesn't. This also causes ambiguity as described in .NET: Unintuitive behaviour with StreamAsync overloads binding runId more tightly than input; #1773. -
IMessageHandleris inMAAI.Workflows.Reflectionnamespace but is used beyond ReflectingExecutor. Should be in a more general namespace. -
Workflow Validation documentation claims don't match actual behavior:
-
Ensures message types are compatible between connected executors
WorkflowBuilder builder = new WorkflowBuilder(sourceExecutor) .AddEdge(sourceExecutor, new FloatExecutor()); var workflow = builder.Build(); // no error
RunAsync<T>with mismatched type also silently completes:Func<string, IWorkflowContext, CancellationToken, ValueTask<string>> reverseFunc = (text, ctx, ct) => ValueTask.FromResult(string.Concat(text.Reverse())); var executor = reverseFunc.AsExecutor("ReverseExecutor"); Workflow workflow = new WorkflowBuilder(executor).Build(); await using Run run = await InProcessExecution.RunAsync(workflow, 42); // no error with int
-
Graph Connectivity: Verifies all executors are reachable from the start executor
WorkflowBuilder builder = new WorkflowBuilder(first) .AddEdge(second, third); // second & third unreachable from first builder.Build(); // no error
-
Executor Binding: Confirms all executors are properly bound and instantiated
WorkflowBuilder builder = new WorkflowBuilder(new UnboundExecutor()); builder.Build(); // no error internal class UnboundExecutor() : Executor("UnboundExecutor"), IMessageHandler<string, string> { public ValueTask<string> HandleAsync(string message, IWorkflowContext context, CancellationToken cancellationToken = default) => ValueTask.FromResult(message); protected override RouteBuilder ConfigureRoutes(RouteBuilder routeBuilder) => routeBuilder; // Empty - handler never bound! }
-
Edge Validation: Checks for duplicate edges and invalid connections
(See number 3 for duplicate edge example)
-
-
Is
ReflectingExecutor<TExecutor> : Executor where TExecutor : ReflectingExecutor<TExecutor>only generic for trimming? Is binary size a primary goal? -
FanInEdge - how to detect when all sources have completed without an ahead-of-time count?