[superseded by #977] feat(mermaid): WCAG 2.2 AA colour contrast validation and updated diagram standards#975
[superseded by #977] feat(mermaid): WCAG 2.2 AA colour contrast validation and updated diagram standards#975ashleyshaw wants to merge 2 commits into
Conversation
…standards Adds a new colour contrast validator, a PR-triggered workflow, and comprehensive mermaid diagram standards to address dark-mode contrast failures where fill-only style declarations render with white text (~1.1:1 contrast ratio) rather than the intended dark text. - scripts/validation/validate-mermaid-colour-contrast.js: new WCAG AA checker that detects fill declarations missing explicit color, calculates relative luminance and contrast ratio per the WCAG 2.2 formula, and produces a dated report under .github/reports/mermaid/ - .github/workflows/validate-mermaid-pr.yml: new workflow triggered on pull_request and feature branch pushes for any changed .md files; runs syntax, accessibility, and contrast checks; posts a summary comment on the PR and fails the build on contrast errors - instructions/mermaid.instructions.md: updated to v2.0 with approved WCAG AA colour palette (fill+color+stroke triples, all ≥ 4.5:1), canonical emoji vocabulary, Phosphor icon limitation note, required accTitle/accDescr header block structure, and repo-wide update process - package.json: adds validate:mermaid-contrast and validate:mermaid (runs all three validators in sequence) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXjmeDonrDybfPZsH6Aa6w
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository YAML (base), Organization UI (inherited) Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 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 |
|
✅ Template check passed after update. Thanks for fixing the PR description. |
🔍 Reviewer Summary for PR #975CI Status: ❌ Recommendations
|
lint-staged@17.0.7 requires Node >=22.22.1; the previous node-version: 20 caused npm ci to fail with EBADENGINE on every run of validate-mermaid-pr. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01LXjmeDonrDybfPZsH6Aa6w
There was a problem hiding this comment.
Code Review
This pull request introduces comprehensive standards and validation tools for Mermaid diagrams, including updated guidelines in instructions/mermaid.instructions.md, new npm scripts in package.json, and a new WCAG 2.2 AA colour contrast validation script. Feedback on the validation script highlights a regex bug that could cause silent failures on non-standard hex lengths, potential false positives with hyphenated CSS properties, and misleading/duplicate reporting when text colors are missing. The reviewer also suggested calculating and reporting actual line numbers for better usability, and recommended documenting the repository-wide update process in /docs/MIGRATION.md to comply with repository rules.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| function parseColour(colour) { | ||
| if (!colour) return null; | ||
| const trimmed = colour.trim().toLowerCase(); | ||
| if (/^#[0-9a-f]{3,6}$/.test(trimmed)) return trimmed; |
There was a problem hiding this comment.
The regular expression /^#[0-9a-f]{3,6}$/ matches hex color strings of length 3, 4, 5, or 6. However, 4-digit and 5-digit hex colors are not standard for Mermaid and will cause normaliseHex and relativeLuminance to produce NaN values (due to slicing out-of-bounds). This results in the contrast check silently passing because NaN < 4.5 evaluates to false. Restricting the match to exactly 3 or 6 hex digits prevents this issue.
| if (/^#[0-9a-f]{3,6}$/.test(trimmed)) return trimmed; | |
| if (/^#([0-9a-f]{3}|[0-9a-f]{6})$/.test(trimmed)) return trimmed; |
| const fillMatch = props.match(/\bfill\s*:\s*([^,;\s]+)/i); | ||
| const fill = fillMatch ? fillMatch[1].trim() : null; | ||
|
|
||
| // Extract text colour | ||
| const colorMatch = props.match(/\bcolor\s*:\s*([^,;\s]+)/i); | ||
| const color = colorMatch ? colorMatch[1].trim() : null; | ||
|
|
There was a problem hiding this comment.
The word boundary \b matches hyphenated SVG/CSS properties like stop-color or stroke-color because - is a non-word character. This can lead to false positives where the validator incorrectly assumes an explicit text color is set. Using a negative lookbehind (?<!-) ensures that fill and color are not part of other hyphenated properties.
| const fillMatch = props.match(/\bfill\s*:\s*([^,;\s]+)/i); | |
| const fill = fillMatch ? fillMatch[1].trim() : null; | |
| // Extract text colour | |
| const colorMatch = props.match(/\bcolor\s*:\s*([^,;\s]+)/i); | |
| const color = colorMatch ? colorMatch[1].trim() : null; | |
| // Extract fill colour | |
| const fillMatch = props.match(/(?<!-)\bfill\s*:\s*([^,;\s]+)/i); | |
| const fill = fillMatch ? fillMatch[1].trim() : null; | |
| // Extract text colour | |
| const colorMatch = props.match(/(?<!-)\bcolor\s*:\s*([^,;\s]+)/i); | |
| const color = colorMatch ? colorMatch[1].trim() : null; |
| if (!color) { | ||
| // No explicit text colour — determine the theme default | ||
| const defaultTextHex = | ||
| MERMAID_THEME_TEXT_DEFAULTS[theme] ?? MERMAID_THEME_TEXT_DEFAULTS.default; | ||
| const ratio = contrastRatio(fillHex, defaultTextHex); | ||
|
|
||
| issues.push({ | ||
| level: "warning", | ||
| message: | ||
| `Node "${nodeId}": fill ${fill} has no explicit color. ` + | ||
| `Against the "${theme}" theme default text (${defaultTextHex}) ` + | ||
| `the contrast ratio is ${ratio.toFixed(2)}:1 — ` + | ||
| (ratio >= WCAG_AA_NORMAL_TEXT | ||
| ? `passes AA for the "${theme}" theme, but will FAIL in dark mode. Add color: explicitly.` | ||
| : `FAILS WCAG AA (${WCAG_AA_NORMAL_TEXT}:1 required). Add color: explicitly.`), | ||
| }); | ||
|
|
||
| // Also check against dark-mode text (white) for the "missing color" case | ||
| const darkRatio = contrastRatio(fillHex, "#ffffff"); | ||
| if (darkRatio < WCAG_AA_NORMAL_TEXT) { | ||
| issues.push({ | ||
| level: "error", | ||
| message: | ||
| `Node "${nodeId}": fill ${fill} without explicit color FAILS in dark mode ` + | ||
| `(white text contrast: ${darkRatio.toFixed(2)}:1, minimum 4.5:1).`, | ||
| }); | ||
| } | ||
|
|
||
| return issues; | ||
| } |
There was a problem hiding this comment.
When color is missing, the validator hardcodes the assumption that the failure will occur in dark mode, which is misleading for dark fills (which actually fail in light mode). Additionally, if the theme is already "dark", it reports both a warning and an error against white text, leading to duplicate findings. Refactoring this logic to evaluate both light and dark modes explicitly provides clearer, more accurate feedback and avoids duplicate reports.
if (!color) {
const lightRatio = contrastRatio(fillHex, "#333333");
const darkRatio = contrastRatio(fillHex, "#ffffff");
const failsLight = lightRatio < WCAG_AA_NORMAL_TEXT;
const failsDark = darkRatio < WCAG_AA_NORMAL_TEXT;
if (failsLight || failsDark) {
let failMode = "";
if (failsLight && failsDark) {
failMode = "both light and dark modes";
} else if (failsLight) {
failMode = "light mode (dark text)";
} else {
failMode = "dark mode (white text)";
}
issues.push({
level: "error",
message: "Node \"" + nodeId + "\": fill " + fill + " without explicit color FAILS in " + failMode + " (light contrast: " + lightRatio.toFixed(2) + ":1, dark contrast: " + darkRatio.toFixed(2) + ":1). Add an explicit color: to guarantee contrast."
});
} else {
issues.push({
level: "warning",
message: "Node \"" + nodeId + "\": fill " + fill + " has no explicit color. While it passes contrast in both light and dark modes, adding an explicit color: is highly recommended."
});
}
return issues;
}| report.findings.push({ | ||
| file: relPath, | ||
| diagramIndex: di + 1, | ||
| theme, | ||
| level: issue.level, | ||
| message: issue.message, | ||
| rawStyle: style.raw, | ||
| }); |
There was a problem hiding this comment.
The validator currently does not output or report the actual line number of the style declaration in the markdown file, making it harder for developers to locate and fix contrast issues in large files. Calculating the actual 1-based line number in the markdown file using diagram.startLine + style.line and including it in the findings report improves usability.
const fileLine = diagram.startLine + style.line;
report.findings.push({
file: relPath,
diagramIndex: di + 1,
theme,
level: issue.level,
message: issue.message,
rawStyle: style.raw,
line: fileLine,
});| const icon = issue.level === "error" ? "❌" : "⚠️ "; | ||
| console.log(` ${icon} Diagram ${di + 1}: ${issue.message}`); | ||
| } |
There was a problem hiding this comment.
Including the actual line number of the style declaration in the console output makes it much easier for developers to locate and fix contrast issues.
const icon = issue.level === "error" ? "❌" : "⚠️ ";
const fileLine = diagram.startLine + style.line;
console.log(" " + icon + " Diagram " + (di + 1) + " (line " + fileLine + "): " + issue.message);| .map( | ||
| (f) => | ||
| `### ${f.level.toUpperCase()}: \`${f.file}\` — Diagram #${f.diagramIndex}\n\n` + | ||
| `- **Theme**: ${f.theme}\n` + | ||
| `- **Style**: \`${f.rawStyle}\`\n` + | ||
| `- **Issue**: ${f.message}\n`, | ||
| ) |
There was a problem hiding this comment.
Updating the markdown report generator to include the calculated line number ensures that the generated reports are highly actionable and precise.
| .map( | |
| (f) => | |
| `### ${f.level.toUpperCase()}: \`${f.file}\` — Diagram #${f.diagramIndex}\n\n` + | |
| `- **Theme**: ${f.theme}\n` + | |
| `- **Style**: \`${f.rawStyle}\`\n` + | |
| `- **Issue**: ${f.message}\n`, | |
| ) | |
| .map( | |
| (f) => | |
| "### " + f.level.toUpperCase() + ": " + String.fromCharCode(96) + f.file + String.fromCharCode(96) + " — Diagram #" + f.diagramIndex + " (line " + f.line + ")\n\n" + | |
| "- **Theme**: " + f.theme + "\n" + | |
| "- **Style**: " + String.fromCharCode(96) + f.rawStyle + String.fromCharCode(96) + "\n" + | |
| "- **Issue**: " + f.message + "\n", | |
| ) |
|
|
||
| ## Repository-wide Update Process | ||
|
|
||
| When diagrams across the repository need to be updated (new palette, new structural requirements): |
There was a problem hiding this comment.
In accordance with the repository's general rules, repository-wide update and migration steps should also be documented in the central /docs/MIGRATION.md file. This ensures that contributors can easily follow migration rules across the repository.
References
- Document migration maps and notes in a central
/docs/MIGRATION.mdfile to ensure contributors can follow migration rules mentioned in README files across the repository.
Linked issues
Closes #976
Changelog
Added
scripts/validation/validate-mermaid-colour-contrast.js: WCAG 2.2 AA colour contrast validator for all Mermaidstyledeclarations across all.mdfiles; detects missing explicitcoloralongsidefilland calculates relative luminance contrast ratios per the WCAG 2.2 formula.github/workflows/validate-mermaid-pr.yml: PR and feature-branch workflow that runs all three mermaid validators (syntax, accessibility, contrast) on changed.mdfiles, posts a summary comment, and blocks merge on contrast failuresnpm run validate:mermaid-contrastandnpm run validate:mermaidscripts to run the contrast validator and all three validators in sequenceChanged
instructions/mermaid.instructions.mdupdated to v2.0: approved WCAG AA colour palette (7 semantic fill/color/stroke triples, all ≥ 4.5:1 in both light and dark mode), canonical emoji vocabulary, Phosphor icon limitation note, requiredaccTitle/accDescrheader block structure, and repo-wide update processChecklist (Global DoD / PR)
Summary
Adds a WCAG 2.2 AA colour contrast validator and automated PR workflow to catch the dark-mode contrast failures present in all existing Mermaid diagrams (36 errors on first run). Updates diagram standards to v2.0 with an approved colour palette, emoji vocabulary, and repo-wide update process.
Root cause:
style X fill:#e1f5fewithout explicitcolor— Mermaid inherits white text in dark mode, contrast ~1.1:1 against light pastel fills.Follow-up: #976 tracks the repo-wide palette sweep to fix all 47 existing diagram files.
🤖 Generated with Claude Code
https://claude.ai/code/session_01LXjmeDonrDybfPZsH6Aa6w