Repository navigation
feat(mermaid): add C4 context diagram support - #156
Conversation
mermaid/c4 builds the C4Context diagram: the people and the software systems around the one being described. Scope stays there on purpose, because mermaid marks its C4 support experimental and says the syntax may change; C4Container, C4Component, C4Dynamic, C4Deployment and the UpdateElementStyle family can each arrive later as a new method without changing anything that exists. A boundary is a pair of calls rather than a callback taking a nested builder: Boundary, EnterpriseBoundary and SystemBoundary open one and BoundaryEnd closes it, the way mermaid/treemap walks its own nesting with Section and Parent. A boundary left open is reported from Build, because mermaid refuses a diagram whose brace never closes and the whole drawing is lost. The escaping was measured against the renderer rather than reasoned about, and neither escape that looks obvious works. A backslash makes mermaid refuse the diagram; doubling the quote silently splits the argument, so Rel(a, b, "x""y") draws "y" as the technology rather than as part of the label. What C4 implements is its own entity form, so a quote becomes "#quot;" and a "#" becomes "#35;" ahead of it, or a label holding "#quot;" would come back as a character the caller never wrote. The title is the one place the text is unquoted: mermaid takes the rest of the line, quotes included, so quoting it would draw the quotation marks, and the escaped pair there is "#" and ";" instead. The render check learned to decode those escapes. It compares a declared title against the drawing, and mermaid decodes "#35;" before drawing it, so without this an escaped title reads as missing. Two fixtures pin both halves: one that must pass and one whose title really is lost. Closes #128
|
Warning Review limit reached
Next review available in: 45 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThe PR adds a chainable ChangesC4 context diagrams
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 3❌ Failed checks (3 warnings)
✅ Passed checks (2 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.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 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 `@doc/c4/main.go`:
- Around line 1-16: Remove the //go:build linux || darwin constraint from
doc/c4/main.go so the main package and its //go:generate directive are included
on Windows. Leave the generator implementation and existing directive unchanged.
In `@README.md`:
- Around line 2179-2182: Update the README markdown builder example around the
C4 Context section to include markdown.WithBlockSpacing(), matching
doc/c4/main.go. Regenerate the displayed output blocks so they include the blank
line after “## C4 Context” and exactly match doc/c4/generated.md.
In `@scripts/mermaid-check/check.mjs`:
- Around line 161-170: Update decodeEntities to decode numeric entities using
Unicode code points rather than String.fromCharCode, so values such as `#128512`;
produce the correct emoji. Reuse the existing DOM decoding path or apply
String.fromCodePoint with Mermaid-compatible range handling, while preserving
named-entity decoding.
🪄 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: 880dca4f-e322-44cd-b000-e67aa3122919
📒 Files selected for processing (16)
README.mdSPEC.mdcontract_test.godoc/c4/generated.mddoc/c4/main.godoc/edgecase/c4.mddoc/edgecase/main.gomermaid/c4/c4.gomermaid/c4/c4_test.gomermaid/c4/config.gomermaid/c4/examples_test.gomermaid/c4/testdata/golden/c4.mdscripts/mermaid-check/check.mjsscripts/mermaid-check/selftest.mjsscripts/mermaid-check/testdata/c4-escaped-title.mdfixturescripts/mermaid-check/testdata/c4-title-line-break.mdfixture
| //go:build linux || darwin | ||
|
|
||
| // Package main is generating mermaid C4 context diagram. | ||
| package main | ||
|
|
||
| import ( | ||
| "io" | ||
| "os" | ||
|
|
||
| "github.com/nao1215/markdown" | ||
| "github.com/nao1215/markdown/mermaid/c4" | ||
| ) | ||
|
|
||
| // This file is gated by //go:build linux || darwin, so //go:generate is skipped | ||
| // on Windows. To regenerate generated.md on Windows, run under WSL or via CI. | ||
| //go:generate go run main.go |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- tracked files ---'
git ls-files 'doc/c4' 'go.mod' 'go.work' | sed -n '1,120p'
printf '%s\n' '--- generator source ---'
cat -n doc/c4/main.go
printf '%s\n' '--- C4 directory ---'
find doc/c4 -maxdepth 1 -type f -print -exec wc -l {} \;
printf '%s\n' '--- generation directives and related constraints ---'
rg -n --glob '!generated.md' 'go:generate|go:build|mermaid/c4|package main' doc/c4 .Repository: nao1215/markdown
Length of output: 11896
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- module metadata ---'
cat -n go.mod | sed -n '1,80p'
printf '%s\n' '--- Windows package selection ---'
GOOS=windows GOARCH=amd64 go list -e -json ./doc/c4 | jq '{Dir,Name,GoFiles,IgnoredGoFiles,Imports,Error}'
printf '%s\n' '--- generation commands discovered for Windows ---'
GOOS=windows GOARCH=amd64 go generate -n ./doc/c4 2>&1 || true
printf '%s\n' '--- native package selection ---'
GOOS=linux GOARCH=amd64 go list -e -json ./doc/c4 | jq '{Dir,Name,GoFiles,IgnoredGoFiles,Imports,Error}'
printf '%s\n' '--- build constraints in imported C4 package ---'
rg -n --glob '*.go' '^//go:build|^// \+build|^package |^import ' mermaid/c4Repository: nao1215/markdown
Length of output: 2944
🏁 Script executed:
#!/bin/bash
set -eu
for pkg in . ./mermaid/c4 ./doc/c4; do
printf '%s\n' "--- Windows package: $pkg ---"
GOOS=windows GOARCH=amd64 go list -e -json "$pkg" |
jq '{ImportPath,Name,GoFiles,IgnoredGoFiles,Imports,Error}'
done
printf '%s\n' '--- platform-specific files in generator dependencies ---'
rg -n --glob '*.go' '^//go:build|^// \+build' markdown.go mermaid/c4 2>/dev/null || trueRepository: nao1215/markdown
Length of output: 1619
Remove the Windows build constraint from doc/c4/main.go.
On Windows, Go ignores main.go, so go generate ./doc/c4 finds no directive. The generator and its dependencies use Windows-compatible files and portable APIs.
🤖 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/c4/main.go` around lines 1 - 16, Remove the //go:build linux || darwin
constraint from doc/c4/main.go so the main package and its //go:generate
directive are included on Windows. Leave the generator implementation and
existing directive unchanged.
| err = markdown.NewMarkdown(f). | ||
| H2("C4 Context"). | ||
| CodeBlocks(markdown.SyntaxHighlightMermaid, diagram). | ||
| Build() |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Make the README example reproduce the linked generated file.
Line 2179 uses default spacing, but doc/c4/main.go Line 43 uses markdown.WithBlockSpacing(). The committed doc/c4/generated.md also contains a blank line after ## C4 Context, while both README output blocks omit it.
Add markdown.WithBlockSpacing() to the README example and regenerate the displayed output.
Suggested fix
- err = markdown.NewMarkdown(f).
+ err = markdown.NewMarkdown(f, markdown.WithBlockSpacing()).Also applies to: 2194-2229
🤖 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 `@README.md` around lines 2179 - 2182, Update the README markdown builder
example around the C4 Context section to include markdown.WithBlockSpacing(),
matching doc/c4/main.go. Regenerate the displayed output blocks so they include
the blank line after “## C4 Context” and exactly match doc/c4/generated.md.
mermaid rewrites "#123;" to the HTML reference "{" and lets the browser resolve it, so a character code truncates anything past U+FFFF: "#128512;" came out as U+F600 rather than as the emoji, and the render check would have called a title that is drawn missing. Decode through the DOM the way mermaid does, and put an astral escape in the fixture so the difference is what fails if it comes back. Also spell "color" the way the rest of the repository does.
|
Thanks — two of these were right and are fixed in the latest commit. Numeric decoding. Correct, and a real bug in what I added. mermaid rewrites "colour". Fixed. The Windows build constraint on
|
Code Metrics Report
Details | | main (39c5128) | #156 (2b43767) | +/- |
|---------------------|----------------|----------------|-------|
+ | Coverage | 96.2% | 96.3% | +0.0% |
| Files | 65 | 67 | +2 |
| Lines | 2966 | 3149 | +183 |
+ | Covered | 2856 | 3034 | +178 |
| Test Execution Time | 9s | 9s | 0s |Code coverage of files in pull request scope (0.0% → 97.2%)
Reported by octocov |
Closes #128.
Scope
mermaid/c4builds theC4Contextdiagram: the people and the software systems around the one being described. The scope stops there on purpose — mermaid marks its C4 support experimental and says the syntax may change — soC4Container,C4Component,C4Dynamic,C4Deploymentand theUpdateElementStylefamily are left out. Each can arrive later as a new method without changing anything that exists.API
NewDiagram(w, opts...)withWithTitle, then the usualBuild/Error/String/LF. Elements arePerson,PersonExt,System,SystemExt,SystemDb,SystemQueue, each takingWithDescription. Relationships areRelandBiRel, each takingWithTechnology.Boundaries are an explicit pair of calls, which the issue asked me to state:
Boundary(withWithBoundaryType),EnterpriseBoundaryandSystemBoundaryopen one, everything after belongs to it, andBoundaryEndcloses it. The alternative in the issue wasBoundary(id, label string, fn func(*Diagram)). A callback taking a nested builder would be a new idiom in this library; the explicit pair keeps the chain flat, which is how every other builder reads, and it is exactly howmermaid/treemapwalks its own nesting withSectionandParent. A boundary left open is reported fromBuildrather than written out, because mermaid refuses a diagram whose brace never closes and the whole drawing is lost.Escaping, measured rather than reasoned about
Both escapes that look obvious are wrong, and I found that by rendering all three forms:
\"""Rel(a, b, "x""y")drawsyas the technology#quot;"— this is the oneSo a quotation mark in a label becomes
#quot;, and a#becomes#35;ahead of it in the same pass — otherwise a caller's literal#quot;would come back as a character they never wrote.The title is the exception. mermaid reads the rest of the line as the C4 title, quotes and all, so quoting it would draw the quotation marks; there a bare
"is fine and the pair that needs escaping is#(starts a comment) and;(ends the statement), written#35;and#59;.Render check
scripts/mermaid-check/check.mjscompares a declared title against the drawing, and mermaid decodes#35;before drawing it — so without a change there, an escaped C4 title reads as missing. The checker now decodes the#name;and#123;forms the same way mermaid does. Two fixtures pin both halves:c4-escaped-titlemust pass, andc4-title-line-break(a title broken with<br/>, which really is lost) must still fail.Files
mermaid/c4/,doc/c4/,doc/edgecase/(the C4 entry carries every character the type can take, which is all of them plus a backslash),README.md,SPEC.md(one line: C4 is the diagram type that does not take its title from front matter),contract_test.go(the root test that asserts every subpackage produces a mermaid block enumerates them, so C4 is registered there), and the render checker.Verification
go test ./mermaid/c4/passes at 97.3% statement coverage.golangci-lint run ./...reports 0 issues.node scripts/mermaid-check/selftest.mjs— 9 fixtures, 0 failed.doc/c4/generated.md, the edge-case document, and the README blocks.Summary by CodeRabbit
New Features
Documentation
Bug Fixes