-
Notifications
You must be signed in to change notification settings - Fork 155
Harden pr-description-skill mermaid conventions (diagram-type-by-intent + GH-renderer gotchas) #984
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,229 @@ | ||||||
| # Mermaid conventions for PR bodies | ||||||
|
|
||||||
| Load this asset before drafting any mermaid block in a PR body. It | ||||||
| defines (a) which diagram TYPE to pick per intent, (b) the boxing / | ||||||
| styling vocabulary that highlights NEW behavior, and (c) the | ||||||
| GitHub-renderer gotchas that `mmdc` does NOT always catch. | ||||||
|
|
||||||
| This asset is scoped to **PR bodies**. Architectural design diagrams | ||||||
| (component, thread fan-out, dependency graph) are owned by the | ||||||
| `genesis` skill and have a different convention set; do not conflate. | ||||||
|
|
||||||
| ## Diagram type by intent | ||||||
|
|
||||||
| A PR body diagram answers ONE question. Pick the type that matches | ||||||
| the question; do not mix. | ||||||
|
|
||||||
| | Reviewer's question | Diagram type | Boxing convention for "what changed" | | ||||||
| |---|---|---| | ||||||
| | Which jobs / participants run, in what order? (execution flow) | `sequenceDiagram` | `rect rgb(255, 247, 200)` block around new participant interactions; `Note over X` for invariants | | ||||||
| | What is the data / control pipeline? (stages, transformations) | `flowchart LR` | `subgraph` per stage; `classDef new stroke-dasharray: 5 5` for new stages; class assignment via `class N1,N2 new` | | ||||||
| | What does the state machine look like? | `stateDiagram-v2` | `note right of S: NEW` markers on changed states | | ||||||
|
||||||
| | What does the state machine look like? | `stateDiagram-v2` | `note right of S: NEW` markers on changed states | | |
| | What does the state machine look like? | `stateDiagram-v2` | multi-line `note right of S` ... `end note` markers on changed states | |
Copilot
AI
Apr 27, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The flowchart template marks V2 with inline :::new, but the conventions immediately below say to avoid inline N:::new and instead use a separate class N new assignment. Please update the template to follow the documented convention (and keep only one mechanism for marking NEW nodes).
| V2[policy check]:::new | |
| V2[policy check] |
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -176,7 +176,7 @@ scannable. | |||||||||||||
| | 3 | Problem (WHY) | Observed failure modes; max 6 bullets, max 3 quoted anchors | | ||||||||||||||
| | 4 | Approach (WHAT) | Table or 3-7 bullets; may say "additive: see Implementation" | | ||||||||||||||
| | 5 | Implementation (HOW) | One short paragraph per file or a table | | ||||||||||||||
| | 6 | Diagrams | 1-3 validated mermaid blocks, each with a legend | | ||||||||||||||
| | 6 | Diagrams | 1-3 validated mermaid blocks, each with a legend; diagram type chosen per intent (`assets/mermaid-conventions.md`) | | ||||||||||||||
| | 7 | Trade-offs | 3-5 bullets (1-2 if mechanical) | | ||||||||||||||
| | 8 | Benefits | 3-5 numbered, measurable items | | ||||||||||||||
| | 9 | Validation | Real command output, ideally inside `<details>` if long | | ||||||||||||||
|
|
@@ -224,8 +224,12 @@ Run these steps in order. Tick each before moving on. | |||||||||||||
| 4. [ ] Fill in the template top-to-bottom using only facts from | ||||||||||||||
| the activation contract. Every WHY-claim gets a verbatim | ||||||||||||||
| quoted anchor. If you cannot anchor a claim, drop it. | ||||||||||||||
| 5. [ ] Generate 1-3 mermaid diagrams. Add a one-sentence legend | ||||||||||||||
| above each. | ||||||||||||||
| 5. [ ] Generate 1-3 mermaid diagrams. **Before drafting any block, | ||||||||||||||
| load `assets/mermaid-conventions.md`** to pick the right | ||||||||||||||
| diagram type per intent (sequenceDiagram for execution flow, | ||||||||||||||
| flowchart LR for pipeline / architecture, stateDiagram-v2 for | ||||||||||||||
| state machines) and apply the boxing convention for NEW | ||||||||||||||
| behavior. Add a one-sentence legend above each diagram. | ||||||||||||||
| 6. [ ] **Validate every mermaid block deterministically (see | ||||||||||||||
| below). Do NOT save the draft until every block validates.** | ||||||||||||||
| 7. [ ] Load `assets/section-rubric.md` and run the self-check pass. | ||||||||||||||
|
|
@@ -255,24 +259,26 @@ done | |||||||||||||
| If `mmdc` reports any error, fix the diagram and re-run. The skill | ||||||||||||||
| MUST NOT save the draft until every mermaid block validates. | ||||||||||||||
|
|
||||||||||||||
| ### Common mermaid pitfalls | ||||||||||||||
|
|
||||||||||||||
| - **Semicolons in `classDiagram` link labels break the parser.** A | ||||||||||||||
| label like `dispatches; verifies 3 artifacts` errors at the | ||||||||||||||
| semicolon. Use commas or rephrase: `dispatches, verifies 3 artifacts`. | ||||||||||||||
| - **`note right of X` in `stateDiagram-v2`** must close with | ||||||||||||||
| `end note` on its own line. A note that bleeds into the next | ||||||||||||||
| state declaration is a parse error. | ||||||||||||||
| - **Round brackets `()` in flowchart node labels** need quoting: | ||||||||||||||
| write `A["foo (bar)"]`, not `A[foo (bar)]`. | ||||||||||||||
| - **Double quotes inside node labels** must be HTML-escaped as | ||||||||||||||
| `"`. Mermaid does not support nested or escaped double | ||||||||||||||
| quotes inside `"..."` labels. | ||||||||||||||
| - **Pipes `|` and angle brackets `<>` inside labels** also need | ||||||||||||||
| quoting or HTML escapes; they are operators in mermaid syntax. | ||||||||||||||
| - **Edge labels with colons** can confuse `stateDiagram-v2`; prefer | ||||||||||||||
| arrow-then-label form: `A --> B : trigger received`, not | ||||||||||||||
| `A --> B[trigger: received]`. | ||||||||||||||
| ### Diagram type and pitfalls reference | ||||||||||||||
|
|
||||||||||||||
| The full diagram-type-by-intent table, canonical templates, and the | ||||||||||||||
| GitHub-renderer gotcha list (`mmdc` does NOT always catch GitHub | ||||||||||||||
| rejections) live in `assets/mermaid-conventions.md`. Load it whenever | ||||||||||||||
| a PR body needs a mermaid block. | ||||||||||||||
|
|
||||||||||||||
| Critical drift-known gotcha (the one most likely to bite, captured | ||||||||||||||
| inline because it is not obvious from `mmdc` output): | ||||||||||||||
|
|
||||||||||||||
| - **Square brackets in flowchart edge labels MUST be quoted.** | ||||||||||||||
| `A -->|[EXEC] work| B` parses on `mmdc` but is rejected by | ||||||||||||||
| GitHub's renderer (`Expecting 'TAGEND', ..., got 'SQS'`). Quote | ||||||||||||||
| the label: `A -->|"[EXEC] work"| B`. The same rule applies to | ||||||||||||||
| parentheses, colons, slashes, and pipes in edge labels. | ||||||||||||||
|
|
||||||||||||||
| For everything else (semicolons in classDiagram links, `note right | ||||||||||||||
| of` closing rules, round brackets in node labels, inline | ||||||||||||||
| `:::cssClass` failing in classDiagram on GitHub), see | ||||||||||||||
|
Comment on lines
+278
to
+280
|
||||||||||||||
| For everything else (semicolons in classDiagram links, `note right | |
| of` closing rules, round brackets in node labels, inline | |
| `:::cssClass` failing in classDiagram on GitHub), see | |
| For everything else (semicolons in classDiagram links, `note right of` | |
| closing rules, round brackets in node labels, inline `:::cssClass` | |
| failing in classDiagram on GitHub), see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This parenthetical contains inline-code spans that are split across lines (starts at
`note rightand ends atof`on the next line). That tends to render poorly/ambiguously in Markdown. Please keep each inline-code span on a single line (or rephrase to avoid splitting the code span).