ci: render every mermaid diagram type against labels that fight the syntax - #147
Merged
Conversation
📝 WalkthroughWalkthroughThis PR adds a Go generator for Mermaid edge-case documents, commits fixtures for 17 diagram types, and documents their inclusion in the existing CI rendering workflow. ChangesMermaid edge-case rendering
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant Generator as doc/edgecase/main.go
participant Fixtures as doc/edgecase/*.md
participant Workflow as doc_render.yml
participant Renderer as Mermaid rendering check
Generator->>Fixtures: generate committed diagrams
Workflow->>Fixtures: select Markdown files with existing glob
Workflow->>Renderer: render diagram fixtures
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
This comment has been minimized.
This comment has been minimized.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
doc/edgecase/main.go (1)
62-107: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winFail when a diagram has no punctuation profile.
supportedreturns""for an unknown key. This is also the intentional value forarchitecture. If a futurediagramsentry has no profile, the generator writes plain labels and CI can pass without edge-case coverage.Return an existence flag, or panic for a missing key. Keep the explicit empty
architecturevalue valid.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@doc/edgecase/main.go` around lines 62 - 107, Update supported so it distinguishes a missing diagram key from the intentional empty profile for architecture; return an existence indicator alongside the profile or otherwise panic when the key is absent, and make the caller validate that indicator before generating labels. Preserve the explicit empty architecture value as valid.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/doc_render.yml:
- Around line 8-12: Update the render-input glob used by the workflow command to
include both .md and .mmd files, preserving the existing recursive discovery so
documentation, edgecase fixtures, and future diagrams are all rendered.
---
Nitpick comments:
In `@doc/edgecase/main.go`:
- Around line 62-107: Update supported so it distinguishes a missing diagram key
from the intentional empty profile for architecture; return an existence
indicator alongside the profile or otherwise panic when the key is absent, and
make the caller validate that indicator before generating labels. Preserve the
explicit empty architecture value as valid.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 5456d89d-2ca0-483e-919d-e600bcb88638
📒 Files selected for processing (19)
.github/workflows/doc_render.ymldoc/edgecase/architecture.mddoc/edgecase/block.mddoc/edgecase/class.mddoc/edgecase/er.mddoc/edgecase/flowchart.mddoc/edgecase/gantt.mddoc/edgecase/gitgraph.mddoc/edgecase/kanban.mddoc/edgecase/main.godoc/edgecase/mindmap.mddoc/edgecase/packet.mddoc/edgecase/piechart.mddoc/edgecase/quadrant.mddoc/edgecase/requirement.mddoc/edgecase/sequence.mddoc/edgecase/state.mddoc/edgecase/userjourney.mddoc/edgecase/xychart.md
Comment on lines
+8
to
+12
| # That covers two sets of diagrams. The samples under doc/ are documentation and | ||
| # read as such, which makes them weak tests: the labels a reader wants to see are | ||
| # the ones that never break a renderer. doc/edgecase/ holds one document per | ||
| # diagram type whose labels are the opposite, so a quoting defect fails here. | ||
| # Both are picked up by the glob below, and so is any diagram added later. |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Include .mmd fixtures in the render input.
The command on Line 58 uses only *.md. It ignores .mmd files. The claim that future diagrams are automatically included is false for .mmd fixtures, and Issue #125 requires both extensions.
Proposed fix
- run: git ls-files -z '*.md' | node scripts/mermaid-check/check.mjs --stdin0
+ run: git ls-files -z -- '*.md' '*.mmd' | node scripts/mermaid-check/check.mjs --stdin0🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In @.github/workflows/doc_render.yml around lines 8 - 12, Update the
render-input glob used by the workflow command to include both .md and .mmd
files, preserving the existing recursive discovery so documentation, edgecase
fixtures, and future diagrams are all rendered.
…yntax The render job already draws every committed diagram, but the diagrams it had to draw were the samples under doc/, which are documentation. That makes them weak tests: the labels a reader wants to see are exactly the ones that never break a renderer, so the job could only catch a defect that broke a plain label. doc/edgecase/ adds one document per diagram type whose labels are the opposite, made of the punctuation that means something to mermaid. One document per type rather than one holding all of them, so a failure names the subpackage instead of a line number that moves whenever a label changes. The existing glob picks them up, and will pick up a new diagram type too. Each type carries the punctuation it can actually take today, measured rather than guessed: every character was put through every type on its own and rendered. What is missing from an entry is a quoting gap, and each one is recorded with its evidence in the generator and in the tracking issue. The entries only ever get harder, so a fix that closes a gap widens the label it is tested with.
The comment explaining the edge case documents was the only change to it, and the glob already picks the documents up, so the workflow needs no edit at all. Removing it also keeps this pull request mergeable without the workflow scope.
nao1215
force-pushed
the
ci/render-every-mermaid-type
branch
from
August 11, 2026 16:27
6e59252 to
39da422
Compare
Code Metrics Report
Details | | main (d75b165) | #147 (7cbbb08) | +/- |
|---------------------|----------------|----------------|------|
| Coverage | 95.9% | 95.9% | 0.0% |
| Files | 57 | 57 | 0 |
| Lines | 2663 | 2663 | 0 |
| Covered | 2555 | 2555 | 0 |
- | Test Execution Time | 5s | 10s | +5s |Reported by octocov |
This was referenced Aug 11, 2026
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.
Closes #125
Why
The render job already draws every committed diagram, but the diagrams it had to draw were the samples under
doc/, which are documentation. That makes them weak tests: the labels a reader wants to see are exactly the ones that never break a renderer, so the job could only catch a defect that broke a plain label.What
doc/edgecase/adds one document per diagram type whose labels are the opposite, made of the punctuation that means something to mermaid: quotes,#,;, brackets, braces, parentheses,<br/>,:,,,*,-,|,%%, an emoji, and Japanese text.One document per type rather than one holding all seventeen, so that a failure names the subpackage instead of a line number that moves whenever a label changes. The existing
git ls-files '*.md'glob picks them up, and will pick up a new diagram type too, so nothing in the workflow needs editing when one is added.What the measurements found
Each type carries the punctuation it can actually take today, measured rather than guessed: every character was put through every type on its own and rendered with mermaid-cli. That turned up defects in thirteen of the seventeen subpackages, from
arch(which takes no punctuation at all, not even a space) togantt(which loses the diagram on a colon).Those are filed as #146 with the full matrix and reproduction steps. They are not fixed here on purpose: thirteen quoting fixes in one pull request would be unreviewable, and each changes generated output and needs its own regression test.
So the entries in
supportedare the honest current state, with the reason for each gap in a comment next to it. The rule that keeps the file from rotting is written there too: a fix that closes a gap widens the entry it is tested with, so the labels only ever get harder.Verification
All seventeen documents render locally with the same checker CI uses.
go generate ./...leaves a clean tree,go vetandgolangci-lintare clean, and the magic numbers in the samples carry//nolint:mndthe way the other generators underdoc/do.No library change
Generator and workflow comment only. No Go source file outside
doc/changed, and no existing generated document changed.Summary by CodeRabbit
Documentation
Tests