fix(serve): mermaid fenced blocks broken by HTML-entity escaping#273
Merged
Conversation
`render_markdown` injected synthetic `<pre class="mermaid">` wrappers but
still let the fenced body flow through pulldown-cmark's `html::push_html`,
which HTML-entity-escapes Event::Text — so mermaid's core `-->` arrow
rendered as `-->` (and class-diagram `<|--` as `<|--`), and
mermaid.js rejected the diagram with "Syntax error in text". Reported by
a user against v0.6.0 and v0.8.0.
Re-tag the in-mermaid Text segments as Event::Html so they pass through
unescaped. Mermaid diagram source contains no HTML tags; a `</pre>`
smuggled into a ```mermaid block would at worst close the block early,
and `<script>` etc. are still stripped by the `sanitize_html` pass.
(`document.rs`'s separate document renderer already emits the body
verbatim via `join("\n")` — only `render_markdown` had the bug.)
Coverage:
- two markdown.rs unit tests: stateDiagram `[*] -->` arrows survive
verbatim with no `-->`; classDiagram `<|--` survives with no `<|--`.
- new tests/playwright/mermaid.spec.ts: ARCH-CORE-001's embedded
```mermaid flowchart serves with `Config --> Store` verbatim, and
mermaid.js renders it to an SVG with no "Syntax error" box.
Fixes REQ-032.
Implements: REQ-032
Verifies: REQ-032
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
📐 Rivet artifact deltaNo artifact changes in this PR. Code-only changes (renderer, CLI wiring, tests) don't touch the artifact graph. |
Wraps an over-long assert!() the formatter flagged. No behaviour change. Trace: skip Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
⚠️ Performance Alert ⚠️
Possible performance regression was detected for benchmark 'Rivet Criterion Benchmarks'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 1.20.
| Benchmark suite | Current: 90bb1a1 | Previous: 02b9328 | Ratio |
|---|---|---|---|
validate/10000 |
20318603 ns/iter (± 1714130) |
16904098 ns/iter (± 1013276) |
1.20 |
query/10000 |
138521 ns/iter (± 410) |
91811 ns/iter (± 267) |
1.51 |
This comment was automatically generated by workflow using github-action-benchmark.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
4 tasks
avrabe
added a commit
that referenced
this pull request
May 15, 2026
…amilies (#275) * test(serve): broaden mermaid escape-token coverage to multi-diagram families Hardens #273 against regressions across mermaid diagram types beyond the flowchart `-->` and class `<|--` cases the fix originally covered. Adds: - A type-agnostic markdown unit test asserting that escape-sensitive tokens across sequence (`->>`, `-->>`), ER (`||--o{`), class stereotype (`<<async>>`), and flowchart (`&` chains) families all survive the render verbatim, and no `>` / `<` / `&` entity escaping leaks into output. - A second ```mermaid block on ARCH-CORE-001 (stateDiagram-v2) so the end-to-end Playwright fixture exercises a non-flowchart diagram shape — the `[*] -->` token from the original bug report is now covered end-to-end. - A Playwright matrix update asserting both ```mermaid blocks render to SVG (count >= 2) and that `Validating --> Serving : graph built` survives verbatim across diagram families. The fix from #273 stops body escaping wholesale, so the broader coverage is belt-and-braces — a regression in any escape path would now fire loudly across multiple test surfaces. Verifies: REQ-032 Refs: #273 Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * test(playwright): relax artifact-mermaid count to >=1 after dogfood fixture grew The fenced ```mermaid coverage extension in this PR added a second mermaid block to ARCH-CORE-001 (`stateDiagram-v2` alongside the existing `flowchart LR`). That broke `artifacts.spec.ts:58`'s `toHaveCount(1)` assertion, which had hard-coded the assumption that there's exactly one mermaid block in the fixture. Replace the strict `toHaveCount(1)` with a soft `toBeGreaterThanOrEqual(1)` so adding diagrams to the artifact doesn't break this regression test. The test's intent — confirm that .mermaid wrappers are emitted and the first one renders to SVG — is preserved. Verifies: REQ-032 * fix(yaml_hir): block-scalar descriptions mangled by flow-syntax chars PR #275 Playwright failed because rivet serve renders ARCH-CORE-001's state-diagram mermaid as `[*]\n--> Loading\n\nLoading --> Validating\n:\n config + …` — every comma, colon, and `]` becomes a line break. Bundle output is correct; rivet serve is wrong; same source YAML. Root cause: the rowan-based YAML CST lexer at `yaml_cst.rs:401` (`lex_plain_scalar`) breaks plain scalars at `,`, `]`, `}`, and `:` followed by space — these are YAML flow-syntax characters. The break is correct at the YAML grammar level. Inside a `description: |` block scalar, though, those characters are literal text — not delimiters — and shouldn't trigger token boundaries. `block_scalar_text` (`yaml_hir.rs:1451`) compounded the problem: it iterates `BlockScalarLine` *tokens* (one per pre-lexed chunk) and joins them, treating whitespace-only tokens as `\n` separators. So a source line like `Loading --> Validating : config + artifacts read` gets tokenised as roughly `[Loading --> Validating, :, config + ...]`, each chunk becomes a "BlockScalarLine", and the join inserts newlines between them. Bypass the token-iteration path entirely. Use the raw `BlockScalar` node text (`block.text().to_string()`) which is exactly the source slice — including whatever flow-syntax characters appear literally inside the block. Strip the `|` / `>` header line, dedent by the common leading-whitespace prefix, and emit lines joined with `\n` (matches YAML 1.2 default clip-chomping for `|`). The two-path divergence — `rivet bundle` uses `serde_yaml` via the `GenericYamlAdapter` and was always correct; `rivet serve` uses the salsa-tracked `parse_artifacts_v2` which unconditionally calls `yaml_hir::extract_schema_driven` regardless of source format — explains why the bug was latent for so long. Most descriptions don't contain `,` or `:` at flow-syntax positions, so the disagreement was silent. Regression test: `block_scalar_with_flow_syntax_chars_preserves_lines` covers description content with all four trigger characters (`,` `]` `:` `[`) and a state-diagram fragment. The PR #275 Playwright assertions on `[*] --> Loading`, `Validating --> Serving : graph built`, etc. now pass locally — verified by running `rivet serve --port 47293` against the dogfood project and curling `/artifacts/ARCH-CORE-001`. Fixes: REQ-032 Refs: #275 --------- Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
User-reported bug (v0.6.0 + v0.8.0): a
\``mermaidfenced block inside a YAML artifact description renders-->as-->in the dashboard's[*] -->arrows survive verbatim with no-->; classDiagram<|--survives with no<|--./artifacts/ARCH-CORE-001hasConfig --> Storeverbatim and no-->; mermaid.js renders it to an<svg>with no "Syntax error" box.Follow-up (not in this PR)
The fix is diagram-type-agnostic (it just stops escaping the body), so it covers flowchart/state/class/sequence/ER/gantt/etc. at once. A broader Playwright matrix that exercises one fixture artifact per diagram type would be nice belt-and-suspenders but is its own task — flagging per the request to "test all kinds of mermaid graphics".
Test plan
cargo test -p rivet-core markdowngreen (incl. 2 new tests)mermaid.spec.tsgreen in the E2E job🤖 Generated with Claude Code