Skip to content

docs(design): write the diagram system into DESIGN.md - #12649

Open
jd wants to merge 1 commit into
devs/jd/jd/ship-diagram-token-system/let-diagram-name-own-roles-lock-door--6cfa48c8from
devs/jd/jd/ship-diagram-token-system/write-diagram-system-design-md--d961fd51
Open

docs(design): write the diagram system into DESIGN.md#12649
jd wants to merge 1 commit into
devs/jd/jd/ship-diagram-token-system/let-diagram-name-own-roles-lock-door--6cfa48c8from
devs/jd/jd/ship-diagram-token-system/write-diagram-system-design-md--d961fd51

Conversation

@jd

@jd jd commented Sep 1, 2026

Copy link
Copy Markdown
Member

DESIGN.md did not contain the word "diagram", and that silence is the whole
reason this drifted: the diagram surface became the single largest violation of
this document's own STRICT rules — no hex outside tokens.css, every dark remap
in theme.css, components consume semantic tokens — while the document said
nothing about it either way. A lint stops a color reaching a fence; only this
explains what to write instead.

The new section carries the role table, the three layout kinds, how a role
becomes a surface/border/label, and the four surfaces that share the vocabulary.
It also records the two judgment calls a reader would otherwise have to
reverse-engineer: why queued and mergify share one accent, and why diagram
borders lift toward white on dark when "the product palette does not flip" three
sections above. The STRICT rules and the self-correction list gain the diagram
case, and the SVG-hex exemption now says explicitly that diagrams are not
covered by it.

AGENTS.md gets the short version and the new command, per this file's own
instruction to keep the two aligned.

Also normalizes the diagram code to the site's American spelling. The three
commits below this one were written in British English, which no other file in
the repo uses — and the lint prints its guidance to contributors of a site that
says "color". It is a style pass rather than a fix, folded in here because this
is the commit that codifies house style; it changes no behavior, and the tests
and the lint pass unchanged.

@jd

jd commented Sep 1, 2026

Copy link
Copy Markdown
Member Author

This pull request is part of a Mergify stack:

# Pull Request Link
1 feat(diagrams): resolve every diagram color from tokens, at paint time #12646
2 feat(diagrams): bring the hand-drawn diagrams onto the same palette #12647
3 feat(diagrams): let each diagram name its own roles, and lock the door #12648
4 docs(design): write the diagram system into DESIGN.md #12649 👈

@mergify

mergify Bot commented Sep 1, 2026

Copy link
Copy Markdown
Contributor

Merge Protections

🔴 4 of 7 protections blocking · waiting on 👀 reviews, 🤖 CI, ⛓️ dependency and 🙋 you

Protection Waiting on
🔴 ⛓️ Depends-On Requirements ⛓️ dependency
🔴 🤖 Continuous Integration 🤖 CI and 🙋 you
🔴 👀 Review Requirements 👀 reviews
🔴 🔎 Reviews 👀 reviews
🟢 Enforce conventional commit
🟢 📕 PR description
🟢 🚦 Auto-queue

🔴 ⛓️ Depends-On Requirements

Waiting for

This rule is failing.

Requirement based on the presence of Depends-On in the body of the pull request

🔴 🤖 Continuous Integration

Waiting for

  • check-success = build
  • check-success = lint
  • check-success = test
  • any of:
    • check-success = test-broken-links
    • label = ignore-broken-links
This rule is failing.
  • all of:
    • check-success = build
    • check-success = lint
    • check-success = test
    • any of:
      • check-success = test-broken-links
      • label = ignore-broken-links
    • any of:
      • check-success=Cloudflare Pages
      • -head-repo-full-name~=^Mergifyio/

🔴 👀 Review Requirements

Waiting for

  • #approved-reviews-by >= 2
This rule is failing.
  • any of:
    • #approved-reviews-by >= 2
    • author = dependabot[bot]
    • author = renovate[bot]
    • all of:
      • author = mergify-ci-bot
      • -head ~= ^docs-agent/

🔴 🔎 Reviews

Waiting for

  • #review-requested = 0
This rule is failing.
  • #review-requested = 0
  • #changes-requested-reviews-by = 0
  • #review-threads-unresolved = 0

Show 3 satisfied protections

🟢 Enforce conventional commit

Make sure that we follow https://www.conventionalcommits.org/en/v1.0.0/

  • title ~= ^(fix|feat|internal|docs|style|refactor|perf|test|build|ci|chore|revert|ui)(?:\(.+\))?!?:

🟢 📕 PR description

  • body ~= (?ms:.{48,})

🟢 🚦 Auto-queue

When all merge protections are satisfied, this pull request will be queued automatically.

@jd
jd marked this pull request as ready for review September 1, 2026 15:35
Copilot AI lite review requested due to automatic review settings September 1, 2026 15:35

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

DESIGN.md contains inline code spans that include triple backticks (```), which breaks Markdown rendering and should be fixed before merging.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR documents the diagrams design system in DESIGN.md (roles, kinds, and paint rules), aligns contributor guidance in AGENTS.md, and normalizes diagram-related prose/comments to American English across the supporting implementation and lint/test files.

Changes:

  • Add a comprehensive “Diagrams” section to DESIGN.md and update STRICT rules/self-correction guidance accordingly.
  • Update AGENTS.md with the diagrams rule summary and the pnpm check:diagram-tokens command.
  • Normalize “colour” → “color” wording and add cross-references to the new DESIGN.md section in relevant code/docs comments.
File summaries
File Description
src/util/diagramSvg.ts Updates documentation comments and adds a pointer to the new DESIGN.md “Diagrams” section.
src/styles/theme.css Clarifies diagram token comments and normalizes spelling in comments.
src/styles/index.css Normalizes spelling in the diagrams comment block.
src/components/GitGraph.astro Normalizes spelling in component docs/comments.
scripts/check-diagram-tokens.test.mjs Normalizes spelling in test descriptions/comments.
scripts/check-diagram-tokens.mjs Normalizes spelling in CLI help/output and rule labels/comments.
plugins/remark-graphviz.ts Normalizes spelling in plugin documentation comments.
plugins/remark-graphviz.test.ts Normalizes spelling in test descriptions/comments.
DESIGN.md Adds the diagrams design-system documentation and updates STRICT rules/self-correction entries.
AGENTS.md Adds diagrams guidance and documents the new safety check command.
Review details

Suppressed comments (2)

DESIGN.md:281

  • This code span includes triple backticks, which will terminate the inline code early and render incorrectly; wrap it with a longer backtick delimiter.
- ` ```dot ` fences, rendered by `plugins/remark-graphviz.ts`

DESIGN.md:457

  • This inline code span contains triple backticks (```), which breaks Markdown rendering; use a longer delimiter so the fence marker is shown as intended.
- A color attribute inside a ` ```dot ` fence → replace it with the role that says what the
  • Files reviewed: 10/10 changed files
  • Comments generated: 1
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread DESIGN.md Outdated
DESIGN.md did not contain the word "diagram", and that silence is the whole
reason this drifted: the diagram surface became the single largest violation of
this document's own STRICT rules — no hex outside `tokens.css`, every dark remap
in `theme.css`, components consume semantic tokens — while the document said
nothing about it either way. A lint stops a color reaching a fence; only this
explains what to write instead.

The new section carries the role table, the three layout kinds, how a role
becomes a surface/border/label, and the four surfaces that share the vocabulary.
It also records the two judgment calls a reader would otherwise have to
reverse-engineer: why `queued` and `mergify` share one accent, and why diagram
borders lift toward white on dark when "the product palette does not flip" three
sections above. The STRICT rules and the self-correction list gain the diagram
case, and the SVG-hex exemption now says explicitly that diagrams are not
covered by it.

`AGENTS.md` gets the short version and the new command, per this file's own
instruction to keep the two aligned.

Also normalizes the diagram code to the site's American spelling. The three
commits below this one were written in British English, which no other file in
the repo uses — and the lint prints its guidance to contributors of a site that
says "color". It is a style pass rather than a fix, folded in here because this
is the commit that codifies house style; it changes no behavior, and the tests
and the lint pass unchanged.

Change-Id: Id961fd51cd73f70a9cab6ebc897c457e6dbfc581
@jd
jd force-pushed the devs/jd/jd/ship-diagram-token-system/write-diagram-system-design-md--d961fd51 branch from ff56a82 to 5c91b2f Compare September 2, 2026 00:57
@jd

jd commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Revision history

# Type Changes Reason Date
1 initial ff56a82 2026-09-02 00:57 UTC
2 content ff56a82 → 5c91b2f review (#12649): widen the code-span delimiters around the dot fence examples in DESIGN.md 2026-09-02 00:57 UTC

@mergify
mergify Bot had a problem deploying to Mergify Merge Protections September 2, 2026 00:57 Failure
@jd

jd commented Sep 2, 2026

Copy link
Copy Markdown
Member Author

Re-pushed to address the review: ff56a825c91b2f (compare).

  • Double-backtick delimiters on the three ```dot code spans in DESIGN.md (lines 255, 281, 457).

Worth recording: the single-backtick form was not actually broken. CommonMark closes a code span on a backtick run of the same length as the opener, so the run of 3 inside was skipped — mdast-util-from-markdown produces byte-identical output for both forms. The wider delimiter is the correct idiom and removes a subtlety an editor could break, but nothing was rendering wrong.

pnpm check, pnpm test and pnpm build green on this commit on its own.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants