ci(DST-1526): lint docs prose with Vale in pre-commit and CI - #5779
ci(DST-1526): lint docs prose with Vale in pre-commit and CI#5779aromko wants to merge 7 commits into
Conversation
Encodes the prose rules that until now lived in review comments: no em dashes, no semicolons in prose, no en dash asides. `pnpm lint:prose` runs Vale over the docs site, the changesets and the published READMEs, wired into lint-staged and a CI check. Scope is prose written for a reader. Prose written for an agent (CLAUDE.md, .memory/, .claude/, docs/superpowers/, packages/*/src/**/README.md) is out, as is docs/legal/ and everything generated. Changesets are linted at the source, because the release notes they generate are not tracked. The binary is pinned to 3.19.0 and downloaded by scripts/install-vale.mjs with checksum verification, rather than via the @vvago/vale wrapper. The wrapper derives its download URL from its own package version and is stalled at 3.17.1, which has two defects that matter here: it ignores the table.cell scope exclusion, so it flags the em dash in `| — (no class) |`, and it reports only the first match per paragraph, missing 10 findings in this repo. Pinning 3.19.0 also means MDX parses natively, so no mdx2vast bridge and no `vale sync`. Notes for anyone changing this: - Vale matches the path sections in .vale.ini against the path it is handed, so an absolute path matches no section and lints with no rules. That is why the hook passes `--relative`; without it the gate silently passes everything. - Exclusion is an empty `BasedOnStyles =`. `= NONE` is a fatal error. - A path-section `*` spans `/`, so `[packages/*/README.md]` also matches packages/cli/src/lib/validate/README.md, hence the later `[packages/*/src/**]` section. - The lint-staged commands are one array, not two globs: separate glob entries run concurrently, which would race prettier's write against Vale's read. - Use `pnpm lint:prose`, not `pnpm exec vale`. pnpm exec puts a relative ./node_modules/.bin first on PATH, and Go refuses to resolve an executable through a relative PATH entry. lint-staged was previously undeclared and resolved through the npx cache, so it is now a devDependency. It pins 17.3.0 rather than latest because .npmrc sets minimum-release-age to 7 days and 17.4.1 is newer than that.
Fixes every violation the new prose lint reports, so the check starts green rather than accumulating suppressions as files are touched. 105 em dashes, 27 prose semicolons and 3 en dashes across 28 files. Punctuation only. The rewrites replace a dash or semicolon with a comma, a colon, or a sentence break, and four words change beyond that: two connectives where a dash was joining a list, one relative pronoun where a dash introduced a parenthetical, and a slash spelled out as "or". Line counts and code fences are unchanged in every file. Two thirds of the em dashes were the `- **Term** — definition` list form, which these docs already write as `- **Term**: definition` 122 times against 13 with a dash. One factual correction rides along: numericformat/index.mdx documented `"1,000 – 5,000"`, but Intl.NumberFormat.formatRange returns "1,000–5,000" with no spaces. The spaced en dash in dateformat/index.mdx and release-2025-09-01.mdx is left alone, because Intl.DateTimeFormat.formatRange really does emit U+2013 with spaces and those lines quote rendered output. That is the same call #5496 made by hand; the en dash rule now encodes it by matching only the aside form.
Adds a Prose Style section and one Workflow line, so an agent writes compliant prose from the start instead of learning the rules by being corrected. The rules are restated here rather than linked, because the load-bearing part is the scope paragraph and that has no home in a rule file. Without it the section reads as a repo-wide ban, and the agent starts stripping em dashes out of .memory/ and .claude/, where the density is deliberate and .claude/README.md positively requires the character in a skill description. The en dash rule carries its reasoning into the message: German uses a spaced en dash where English uses an em dash, so it is an easy slip for this team to make.
Drops the root `postinstall` hook. Nine workflows run `pnpm install` and none of them use Vale, while `prose.yml` — the one job that does — never runs it. Since `cache: 'pnpm'` caches the store rather than node_modules, the hook fired on all of them: about 176MB of release egress and 15s of runner time per push for a binary that was never executed, and a GitHub release outage could fail an install for Test and Release. The two call sites that need the binary already install it themselves, so the download moves to first use. `pnpm lint:prose` and the pre-commit hook each cost 34ms when the binary is already present. Also from the review pass: - `prose.yml` runs `pnpm lint:prose` instead of re-spelling it as two steps, so the local command and the CI check cannot drift apart. - `renovate.json` gains a regex custom manager for the pinned version. Every other external pin here sits where a manager reads it (`.node-version`, `packageManager`, action refs); this one was invisible, on the one tool whose release notes matter, since 3.18 changed MDX parsing and 3.19 changed JSX-children scoping. Those bumps are excluded from automerge for the same reason: green CI proves the current corpus passes, not that the scope semantics the rule comments rely on still hold. - `install-vale.mjs`: one `valeVersion()` helper replaces a single-use wrapper and a duplicated exec, and `fetchOrDie` drops a parameter that restated its own URL. The `existsSync` guard went with it; the catch it sat in already covers a missing, truncated or wrong-arch binary, which is what makes the self-heal work. - `Semicolon.yml` gains the `heading` scope its two siblings had, which was omitted with no stated reason. - `EnDash.yml` documents its regex, which is the least obvious in the set, and drops the semicolon from its own message — the sibling rule forbids exactly that, and a CI failure message is the prose developers read most often.
🦋 Changeset detectedLatest commit: 5f38f3e The changes in this PR will be included in the next version bump. This PR includes changesets to release 6 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
github-actions
Bot
added
type:docs
|
Accessibility tests executed. Download the report here. |
Coverage Report for Marigold Code Coverage
File CoverageNo changed files found. |
|
Accessibility tests executed. Download the report here. |
Reverts the prose job to the two explicit steps. Routing it through `pnpm lint:prose` read tidier but cost 16.5s of the job's 29s: pnpm 11 verifies dependencies before running a script, so a job with no `node_modules` installed the whole workspace — husky and the docs registry build included — to run a 0.2s check that needs none of it. The previous commit's claim that this job runs with "no `pnpm install` and no build" was made true again rather than left aspirational, and the two steps now carry a comment saying they have to stay in step with `lint:prose`. `CLAUDE.md` said CI "runs the same command", which the spelled-out steps make false in the literal sense; it now says the check runs the same rules. Also fixes the five violations that arrived with the merge from `main`, in changesets and Stepper docs written before this gate existed. Four semicolons and two em dashes, punctuation only. This is the standing cost of a repo-wide gate: work that lands between the cleanup and the merge brings its own violations, and the branch has to absorb them to stay green.
|
Accessibility tests executed. Download the report here. |
Removes every comment added by this branch across the installer, the Vale config, the three rule files, the workflow, the pre-commit hook and the gitignore entry. The reasoning they carried is in this branch's commit messages. Renovate's `description` keys are left in place: they are functional config that shows up in the PRs it opens, not annotations on code.
|
Accessibility tests executed. Download the report here. |
| .npmrc | ||
| .nvmrc | ||
| .prettierignore | ||
| .vale.ini |
There was a problem hiding this comment.
🔴 .vale/bin/ needs to be ignored here too, or pnpm format breaks for everyone.
.gitignore hides the downloaded binary from git, but Prettier does not read .gitignore, and prettier --write "**/*" matches dotted directories. That is exactly why .vale.ini had to be added on this line. Reproduced on this branch:
$ npx prettier --check "**/*"
[error] No parser could be inferred for file ".vale/bin/vale".
The sequence hits every developer: commit once, the pre-commit hook downloads .vale/bin/vale, then the next pnpm format exits non-zero. CLAUDE.md tells people to run it before committing. CI stays green because the Format job never installs Vale, so nothing catches this before it lands.
| .vale.ini | |
| .vale.ini | |
| .vale/bin/ |
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "*": "prettier --write" | |||
| "*": ["prettier --write", "node scripts/install-vale.mjs", ".vale/bin/vale"] | |||
There was a problem hiding this comment.
scripts/install-vale.mjs prints no pinned build for <key>; skipping and exits 0 when there is no asset for the platform, but the next command runs .vale/bin/vale unconditionally, so the commit fails with ENOENT instead of skipping.
Windows is the realistic case. There is no win32-x64 entry in ASSETS, and adding one would need zip extraction and a vale.exe name rather than tar xzf plus vale. As it stands a Windows contributor cannot commit at all.
Either add the win32 build to the script, or route the vale step through a small wrapper that no-ops when the binary is absent, so the graceful degradation the script already implements actually reaches the caller.
| @@ -1,3 +1,3 @@ | |||
| { | |||
| "*": "prettier --write" | |||
| "*": ["prettier --write", "node scripts/install-vale.mjs", ".vale/bin/vale"] | |||
There was a problem hiding this comment.
💡 Two smaller things about this line.
Scope it. The * glob hands every staged file to Vale, including PNGs, SVGs and pnpm-lock.yaml. Nothing matches a .vale.ini section so nothing is reported, but the hook pays for it on every commit. The installer also receives the same filename list and ignores it, which reads as a lint-staged task but is really a setup step, so .husky/pre-commit ahead of npx lint-staged is a better home for it.
| "*": ["prettier --write", "node scripts/install-vale.mjs", ".vale/bin/vale"] | |
| "*": "prettier --write", | |
| "*.{md,mdx}": ".vale/bin/vale" |
(pairs with moving node scripts/install-vale.mjs into the husky hook)
--relative is load-bearing and invisible. If that flag is what makes the repo-relative .vale.ini section globs match, then dropping it later makes Vale silently lint nothing and exit 0 rather than failing loudly. One sentence in the CLAUDE.md Prose Style section would keep that from being rediscovered the hard way.
| @@ -0,0 +1,16 @@ | |||
| --- | |||
| '@marigold/docs': patch | |||
There was a problem hiding this comment.
@marigold/cli needs a patch bump too.
npm publishes README.md with the package, and packages/cli/README.md is rewritten in 57 lines here (67 of the 135 fixes by your own count). Without a bump the npm page keeps the old prose indefinitely, so the rewrite never reaches the place the README is actually read.
| '@marigold/docs': patch | |
| '@marigold/docs': patch | |
| '@marigold/cli': patch |
The alternative is dropping {packages,themes,config}/*/README.md from .vale.ini scope, which would also remove half the prose churn from this PR.
| Three rules block CI: no em dashes, no semicolons in prose, and no en dash asides. Table | ||
| cells are exempt, because there an em dash is a legitimate "not applicable" marker, and | ||
| ranges keep the en dash so quoted component output stays accurate. All 135 existing | ||
| violations across 30 files are rewritten in this change, so the check starts green. |
There was a problem hiding this comment.
💡 The file count disagrees with itself. This says 30 files, the PR description says 28, and the branch touches 32 prose files (33 changed minus this changeset). Since this text ships to the published changelog, either fix the number or drop it and keep the violation count.
While you are in here: DST-1526 lists "Rewriting existing docs to pass" under Not included. I think overruling that is right, because vale . lints the whole repo, so fix-forward would leave the check red on day one no matter which files a PR touches. Worth saying so explicitly in the PR description so the reporter sees it was a decision rather than an oversight.
| Focus highlight marks the item the user is currently navigating to via keyboard. Unlike selection, which shows a confirmed choice, focus indicates where the user's cursor is right now. Two intensities handle different contexts: a subtle tint for menu items and drop zones, and a bolder fill for inline segments like date fields where the focused part needs to stand out clearly. | ||
|
|
||
| A focus highlight is not a focus indicator on its own. `focus-highlight` composites to around 1.05:1 against the surface it tints, well under the 3:1 that [WCAG 1.4.11](https://www.w3.org/WAI/WCAG22/Understanding/non-text-contrast.html) asks of a state indicator, and in a menu the same tint fires on hover because focus follows the mouse. Pair it with a focus ring — `ui-state-focus` where there is a border to recolor, `ui-state-focus-item` on borderless rows — and let the tint track the cursor while the ring proves where keyboard focus is. | ||
| A focus highlight is not a focus indicator on its own. `focus-highlight` composites to around 1.05:1 against the surface it tints, well under the 3:1 that [WCAG 1.4.11](https://www.w3.org/WAI/WCAG22/Understanding/non-text-contrast.html) asks of a state indicator, and in a menu the same tint fires on hover because focus follows the mouse. Pair it with a focus ring: `ui-state-focus` where there is a border to recolor, `ui-state-focus-item` on borderless rows, and let the tint track the cursor while the ring proves where keyboard focus is. |
There was a problem hiding this comment.
The opening dash turned into a colon and the closing dash into a comma, so and let the tint track the cursor now reads as the third item after the two token names rather than as the resumption of the sentence.
Keeping the closing boundary fixes it:
Pair it with a focus ring,
ui-state-focuswhere there is a border to recolor andui-state-focus-itemon borderless rows. Let the tint track the cursor while the ring proves where keyboard focus is.
This is the one prose edit in the set where the rewrite costs the reader something. The rest hold up well.
| #### Aligning with a bled container | ||
|
|
||
| `--bleed-px` is the one you want when content sits edge-to-edge and needs to line back up with the container's title. Any content area rendered with `bleed` publishes it — `Panel.Content`, `Panel.CollapsibleContent`, `Card.Content`, `Card.Footer` and `Drawer.Content` — always holding that container's own horizontal padding. The same areas clear it again when they are not bled, so it is in effect only inside the nearest bled container and never leaks into a nested one that keeps its padding. | ||
| `--bleed-px` is the one you want when content sits edge-to-edge and needs to line back up with the container's title. Any content area rendered with `bleed` publishes it: `Panel.Content`, `Panel.CollapsibleContent`, `Card.Content`, `Card.Footer` and `Drawer.Content`, always holding that container's own horizontal padding. The same areas clear it again when they are not bled, so it is in effect only inside the nearest bled container and never leaks into a nested one that keeps its padding. |
There was a problem hiding this comment.
💡 Same shape as the token-overview comment, milder. The colon opens a list of components and always holding that container's own horizontal padding now dangles off the end of it, so it reads as attaching to Drawer.Content rather than to all five. A period after Drawer.Content and a fresh clause ("Each holds that container's own horizontal padding.") would keep the scope clear.
| --- | ||
|
|
||
| **Marigold 11 is Here—Just in Time for Valentine's Day!** 💖 Happy Friday and Happy Valentine's Day! Whether you're celebrating with chocolates, flowers, or just a quiet night of coding, we've got something special for you—Marigold 11 is officially here! This release is packed with improvements, thoughtful refinements, and a few features we think you'll love. So, grab a heart-shaped cookie (or two), and let's dive into what's new! 🚀 | ||
| **Marigold 11 is Here: Just in Time for Valentine's Day!** 💖 Happy Friday and Happy Valentine's Day! Whether you're celebrating with chocolates, flowers, or just a quiet night of coding, we've got something special for you. Marigold 11 is officially here! This release is packed with improvements, thoughtful refinements, and a few features we think you'll love. So, grab a heart-shaped cookie (or two), and let's dive into what's new! 🚀 |
There was a problem hiding this comment.
💡 Consider excluding docs/content/releases/blog/** from the lint.
The generated release.mdx pages are already out of scope for being generated. These dated announcements are equally historical, and an em dash in "Marigold 11 is Here—Just in Time for Valentine's Day!" is idiomatic marketing typography rather than the slip the rule is aimed at. Restyling eight published posts is the one part of this cleanup that rewrites the past instead of the present, and the exclusion would shrink the diff by eight files.
Not blocking, and it is a judgement call rather than a defect.
| - list | ||
| - heading | ||
| tokens: | ||
| - '[a-zA-Z] – [a-z]' |
There was a problem hiding this comment.
💡 Noting the gap so it is on the record rather than rediscovered: the token misses an aside that opens with a capital or a digit, e.g. the API – React Aria wraps it – handles focus. Your PR description already explains that this is the price of keeping Reservix – Marigold and Jan 1 – Dec 31 legal, which is the right trade. No change requested.
| Releases are published by CI, not locally. `.github/workflows/release.yml` runs | ||
| [`changesets/action`](https://github.com/changesets/action) on every push to `main`: it either opens a | ||
| "release: version packages" PR that consumes the pending changesets, or — once that PR is merged — | ||
| "release: version packages" PR that consumes the pending changesets, or, once that PR is merged, |
There was a problem hiding this comment.
💡 Unrelated to this line, but this is the nearest anchor in the diff: the Command overview table above (around line 78) gains no pnpm lint:prose row, even though CLAUDE.md points people at the script. Worth a row next to pnpm lint.
2 participants
Description
Prose style rules for the docs were carried in people's heads and in review comments, so the AI relearned them by being corrected after the fact. This encodes them as Vale checks wired into the pre-commit hook and a CI check, with the writing guidance in
CLAUDE.md.Three rules block CI: no em dashes, no semicolons in prose, no en dash asides. Table cells are exempt, because there an em dash is a legitimate "not applicable" marker, and ranges keep the en dash so quoted component output stays accurate. Scope is prose written for a reader (
docs/content/**,.changeset/*.md, published READMEs, top-level markdown); prose written for an agent is deliberately excluded, as isdocs/legal/and everything generated.All 135 existing violations across 28 files are rewritten here, so the check starts green rather than accumulating suppressions as files are touched. Those edits are punctuation only: four words change beyond punctuation and capitalisation, and line counts and code fences are unchanged in every file.
N/A — no UI changes, so no screenshots and no VRT. No unit tests: the deliverable is a lint configuration, and its behaviour is verified by the check itself passing on this branch.
Closes DST-1526
Test Instructions
pnpm install, thenpnpm lint:prose— expect exit 0. First run downloads the pinned Vale 3.19.0 binary (~1s, checksum verified); later runs are ~200ms.docs/content/**/*.mdx, runpnpm lint:proseagain, and confirm it fails naming the file, line and rule.CLAUDE.mdor anything under.memory/instead, and confirm it does not block: agent-facing prose is out of scope on purpose.| — (no class) |.Breaking Changes
No
Checklist
component-testtag where applicable)pnpm changeset)Notes for the reviewer
Two things worth a closer look:
packages/cli/README.mdis 67 of the 135 fixes — half the prose churn in one file.The en dash rule is the one to scrutinise. It exists because German uses a spaced en dash where English uses an em dash, so it guards a slip this team is likely to make rather than one already made. Its token matches the aside form only (
[a-zA-Z] – [a-z]), so4–9,Jan 1, 2025 – Dec 31, 2025andReservix – Marigoldstay legal. That matters:Intl.DateTimeFormat.formatRangereally does emit a spaced U+2013, so the<DateFormat>doc lines quoting it are accurate and must not be "fixed". It is the same call #5496 made by hand, now enforced rather than remembered.Why the binary is downloaded rather than an npm dependency. The
@vvago/valewrapper derives its download URL from its own package version and is stalled at 3.17.1, which ignores thetable.cellexclusion and reports only the first match per paragraph (it under-reported this repo by 10 findings).scripts/install-vale.mjspins 3.19.0 and verifies the SHA-256, andrenovate.jsongains a regex custom manager so the pin is visible to Renovate, with those bumps held back from automerge since 3.18 and 3.19 both changed parsing semantics.