From d5114c24708c5bc9c82972f676ff13f4c9200aa4 Mon Sep 17 00:00:00 2001 From: alliscode Date: Wed, 25 Feb 2026 09:05:19 -0800 Subject: [PATCH 1/5] Fix Mermaid rendering errors in WorkflowVisualizer.ToMermaidString Fix two bugs in the Mermaid diagram output: 1. Use safe node aliases (node_0, node_1, ...) instead of raw executor IDs as Mermaid node identifiers. Raw IDs containing spaces, dots, or non-ASCII characters (e.g. Japanese) caused Mermaid parse errors. 2. Fix conditional edge arrow syntax from '.--> ' (invalid) to '.-> ' (valid Mermaid dotted arrow syntax). Fixes #1406 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Visualization/WorkflowVisualizer.cs | 31 +++- .../WorkflowVisualizerTests.cs | 172 ++++++++++++++---- 2 files changed, 154 insertions(+), 49 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs index e1b69e9f9e..7ef0f63f55 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs @@ -153,18 +153,31 @@ private static void EmitSubWorkflowsDigraph(Workflow workflow, List line private static void EmitWorkflowMermaid(Workflow workflow, List lines, string indent, string? ns = null) { - string MapId(string id) => ns != null ? $"{ns}/{id}" : id; + // Build a mapping from raw IDs to Mermaid-safe node aliases. + // Mermaid node IDs cannot contain spaces, dots, or non-ASCII characters. + var aliasMap = new Dictionary(); + string GetSafeId(string id) + { + var key = ns != null ? $"{ns}/{id}" : id; + if (!aliasMap.TryGetValue(key, out var alias)) + { + alias = $"node_{aliasMap.Count}"; + aliasMap[key] = alias; + } + + return alias; + } // Add start node var startExecutorId = workflow.StartExecutorId; - lines.Add($"{indent}{MapId(startExecutorId)}[\"{startExecutorId} (Start)\"];"); + lines.Add($"{indent}{GetSafeId(startExecutorId)}[\"{startExecutorId} (Start)\"];"); // Add other executor nodes foreach (var executorId in workflow.ExecutorBindings.Keys) { if (executorId != startExecutorId) { - lines.Add($"{indent}{MapId(executorId)}[\"{executorId}\"];"); + lines.Add($"{indent}{GetSafeId(executorId)}[\"{executorId}\"];"); } } @@ -175,7 +188,7 @@ private static void EmitWorkflowMermaid(Workflow workflow, List lines, s lines.Add(""); foreach (var (nodeId, _, _) in fanInDescriptors) { - lines.Add($"{indent}{MapId(nodeId)}((fan-in))"); + lines.Add($"{indent}{GetSafeId(nodeId)}((fan-in))"); } } @@ -184,9 +197,9 @@ private static void EmitWorkflowMermaid(Workflow workflow, List lines, s { foreach (var src in sources) { - lines.Add($"{indent}{MapId(src)} --> {MapId(nodeId)};"); + lines.Add($"{indent}{GetSafeId(src)} --> {GetSafeId(nodeId)};"); } - lines.Add($"{indent}{MapId(nodeId)} --> {MapId(target)};"); + lines.Add($"{indent}{GetSafeId(nodeId)} --> {GetSafeId(target)};"); } // Emit normal edges @@ -197,17 +210,17 @@ private static void EmitWorkflowMermaid(Workflow workflow, List lines, s string effectiveLabel = label != null ? EscapeMermaidLabel(label) : "conditional"; // Conditional edge, with user label or default - lines.Add($"{indent}{MapId(src)} -. {effectiveLabel} .--> {MapId(target)};"); + lines.Add($"{indent}{GetSafeId(src)} -. {effectiveLabel} .-> {GetSafeId(target)};"); } else if (label != null) { // Regular edge with label - lines.Add($"{indent}{MapId(src)} -->|{EscapeMermaidLabel(label)}| {MapId(target)};"); + lines.Add($"{indent}{GetSafeId(src)} -->|{EscapeMermaidLabel(label)}| {GetSafeId(target)};"); } else { // Regular edge without label - lines.Add($"{indent}{MapId(src)} --> {MapId(target)};"); + lines.Add($"{indent}{GetSafeId(src)} --> {GetSafeId(target)};"); } } } diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs index f6740cc48e..3c76d93729 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs @@ -270,9 +270,9 @@ public void Test_WorkflowViz_ToMermaidString_Basic() // Check that the Mermaid content contains expected elements mermaidContent.Should().Contain("flowchart TD"); - mermaidContent.Should().Contain("executor1[\"executor1 (Start)\"]"); - mermaidContent.Should().Contain("executor2[\"executor2\"]"); - mermaidContent.Should().Contain("executor1 --> executor2"); + mermaidContent.Should().Contain("node_0[\"executor1 (Start)\"]"); + mermaidContent.Should().Contain("node_1[\"executor2\"]"); + mermaidContent.Should().Contain("node_0 --> node_1"); } [Fact] @@ -292,11 +292,14 @@ public void Test_WorkflowViz_Mermaid_Conditional_Edge() var mermaidContent = workflow.ToMermaidString(); - // Conditional edge should be dotted with label - mermaidContent.Should().Contain("start -. conditional .--> mid"); + // Conditional edge should be dotted with label (using .-> not .-->) + mermaidContent.Should().Contain("-. conditional .-> "); // Non-conditional edge should be solid - mermaidContent.Should().Contain("mid --> end"); - mermaidContent.Should().NotContain("end -. conditional"); + mermaidContent.Should().Contain(" --> "); + // Display labels should be present + mermaidContent.Should().Contain("\"start (Start)\""); + mermaidContent.Should().Contain("\"mid\""); + mermaidContent.Should().Contain("\"end\""); } [Fact] @@ -316,23 +319,25 @@ public void Test_WorkflowViz_Mermaid_FanIn_EdgeGroup() var mermaidContent = workflow.ToMermaidString(); // There should be a fan-in node with special styling - var lines = mermaidContent.Split('\n'); - var fanInLines = Array.FindAll(lines, line => line.Contains("((fan-in))")); - fanInLines.Should().HaveCount(1); - - // Extract the intermediate node id from the line - var fanInLine = fanInLines[0].Trim(); - var fanInNodeId = fanInLine.Substring(0, fanInLine.IndexOf("((fan-in))", StringComparison.Ordinal)).Trim(); - fanInNodeId.Should().NotBeNullOrEmpty(); - - // Edges should be routed through the intermediate node - mermaidContent.Should().Contain($"s1 --> {fanInNodeId}"); - mermaidContent.Should().Contain($"s2 --> {fanInNodeId}"); - mermaidContent.Should().Contain($"{fanInNodeId} --> t"); + mermaidContent.Should().Contain("((fan-in))"); - // Ensure direct edges are not present - mermaidContent.Should().NotContain("s1 --> t"); - mermaidContent.Should().NotContain("s2 --> t"); + // Display labels should be present + mermaidContent.Should().Contain("\"start (Start)\""); + mermaidContent.Should().Contain("\"s1\""); + mermaidContent.Should().Contain("\"s2\""); + mermaidContent.Should().Contain("\"t\""); + + // All node IDs should be safe aliases (no raw executor names as IDs) + foreach (var line in mermaidContent.Split('\n')) + { + var trimmed = line.Trim(); + if (trimmed.Contains("[\"") || trimmed.Contains("((")) + { + var bracketIdx = trimmed.IndexOfAny(['[', '(']); + var nodeId = trimmed.Substring(0, bracketIdx); + nodeId.Should().MatchRegex("^node_\\d+$"); + } + } } [Fact] @@ -353,17 +358,16 @@ public void Test_WorkflowViz_Mermaid_Complex_Workflow() var mermaidContent = workflow.ToMermaidString(); - // Check all executors are present - mermaidContent.Should().Contain("start[\"start (Start)\"]"); - mermaidContent.Should().Contain("middle1[\"middle1\"]"); - mermaidContent.Should().Contain("middle2[\"middle2\"]"); - mermaidContent.Should().Contain("end[\"end\"]"); + // Check display labels are present + mermaidContent.Should().Contain("\"start (Start)\""); + mermaidContent.Should().Contain("\"middle1\""); + mermaidContent.Should().Contain("\"middle2\""); + mermaidContent.Should().Contain("\"end\""); - // Check all edges are present - mermaidContent.Should().Contain("start --> middle1"); - mermaidContent.Should().Contain("start --> middle2"); - mermaidContent.Should().Contain("middle1 --> end"); - mermaidContent.Should().Contain("middle2 --> end"); + // Check that safe aliases are used and all edges connect them + mermaidContent.Should().Contain("node_0[\"start (Start)\"]"); + mermaidContent.Should().Contain("node_0 --> node_1"); + mermaidContent.Should().Contain("node_0 --> node_2"); } [Fact] @@ -386,15 +390,19 @@ public void Test_WorkflowViz_Mermaid_Mixed_EdgeTypes() var mermaidContent = workflow.ToMermaidString(); - // Check conditional edge - mermaidContent.Should().Contain("start -. conditional .--> a"); - - // Check fan-out edges - mermaidContent.Should().Contain("a --> b"); - mermaidContent.Should().Contain("a --> c"); + // Check conditional edge uses correct syntax (.-> not .-->) + mermaidContent.Should().Contain("-. conditional .->"); + mermaidContent.Should().NotContain(".-->"); // Check fan-in (should have intermediate node) mermaidContent.Should().Contain("((fan-in))"); + + // Display labels should be present + mermaidContent.Should().Contain("\"start (Start)\""); + mermaidContent.Should().Contain("\"a\""); + mermaidContent.Should().Contain("\"b\""); + mermaidContent.Should().Contain("\"c\""); + mermaidContent.Should().Contain("\"end\""); } [Fact] @@ -411,7 +419,7 @@ public void Test_WorkflowViz_Mermaid_Edge_Label_With_Pipe() var mermaidContent = workflow.ToMermaidString(); // Should escape pipe character - mermaidContent.Should().Contain("start -->|High | Low Priority| end"); + mermaidContent.Should().Contain("-->|High | Low Priority|"); // Should not contain unescaped pipe that would break syntax mermaidContent.Should().NotContain("-->|High | Low"); } @@ -453,4 +461,88 @@ public void Test_WorkflowViz_Mermaid_Edge_Label_With_Newline() // Should not contain literal newline in the label (but the overall output has newlines between statements) mermaidContent.Should().NotContain("Line 1\nLine 2"); } + + [Fact] + public void Test_WorkflowViz_Mermaid_ConditionalEdge_ArrowSyntax_Issue1406() + { + // Issue #1406: Conditional edges produce ".-->" which is invalid Mermaid syntax. + // The correct Mermaid syntax for a dotted arrow with label is "-. label .->" (not ".-->"). + var start = new MockExecutor("start"); + var mid = new MockExecutor("mid"); + + static bool Condition(string? msg) => msg == "foo"; + + var workflow = new WorkflowBuilder("start") + .AddEdge(start, mid, Condition) + .Build(); + + var mermaidContent = workflow.ToMermaidString(); + + // The output should use ".->" not ".-->" for conditional (dotted) edges + mermaidContent.Should().NotContain(".-->", because: "'.-->' is invalid Mermaid syntax for dotted arrows; should be '.->'"); + mermaidContent.Should().Contain("-. conditional .->", because: "'-. label .->' is the correct Mermaid syntax for dotted arrows with labels"); + } + + [Fact] + public void Test_WorkflowViz_Mermaid_IdentifiersWithSpaces_Issue1406() + { + // Issue #1406: Identifiers with spaces are used directly as Mermaid node IDs, + // which causes rendering errors. Mermaid node IDs should not contain spaces. + var executor1 = new MockExecutor("1. User input"); + var executor2 = new MockExecutor("2. Process data"); + + var workflow = new WorkflowBuilder("1. User input") + .AddEdge(executor1, executor2) + .Build(); + + var mermaidContent = workflow.ToMermaidString(); + + // Node definitions should use safe aliases as IDs (no spaces), with display names in quotes + // Bad: '1. User input["1. User input (Start)"]' — spaces in ID break Mermaid + // Good: 'executor0["1. User input (Start)"]' — alias ID is safe + + // Each node definition line (containing ["..."]) should have a space-free ID before the bracket + foreach (var line in mermaidContent.Split('\n')) + { + var trimmed = line.Trim(); + if (trimmed.Contains("[\"")) + { + var bracketIdx = trimmed.IndexOf('['); + var nodeId = trimmed.Substring(0, bracketIdx); + nodeId.Should().NotContain(" ", because: $"Mermaid node IDs must not contain spaces, but got '{nodeId}'"); + } + } + } + + [Fact] + public void Test_WorkflowViz_Mermaid_IdentifiersWithUnicode_Issue1406() + { + // Issue #1406: Non-ASCII characters (e.g. Japanese) in identifiers cause Mermaid rendering errors. + var executor1 = new MockExecutor("ユーザー入力"); + var executor2 = new MockExecutor("データ処理"); + + var workflow = new WorkflowBuilder("ユーザー入力") + .AddEdge(executor1, executor2) + .Build(); + + var mermaidContent = workflow.ToMermaidString(); + + // The display labels should contain the original names + mermaidContent.Should().Contain("ユーザー入力"); + mermaidContent.Should().Contain("データ処理"); + + // But node IDs (before the bracket) should be safe ASCII-only identifiers + foreach (var line in mermaidContent.Split('\n')) + { + var trimmed = line.Trim(); + if (trimmed.Contains("[\"")) + { + var bracketIdx = trimmed.IndexOf('['); + var nodeId = trimmed.Substring(0, bracketIdx); + // Node ID should only contain ASCII alphanumeric and underscores + nodeId.Should().MatchRegex("^[a-zA-Z0-9_]+$", + because: $"Mermaid node IDs should be ASCII-safe, but got '{nodeId}'"); + } + } + } } From be436dd1b6f458a3b6a865f65da7fd74c86aac4e Mon Sep 17 00:00:00 2001 From: alliscode Date: Wed, 25 Feb 2026 11:02:24 -0800 Subject: [PATCH 2/5] Use recognizable sanitized IDs for Mermaid node identifiers\n\nReplace generic node_0/node_1 aliases with IDs derived from the original\nexecutor names. ASCII letters, digits, and underscores are preserved;\nother characters become underscores (collapsed, trimmed). Leading digits\nget an n_ prefix. Collisions are resolved with a numeric suffix.\n\nThis keeps node IDs readable in the Mermaid source while the display\nlabels continue to show the full original names." --- .../Visualization/WorkflowVisualizer.cs | 63 ++++++++++++++++++- .../WorkflowVisualizerTests.cs | 18 +++--- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs index 7ef0f63f55..33f0862014 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs @@ -153,15 +153,31 @@ private static void EmitSubWorkflowsDigraph(Workflow workflow, List line private static void EmitWorkflowMermaid(Workflow workflow, List lines, string indent, string? ns = null) { - // Build a mapping from raw IDs to Mermaid-safe node aliases. - // Mermaid node IDs cannot contain spaces, dots, or non-ASCII characters. + // Build a mapping from raw IDs to Mermaid-safe node aliases that preserve + // as much of the original ID as possible for readability. + // Mermaid node IDs cannot contain spaces, dots, pipes, or most special characters. var aliasMap = new Dictionary(); + var usedAliases = new HashSet(StringComparer.Ordinal); + string GetSafeId(string id) { var key = ns != null ? $"{ns}/{id}" : id; if (!aliasMap.TryGetValue(key, out var alias)) { - alias = $"node_{aliasMap.Count}"; + alias = SanitizeMermaidNodeId(key); + + // Handle collisions by appending a numeric suffix + if (!usedAliases.Add(alias)) + { + var i = 2; + while (!usedAliases.Add($"{alias}_{i}")) + { + i++; + } + + alias = $"{alias}_{i}"; + } + aliasMap[key] = alias; } @@ -314,6 +330,47 @@ private static bool TryGetNestedWorkflow(ExecutorBinding binding, [NotNullWhen(t return false; } + /// + /// Converts a raw node ID into a Mermaid-safe identifier that preserves as much + /// of the original text as possible. ASCII letters, digits, and underscores are kept; + /// everything else (including non-ASCII letters) is replaced with an underscore. + /// A leading digit gets a prefix. Consecutive underscores are collapsed. + /// + private static string SanitizeMermaidNodeId(string id) + { + var sb = new StringBuilder(id.Length); + bool lastWasUnderscore = false; + foreach (var ch in id) + { + bool isAsciiSafe = (ch >= 'a' && ch <= 'z') || (ch >= 'A' && ch <= 'Z') || (ch >= '0' && ch <= '9') || ch == '_'; + if (isAsciiSafe) + { + sb.Append(ch); + lastWasUnderscore = ch == '_'; + } + else if (!lastWasUnderscore) + { + sb.Append('_'); + lastWasUnderscore = true; + } + } + + // Trim trailing underscore + while (sb.Length > 0 && sb[sb.Length - 1] == '_') + { + sb.Length--; + } + + // Mermaid IDs must not start with a digit + if (sb.Length > 0 && sb[0] >= '0' && sb[0] <= '9') + { + sb.Insert(0, "n_"); + } + + // Guard against empty result (e.g. id was all special chars) + return sb.Length == 0 ? "node" : sb.ToString(); + } + // Helper method to escape special characters in DOT labels private static string EscapeDotLabel(string label) { diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs index 3c76d93729..9433d2bd32 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs @@ -270,9 +270,9 @@ public void Test_WorkflowViz_ToMermaidString_Basic() // Check that the Mermaid content contains expected elements mermaidContent.Should().Contain("flowchart TD"); - mermaidContent.Should().Contain("node_0[\"executor1 (Start)\"]"); - mermaidContent.Should().Contain("node_1[\"executor2\"]"); - mermaidContent.Should().Contain("node_0 --> node_1"); + mermaidContent.Should().Contain("executor1[\"executor1 (Start)\"]"); + mermaidContent.Should().Contain("executor2[\"executor2\"]"); + mermaidContent.Should().Contain("executor1 --> executor2"); } [Fact] @@ -327,7 +327,7 @@ public void Test_WorkflowViz_Mermaid_FanIn_EdgeGroup() mermaidContent.Should().Contain("\"s2\""); mermaidContent.Should().Contain("\"t\""); - // All node IDs should be safe aliases (no raw executor names as IDs) + // All node IDs should be safe aliases (ASCII-only identifiers) foreach (var line in mermaidContent.Split('\n')) { var trimmed = line.Trim(); @@ -335,7 +335,7 @@ public void Test_WorkflowViz_Mermaid_FanIn_EdgeGroup() { var bracketIdx = trimmed.IndexOfAny(['[', '(']); var nodeId = trimmed.Substring(0, bracketIdx); - nodeId.Should().MatchRegex("^node_\\d+$"); + nodeId.Should().MatchRegex("^[a-zA-Z_][a-zA-Z0-9_]*$"); } } } @@ -364,10 +364,10 @@ public void Test_WorkflowViz_Mermaid_Complex_Workflow() mermaidContent.Should().Contain("\"middle2\""); mermaidContent.Should().Contain("\"end\""); - // Check that safe aliases are used and all edges connect them - mermaidContent.Should().Contain("node_0[\"start (Start)\"]"); - mermaidContent.Should().Contain("node_0 --> node_1"); - mermaidContent.Should().Contain("node_0 --> node_2"); + // Check that sanitized IDs are used and all edges connect them + mermaidContent.Should().Contain("start[\"start (Start)\"]"); + mermaidContent.Should().Contain("start --> middle1"); + mermaidContent.Should().Contain("start --> middle2"); } [Fact] From d73fd3e2e7fdda5367cefb2fbffec495bb88e54e Mon Sep 17 00:00:00 2001 From: alliscode Date: Wed, 25 Feb 2026 11:07:20 -0800 Subject: [PATCH 3/5] Remove issue number references from test names and comments" --- .../WorkflowVisualizerTests.cs | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs index 9433d2bd32..34709bb69c 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs @@ -463,10 +463,10 @@ public void Test_WorkflowViz_Mermaid_Edge_Label_With_Newline() } [Fact] - public void Test_WorkflowViz_Mermaid_ConditionalEdge_ArrowSyntax_Issue1406() + public void Test_WorkflowViz_Mermaid_ConditionalEdge_ArrowSyntax() { - // Issue #1406: Conditional edges produce ".-->" which is invalid Mermaid syntax. - // The correct Mermaid syntax for a dotted arrow with label is "-. label .->" (not ".-->"). + // Conditional edges must use "-. label .->" (not ".-->") which is the correct + // Mermaid syntax for dotted arrows with labels. var start = new MockExecutor("start"); var mid = new MockExecutor("mid"); @@ -484,10 +484,10 @@ public void Test_WorkflowViz_Mermaid_ConditionalEdge_ArrowSyntax_Issue1406() } [Fact] - public void Test_WorkflowViz_Mermaid_IdentifiersWithSpaces_Issue1406() + public void Test_WorkflowViz_Mermaid_IdentifiersWithSpaces() { - // Issue #1406: Identifiers with spaces are used directly as Mermaid node IDs, - // which causes rendering errors. Mermaid node IDs should not contain spaces. + // Identifiers with spaces must not be used directly as Mermaid node IDs + // because spaces cause rendering errors. var executor1 = new MockExecutor("1. User input"); var executor2 = new MockExecutor("2. Process data"); @@ -515,9 +515,9 @@ public void Test_WorkflowViz_Mermaid_IdentifiersWithSpaces_Issue1406() } [Fact] - public void Test_WorkflowViz_Mermaid_IdentifiersWithUnicode_Issue1406() + public void Test_WorkflowViz_Mermaid_IdentifiersWithUnicode() { - // Issue #1406: Non-ASCII characters (e.g. Japanese) in identifiers cause Mermaid rendering errors. + // Non-ASCII characters (e.g. Japanese) in identifiers cause Mermaid rendering errors. var executor1 = new MockExecutor("ユーザー入力"); var executor2 = new MockExecutor("データ処理"); From 519afc07bac4085f181c1c1ee78f990827ddebe7 Mon Sep 17 00:00:00 2001 From: alliscode Date: Wed, 25 Feb 2026 11:21:08 -0800 Subject: [PATCH 4/5] Address PR review feedback from Copilot\n\n- Add Throw.IfNull(id) guard to SanitizeMermaidNodeId\n- Add safety limit (10,000) to collision resolution loop\n- Restore missing edge assertions (middle1/middle2 --> end)\n- Fix comment to show actual sanitized ID (n_1_User_input)\n- Use stricter regex in Unicode test (must start with letter/underscore)" --- .../Visualization/WorkflowVisualizer.cs | 7 +++++++ .../WorkflowVisualizerTests.cs | 8 +++++--- 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs index 33f0862014..486683a691 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs @@ -172,6 +172,11 @@ string GetSafeId(string id) var i = 2; while (!usedAliases.Add($"{alias}_{i}")) { + if (i >= 10_000) + { + throw new InvalidOperationException($"Unable to generate a unique Mermaid node ID for '{key}'."); + } + i++; } @@ -338,6 +343,8 @@ private static bool TryGetNestedWorkflow(ExecutorBinding binding, [NotNullWhen(t /// private static string SanitizeMermaidNodeId(string id) { + Throw.IfNull(id); + var sb = new StringBuilder(id.Length); bool lastWasUnderscore = false; foreach (var ch in id) diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs index 34709bb69c..267717ea0c 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs @@ -368,6 +368,8 @@ public void Test_WorkflowViz_Mermaid_Complex_Workflow() mermaidContent.Should().Contain("start[\"start (Start)\"]"); mermaidContent.Should().Contain("start --> middle1"); mermaidContent.Should().Contain("start --> middle2"); + mermaidContent.Should().Contain("middle1 --> end"); + mermaidContent.Should().Contain("middle2 --> end"); } [Fact] @@ -499,7 +501,7 @@ public void Test_WorkflowViz_Mermaid_IdentifiersWithSpaces() // Node definitions should use safe aliases as IDs (no spaces), with display names in quotes // Bad: '1. User input["1. User input (Start)"]' — spaces in ID break Mermaid - // Good: 'executor0["1. User input (Start)"]' — alias ID is safe + // Good: 'n_1_User_input["1. User input (Start)"]' — alias ID is safe and sanitized // Each node definition line (containing ["..."]) should have a space-free ID before the bracket foreach (var line in mermaidContent.Split('\n')) @@ -539,8 +541,8 @@ public void Test_WorkflowViz_Mermaid_IdentifiersWithUnicode() { var bracketIdx = trimmed.IndexOf('['); var nodeId = trimmed.Substring(0, bracketIdx); - // Node ID should only contain ASCII alphanumeric and underscores - nodeId.Should().MatchRegex("^[a-zA-Z0-9_]+$", + // Node ID should start with a letter or underscore, followed by ASCII alphanumeric or underscores + nodeId.Should().MatchRegex("^[a-zA-Z_][a-zA-Z0-9_]*$", because: $"Mermaid node IDs should be ASCII-safe, but got '{nodeId}'"); } } From 9825b8078d7e8bb7d49b4319097497eb1ee6d2ba Mon Sep 17 00:00:00 2001 From: alliscode Date: Wed, 25 Feb 2026 11:53:25 -0800 Subject: [PATCH 5/5] Address second round of PR review feedback\n\n- Escape node display labels via EscapeMermaidLabel to handle quotes,\n brackets, and newlines in executor IDs\n- Fix XML doc on SanitizeMermaidNodeId to accurately describe that\n existing consecutive underscores in input are preserved\n- Restore specific edge assertion (mid --> end) in conditional edge test\n- Restore fan-in routing assertions (s1/s2 through intermediate node,\n no direct edges to t) in fan-in test" --- .../Visualization/WorkflowVisualizer.cs | 11 +++++----- .../WorkflowVisualizerTests.cs | 22 ++++++++++++++++--- 2 files changed, 25 insertions(+), 8 deletions(-) diff --git a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs index 486683a691..d09273fbc1 100644 --- a/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs +++ b/dotnet/src/Microsoft.Agents.AI.Workflows/Visualization/WorkflowVisualizer.cs @@ -191,14 +191,14 @@ string GetSafeId(string id) // Add start node var startExecutorId = workflow.StartExecutorId; - lines.Add($"{indent}{GetSafeId(startExecutorId)}[\"{startExecutorId} (Start)\"];"); + lines.Add($"{indent}{GetSafeId(startExecutorId)}[\"{EscapeMermaidLabel(startExecutorId)} (Start)\"];"); // Add other executor nodes foreach (var executorId in workflow.ExecutorBindings.Keys) { if (executorId != startExecutorId) { - lines.Add($"{indent}{GetSafeId(executorId)}[\"{executorId}\"];"); + lines.Add($"{indent}{GetSafeId(executorId)}[\"{EscapeMermaidLabel(executorId)}\"];"); } } @@ -337,9 +337,10 @@ private static bool TryGetNestedWorkflow(ExecutorBinding binding, [NotNullWhen(t /// /// Converts a raw node ID into a Mermaid-safe identifier that preserves as much - /// of the original text as possible. ASCII letters, digits, and underscores are kept; - /// everything else (including non-ASCII letters) is replaced with an underscore. - /// A leading digit gets a prefix. Consecutive underscores are collapsed. + /// of the original text as possible. ASCII letters, digits, and underscores are kept + /// as-is (including existing consecutive underscores). All other characters (including + /// non-ASCII letters) are replaced with underscores, with consecutive invalid characters + /// collapsed into a single underscore. A leading digit gets a prefix. /// private static string SanitizeMermaidNodeId(string id) { diff --git a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs index 267717ea0c..c8cf2cf214 100644 --- a/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs +++ b/dotnet/tests/Microsoft.Agents.AI.Workflows.UnitTests/WorkflowVisualizerTests.cs @@ -294,8 +294,8 @@ public void Test_WorkflowViz_Mermaid_Conditional_Edge() // Conditional edge should be dotted with label (using .-> not .-->) mermaidContent.Should().Contain("-. conditional .-> "); - // Non-conditional edge should be solid - mermaidContent.Should().Contain(" --> "); + // Non-conditional edge should be a specific solid arrow + mermaidContent.Should().Contain("mid --> end"); // Display labels should be present mermaidContent.Should().Contain("\"start (Start)\""); mermaidContent.Should().Contain("\"mid\""); @@ -319,7 +319,23 @@ public void Test_WorkflowViz_Mermaid_FanIn_EdgeGroup() var mermaidContent = workflow.ToMermaidString(); // There should be a fan-in node with special styling - mermaidContent.Should().Contain("((fan-in))"); + var lines = mermaidContent.Split('\n'); + var fanInLines = Array.FindAll(lines, line => line.Contains("((fan-in))")); + fanInLines.Should().HaveCount(1); + + // Extract the intermediate fan-in node id from the line + var fanInLine = fanInLines[0].Trim(); + var fanInNodeId = fanInLine.Substring(0, fanInLine.IndexOf("((fan-in))", StringComparison.Ordinal)).Trim(); + fanInNodeId.Should().NotBeNullOrEmpty(); + + // Edges should be routed through the intermediate node + mermaidContent.Should().Contain($"s1 --> {fanInNodeId}"); + mermaidContent.Should().Contain($"s2 --> {fanInNodeId}"); + mermaidContent.Should().Contain($"{fanInNodeId} --> t"); + + // Ensure direct edges are not present + mermaidContent.Should().NotContain("s1 --> t"); + mermaidContent.Should().NotContain("s2 --> t"); // Display labels should be present mermaidContent.Should().Contain("\"start (Start)\"");