From 9477081ccd334537072d7f3dffe5e05be0f73146 Mon Sep 17 00:00:00 2001 From: Eric Traut Date: Wed, 23 Sep 2026 21:35:29 +0000 Subject: [PATCH] Support quoted labels and ampersands in Mermaid flowcharts (#47678) ## Why Flowcharts with labels such as `A["Review & confirm"]` fell back to source display instead of rendering a diagram. ## What changed Accept double-quoted node and edge labels, strip the surrounding quotes, and allow literal ampersands in flowchart labels. Keep other diagram families' label restrictions and continue rejecting HTML, entity escapes, and malformed quotes. ## Testing Add parser and renderer regression tests for quoted labels, literal ampersands, statement separators, and unsupported text, plus TUI coverage for rendered labels and entity source fallback. GitOrigin-RevId: 9e61a04665f0266d369fa3595411c9a72f10cb9d --- codex-rs/mermaid/README.md | 4 ++ codex-rs/mermaid/src/parse.rs | 24 ++++++- codex-rs/mermaid/src/tests.rs | 67 ++++++++++++++++++- .../tui/src/markdown_render/mermaid_tests.rs | 21 ++++++ ..._mermaid_quoted_labels_and_ampersands.snap | 14 ++++ 5 files changed, 126 insertions(+), 4 deletions(-) create mode 100644 codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__mermaid__tests__mermaid_quoted_labels_and_ampersands.snap diff --git a/codex-rs/mermaid/README.md b/codex-rs/mermaid/README.md index c5a6eca663..d7384ede4b 100644 --- a/codex-rs/mermaid/README.md +++ b/codex-rs/mermaid/README.md @@ -22,6 +22,10 @@ example `class Order {` followed by member lines and a final `}`). Class member is retained in one compartment, including visibility, signatures, and return types. ER attribute types and names use the identifier grammar above. +Flowchart node and edge labels may be enclosed in double quotes. Literal ampersands +are supported in these flowchart labels; other diagram families retain their stricter +label subset. HTML and entity escapes remain unsupported. + These are explicit subsets, not complete Mermaid compatibility. Compound states, flowchart subgraphs, other shapes, sequence activation and parallel fragments, styling, front matter, directives, HTML, escapes, combining/zero-width characters, diff --git a/codex-rs/mermaid/src/parse.rs b/codex-rs/mermaid/src/parse.rs index b9cbaf7e2c..76049220f6 100644 --- a/codex-rs/mermaid/src/parse.rs +++ b/codex-rs/mermaid/src/parse.rs @@ -30,7 +30,7 @@ pub(super) fn parse(header: &str, body: &[&str]) -> Result { rest = rest.trim_start(); let label = if let Some(after) = rest.strip_prefix('|') { let (label, remaining) = after.split_once('|').ok_or(RenderError::Unsupported)?; - check_label(label)?; + let label = flowchart_label(label)?; rest = remaining; label.to_owned() } else { @@ -70,7 +70,7 @@ fn node(rest: &mut &str, graph: &mut Graph) -> Result { let (label, remaining) = rest[open.len()..] .split_once(close) .ok_or(RenderError::Unsupported)?; - check_label(label)?; + let label = flowchart_label(label)?; *rest = remaining; let node = &mut graph.nodes[index]; if node.declared && (node.label != label || node.shape != shape) { @@ -83,7 +83,26 @@ fn node(rest: &mut &str, graph: &mut Graph) -> Result { Ok(index) } +fn flowchart_label(label: &str) -> Result<&str, RenderError> { + let label = if let Some(quoted) = label.strip_prefix('"') { + quoted.strip_suffix('"').ok_or(RenderError::Unsupported)? + } else { + label + }; + check_label_text(label)?; + Ok(label) +} + pub(super) fn check_label(label: &str) -> Result<(), RenderError> { + // Only delimited flowchart labels support literal ampersands. Other families must keep + // rejecting them so statement splitting cannot turn an entity into truncated label text. + if label.contains('&') { + return Err(RenderError::Unsupported); + } + check_label_text(label) +} + +fn check_label_text(label: &str) -> Result<(), RenderError> { if label.trim().is_empty() || label.chars().any(|ch| { ch.is_control() @@ -95,7 +114,6 @@ pub(super) fn check_label(label: &str) -> Result<(), RenderError> { | '|' | '<' | '>' - | '&' | '"' | '\\' | '┌' diff --git a/codex-rs/mermaid/src/tests.rs b/codex-rs/mermaid/src/tests.rs index 6ea97ff846..a81d8ffef6 100644 --- a/codex-rs/mermaid/src/tests.rs +++ b/codex-rs/mermaid/src/tests.rs @@ -35,6 +35,72 @@ fn stadiums_with_other_shapes_in_every_direction() { assert_snapshot!(cases.join("\n\n")); } +#[test] +fn quoted_flowchart_labels_match_unquoted_labels() { + let quoted = r#"A["Review & confirm"] -->|"Yes & continue"| B{"Ready?"} +B --> C(["Checkout"]) +A[Review & confirm]"#; + let unquoted = quoted.replace('"', ""); + assert_eq!( + super::parse::parse("flowchart TD", "ed.lines().collect::>()).unwrap(), + super::parse::parse("flowchart TD", &unquoted.lines().collect::>()).unwrap(), + ); +} + +#[test] +fn entity_labels_keep_source_fallback() { + for entity in ["&", "&", "&"] { + for source in [ + format!("sequenceDiagram\nA->>B: {entity}"), + format!("flowchart TD; A[\"{entity}\"]"), + format!("flowchart TD; A -->|\"{entity}\"| B"), + ] { + assert_eq!( + render(&source, /*max_width*/ 100), + Err(RenderError::Unsupported), + "{source:?}", + ); + } + } +} + +#[test] +fn flowchart_ampersands_preserve_statement_separators() { + let source = "flowchart TD; A[R&D] -->|R&D| B[Review & confirm]; B --> C"; + assert_eq!( + render(&format!("%% &\n{source}"), /*max_width*/ 100), + render(&source.replace(';', "\n"), /*max_width*/ 100), + ); + assert!( + render(source, /*max_width*/ 100) + .unwrap() + .contains("Review & confirm") + ); +} + +#[test] +fn quoted_flowchart_labels_reject_malformed_and_unsafe_text() { + for label in [ + "\"unfinished", + "unfinished\"", + "\"embedded\"quote\"", + "\"\"", + "\"HTML\"", + "\"\u{1b}\"", + ] { + for source in [ + format!("flowchart TD; A[{label}]"), + format!("flowchart TD; A -->|{label}| B"), + ] { + assert_eq!( + render(&source, /*max_width*/ 100), + Err(RenderError::Unsupported), + "{source:?}", + ); + } + } +} + #[test] fn branches_merges_and_retry_loop() { let source = "flowchart TD\nA[Checkout] --> B{In stock?}\nB -->|yes| C[Reserve]\nB -->|no| D[Waitlist]\nC --> E{Paid?}\nE -->|yes| F[Ship]\nE -->|no| G[Retry payment]\nG --> E\nD --> H[Notify buyer]\nF --> H"; @@ -72,7 +138,6 @@ fn rejects_partial_or_unsupported_input() { "flowchart TD; A([same]); A[same]", "flowchart TD; A{same}; A([same])", "flowchart TD; A -->|unclosed B", - "flowchart TD; A[\"quoted\"]", "flowchart TD; A[foo;bar]", "flowchart TD; A[لا]", "flowchart TD; A -->|yes┐| B", diff --git a/codex-rs/tui/src/markdown_render/mermaid_tests.rs b/codex-rs/tui/src/markdown_render/mermaid_tests.rs index 29b438edfc..5fe87a8c49 100644 --- a/codex-rs/tui/src/markdown_render/mermaid_tests.rs +++ b/codex-rs/tui/src/markdown_render/mermaid_tests.rs @@ -41,6 +41,27 @@ fn mermaid_nested_fences_and_unicode() { assert_snapshot!(markdown_text(source, /*width*/ 60)); } +#[test] +fn mermaid_quoted_labels_and_ampersands() { + let source = r#"```mermaid +flowchart LR + A["Your saved order"] --> B["Review & confirm"] --> C["DoorDash checkout"] +```"#; + let output = markdown_text(source, /*width*/ 100); + assert!(output.starts_with('┌')); + assert_snapshot!(output); +} + +#[test] +fn mermaid_entities_keep_source() { + let source = "```mermaid\nsequenceDiagram\nA->>B: &\n```"; + let output = markdown_text(source, /*width*/ 100); + assert!(output.ends_with(&markdown_text( + &source.replacen("mermaid", "unknown", /*count*/ 1), + /*width*/ 100, + ))); +} + #[test] fn mermaid_stadium_flowchart() { let source = "```mermaid diff --git a/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__mermaid__tests__mermaid_quoted_labels_and_ampersands.snap b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__mermaid__tests__mermaid_quoted_labels_and_ampersands.snap new file mode 100644 index 0000000000..4ab47bc1f5 --- /dev/null +++ b/codex-rs/tui/src/markdown_render/snapshots/codex_tui__markdown_render__mermaid__tests__mermaid_quoted_labels_and_ampersands.snap @@ -0,0 +1,14 @@ +--- +source: tui/src/markdown_render/mermaid_tests.rs +expression: output +--- +┌──────────────────┐ ┌──────────────────┐ ┌───────────────────┐ +│ Your saved order │ │ Review & confirm │ │ DoorDash checkout │ +└┬─────────────────┘ └┬─┬───────────────┘ └┬──────────────────┘ + │ ▲ │ ▲ + │ │ │ │ + │ │ │ │ + │ │ │ │ + └─────────────────────┘ │ │ + │ │ + └───────────────────┘