-
Notifications
You must be signed in to change notification settings - Fork 646
.NET: adds support for labels in edges, fixes rendering of labels in dot a… #1507
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?
Changes from 1 commit
a75c93c
e6c26a0
eac37f9
22c754d
642eebd
2b5ddf1
f499fb3
687ce7b
9dd8ae4
2d1976e
ef2576d
1767091
0cdc299
4b835c7
8216a68
9b26a6e
f0982a3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -13,12 +13,13 @@ namespace Microsoft.Agents.AI.Workflows; | |
| /// </summary> | ||
| internal sealed class FanOutEdgeData : EdgeData | ||
| { | ||
| internal FanOutEdgeData(string sourceId, List<string> sinkIds, EdgeId edgeId, AssignerF? assigner = null) : base(edgeId) | ||
| internal FanOutEdgeData(string sourceId, List<string> sinkIds, EdgeId edgeId, AssignerF? assigner = null, string? label = null) : base(edgeId) | ||
| { | ||
| this.SourceId = sourceId; | ||
| this.SinkIds = sinkIds; | ||
| this.EdgeAssigner = assigner; | ||
| this.Connection = new([sourceId], sinkIds); | ||
| this.Label = label; | ||
| } | ||
|
|
||
| /// <summary> | ||
|
|
@@ -37,6 +38,11 @@ internal FanOutEdgeData(string sourceId, List<string> sinkIds, EdgeId edgeId, As | |
| /// </summary> | ||
| public AssignerF? EdgeAssigner { get; } | ||
|
|
||
| /// <summary> | ||
| /// An optional label for the edge. | ||
| /// </summary> | ||
| public string? Label { get; } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. [Minor] This should probably be on EdgeData, rather than the specific EdgeDatas. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should I move it? I was unsure if you wanted to keep EdgeData pure, but I agree to the suggestion. happy to move this to EdgeData... There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's unify them |
||
|
|
||
| /// <inheritdoc /> | ||
| internal override EdgeConnection Connection { get; } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -99,10 +99,26 @@ private static void EmitWorkflowDigraph(Workflow workflow, List<string> lines, s | |
| } | ||
|
|
||
| // Emit normal edges | ||
| foreach (var (src, target, isConditional) in ComputeNormalEdges(workflow)) | ||
| foreach (var (src, target, isConditional, label) in ComputeNormalEdges(workflow)) | ||
| { | ||
| var edgeAttr = isConditional ? " [style=dashed, label=\"conditional\"]" : ""; | ||
| lines.Add($"{indent}\"{MapId(src)}\" -> \"{MapId(target)}\"{edgeAttr};"); | ||
| // Build edge attributes | ||
| var attributes = new List<string>(); | ||
|
|
||
| // Add style for conditional edges | ||
| if (isConditional) | ||
| { | ||
| attributes.Add("style=dashed"); | ||
| } | ||
|
|
||
| // Add label (custom label or default "conditional" for conditional edges) | ||
| if (label != null) | ||
| { | ||
| attributes.Add($"label=\"{EscapeDotLabel(label)}\""); | ||
| } | ||
|
|
||
| // Combine attributes | ||
| var attrString = attributes.Count > 0 ? $" [{string.Join(", ", attributes)}]" : ""; | ||
| lines.Add($"{indent}\"{MapId(src)}\" -> \"{MapId(target)}\"{attrString};"); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do the ids need to be put through There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no, but I thought it would be nice in case we find text in chinese, korean or using other special characters, to scape them for compatibility. Can remove those escape functions though in aras to simplicity, becoming responsibility of the user then. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's keep it consistent, but I do not have a strong opinion as to which of the two. |
||
| } | ||
| } | ||
|
|
||
|
|
@@ -175,14 +191,21 @@ string sanitize(string input) | |
| } | ||
|
|
||
| // Emit normal edges | ||
| foreach (var (src, target, isConditional) in ComputeNormalEdges(workflow)) | ||
| foreach (var (src, target, isConditional, label) in ComputeNormalEdges(workflow)) | ||
| { | ||
| if (isConditional) | ||
| if (label != null) | ||
| { | ||
| // Regular edge with label | ||
| lines.Add($"{indent}{MapId(src)} -->|{label}| {MapId(target)};"); | ||
| } | ||
joslat marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| else if (isConditional) | ||
| { | ||
| lines.Add($"{indent}{MapId(src)} -. conditional .--> {MapId(target)};"); | ||
| // Conditional edge with default label | ||
| lines.Add($"{indent}{MapId(src)} -->|conditional| {MapId(target)};"); | ||
| } | ||
| else | ||
| { | ||
| // Regular edge without label | ||
| lines.Add($"{indent}{MapId(src)} --> {MapId(target)};"); | ||
| } | ||
| } | ||
|
|
@@ -214,9 +237,9 @@ string sanitize(string input) | |
| return result; | ||
| } | ||
|
|
||
| private static List<(string Source, string Target, bool IsConditional)> ComputeNormalEdges(Workflow workflow) | ||
| private static List<(string Source, string Target, bool IsConditional, string? Label)> ComputeNormalEdges(Workflow workflow) | ||
| { | ||
| var edges = new List<(string, string, bool)>(); | ||
| var edges = new List<(string, string, bool, string?)>(); | ||
| foreach (var edgeGroup in workflow.Edges.Values.SelectMany(x => x)) | ||
| { | ||
| if (edgeGroup.Kind == EdgeKind.FanIn) | ||
|
|
@@ -229,14 +252,15 @@ string sanitize(string input) | |
| case EdgeKind.Direct when edgeGroup.DirectEdgeData != null: | ||
| var directData = edgeGroup.DirectEdgeData; | ||
| var isConditional = directData.Condition != null; | ||
| edges.Add((directData.SourceId, directData.SinkId, isConditional)); | ||
| var label = directData.Label ?? (isConditional ? "conditional" : null); | ||
joslat marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| edges.Add((directData.SourceId, directData.SinkId, isConditional, label)); | ||
| break; | ||
|
|
||
| case EdgeKind.FanOut when edgeGroup.FanOutEdgeData != null: | ||
| var fanOutData = edgeGroup.FanOutEdgeData; | ||
| foreach (var sinkId in fanOutData.SinkIds) | ||
| { | ||
| edges.Add((fanOutData.SourceId, sinkId, false)); | ||
| edges.Add((fanOutData.SourceId, sinkId, false, fanOutData.Label)); | ||
| } | ||
| break; | ||
| } | ||
|
|
@@ -276,5 +300,11 @@ private static bool TryGetNestedWorkflow(ExecutorRegistration registration, [Not | |
| return false; | ||
| } | ||
|
|
||
| // Helper method to escape special characters in DOT labels | ||
| private static string EscapeDotLabel(string label) | ||
| { | ||
| return label.Replace("\"", "\\\"").Replace("\n", "\\n"); | ||
| } | ||
|
|
||
| #endregion | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.