Harden pr-description-skill mermaid conventions (diagram-type-by-intent + GH-renderer gotchas)

[danielmeppiel]

TL;DR

Harden pr-description-skill's mermaid guidance so PR-body diagrams (a) pick the right diagram TYPE per reviewer intent and (b) survive GitHub's renderer.

The proposal comment on #983 surfaced the gap concretely: a flowchart edge label |[EXEC] jq normalize, ...| parsed locally but was rejected by GitHub (Expecting 'TAGEND', ..., got 'SQS'). That class of bug was not in the skill's pitfall list.

Problem (WHY)

The current pr-description-skill SKILL.md inlines a partial pitfall list and says "1-3 mermaid diagrams" with no guidance on which diagram TYPE to pick. Two consequences:

• Wrong type for the intent. Execution flow between distinct actors (workflow, jobs, third-party actions) was being rendered as flowchart TD, forcing reviewers to reconstruct the temporal axis from arrows.
• GitHub-renderer rejections that mmdc does NOT catch. Square brackets in flowchart edge labels are the most recent example; :::cssClass on classDiagram relationship lines is a known prior one.

Approach (WHAT)

R3 EXTRACT refactor (per genesis design discipline):

Change	Where
New asset: assets/mermaid-conventions.md (~230 lines)	.apm/skills/pr-description-skill/
Diagram-type-by-intent table	new asset
Canonical templates for sequenceDiagram, flowchart LR, stateDiagram-v2	new asset
Boxing convention: rect rgb(255, 247, 200) for new sequence interactions; classDef new stroke-dasharray: 5 5 + class N new for new flowchart nodes	new asset
GitHub-renderer gotchas (drift-known, mmdc does NOT always catch)	new asset
Slim SKILL.md inline pitfall list to a load-trigger	SKILL.md
Critical bracket-edge-label gotcha kept inline (most likely to bite)	SKILL.md

Implementation (HOW)

• assets/mermaid-conventions.md: diagram-type table with one row per reviewer question; canonical mermaid templates for the three common PR-body shapes; "GitHub-renderer gotchas" section listing the rejections that mmdc may parse cleanly; validation discipline (mmdc + mermaid.live + post-push eyeball).
• SKILL.md execution checklist step 5: now mandates loading the new asset BEFORE drafting any block.
• SKILL.md mermaid validation section: kept the mmdc step; replaced inline pitfall list with a load-trigger to the asset; kept the bracket-edge-label rule inline because it is the most likely to silently bite.
• Mirror to .github/skills/pr-description-skill/ via apm install --target copilot (committed; matches repo convention per CHANGELOG entry).

Note

The genesis skill owns architectural-design diagram conventions. This asset is scoped explicitly to PR bodies (sequence-flow-heavy, not architecture-heavy). No duplication today; if drift appears, R3 EXTRACT into a shared substrate later.

Validation

All three canonical templates in the new asset validate via mmdc:

$ for f in /tmp/mermaid-check/diag*.mmd; do
    npx --yes -p @mermaid-js/mermaid-cli mmdc -i "$f" -o "${f%.mmd}.svg" --quiet
  done
--- diag1.mmd done   # sequenceDiagram
--- diag2.mmd done   # flowchart LR
--- diag3.mmd done   # stateDiagram-v2

The bracket-edge-label gotcha was reproduced and confirmed against GitHub on issue #983: #983 (comment) -- the comment had to be re-edited with |"[EXEC] ..."| (quoted) before it rendered.

Trade-offs

• One asset over inline. The asset adds a load step but lifts ~30 lines of pitfalls out of SKILL.md and adds the diagram-type-by-intent table without bloating the body. Net: progressive disclosure win.
• Kept the bracket gotcha inline in SKILL.md too. Slight duplication, but this is the gotcha most likely to bite a maintainer who skips the asset load.
• No new validator. mmdc stays. The drift-known gotchas are documented in prose, not a script -- adding a GitHub-render validator would require headless browser + GH's mermaid version, not worth the ROI today.

How to test

1. cd /path/to/microsoft/apm && git checkout harden/pr-description-mermaid-conventions
2. Open .apm/skills/pr-description-skill/assets/mermaid-conventions.md and .github/skills/pr-description-skill/assets/mermaid-conventions.md -- verify both exist and match.
3. Run the validation script from the SKILL.md against the new asset's three mermaid blocks; confirm all parse.
4. (Optional) Paste the broken example A -->|[EXEC] x| B into https://mermaid.live and confirm it errors with got 'SQS'; replace with A -->|"[EXEC] x"| B and confirm it renders.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

[Copilot]

Pull request overview

This PR hardens the pr-description-skill mermaid guidance so PR-body diagrams choose an appropriate mermaid diagram type for the reviewer’s intent and avoid known GitHub mermaid renderer pitfalls.

Changes:

• Add a new assets/mermaid-conventions.md reference asset with diagram-type-by-intent guidance, canonical templates, and GitHub-renderer gotchas.
• Update pr-description-skill’s checklist/validation sections to require loading the new asset before authoring diagrams and to keep the most common GitHub-renderer pitfall inline.
• Add an Unreleased CHANGELOG entry describing the maintainer-tooling update.

Show a summary per file

File	Description
CHANGELOG.md	Adds an Unreleased “Maintainer tooling” entry describing the mermaid-guidance hardening.
.github/skills/pr-description-skill/assets/mermaid-conventions.md	New mermaid conventions asset for PR-body diagrams (type selection, templates, GitHub-renderer gotchas).
.github/skills/pr-description-skill/SKILL.md	Points diagram authoring/validation steps to the new asset; keeps the bracket-in-edge-label gotcha inline.
.apm/skills/pr-description-skill/assets/mermaid-conventions.md	Same new asset mirrored into .apm/ source-of-truth location.
.apm/skills/pr-description-skill/SKILL.md	Same SKILL.md updates mirrored into .apm/.

Copilot's findings

• Files reviewed: 5/5 changed files
• Comments generated: 7

[Copilot]

The table row for stateDiagram-v2 suggests single-line note right of S: NEW markers, but this same doc later states that single-line note right of X: text is NOT supported in stateDiagram-v2. Please align the table with the multi-line note ... end note form (or adjust the later rule).

Suggested change

	| 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]

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).

Suggested change

	V2[policy check]:::new
	V2[policy check]

[Copilot]

This parenthetical contains inline-code spans that are split across lines (starts at `note right and ends at of` 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).

Suggested change

	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

[Copilot]

The new Unreleased changelog entry is missing the required PR reference at the end (e.g. "(#123)"). Please append this PR number (and consider shortening the line so it stays scannable, consistent with other entries).

Suggested change

- `pr-description-skill` mermaid guidance hardened: new asset `assets/mermaid-conventions.md` defines diagram-type-by-intent (sequenceDiagram for execution flow with `rect rgb(...)` boxing, flowchart for pipelines/architecture with `classDef new`, stateDiagram-v2 for state machines) and captures GitHub-renderer gotchas that `mmdc` does not always catch (notably: square brackets in flowchart edge labels MUST be quoted -- `|"[label]"|` not `|[label]|`).

- `pr-description-skill` mermaid guidance was hardened: new `assets/mermaid-conventions.md` maps diagram type to intent and documents GitHub renderer gotchas that `mmdc` can miss, including quoted flowchart edge labels like `|"[label]"|`. (#PR_NUMBER)

[Copilot]

The table row for stateDiagram-v2 suggests single-line note right of S: NEW markers, but this same doc later states that single-line note right of X: text is NOT supported in stateDiagram-v2. Please align the table with the multi-line note ... end note form (or adjust the later rule).

Suggested change

	| 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]

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).

Suggested change

	V2[policy check]:::new
	V2[policy check]

[Copilot]

This parenthetical contains inline-code spans that are split across lines (starts at `note right and ends at of` 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).

Suggested change

	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