mirror of
https://github.com/coder/coder.git
synced 2026-09-01 14:53:15 +08:00
Nick Vigilante
ee3572ab9a
Wires Vale into docs CI as an advisory (non-blocking) prose-lint step. Closes [DOCS-40](https://linear.app/codercom/issue/DOCS-40). > **Integration update (rebased onto `main`).** Since this branch was opened, `main` consolidated docs linting into the **required** `lint-docs` job in `ci.yaml` and removed the standalone `docs-ci.yaml` ([#25608](https://github.com/coder/coder/pull/25608)). This PR adds Vale to that `lint-docs` job instead of resurrecting `docs-ci.yaml`, and the `docs/.style/` scaffold defers to the merged [#25466](https://github.com/coder/coder/pull/25466) (DOCS-180). Vale stays advisory. > **Post-review refactor.** Following the Coder Agents review, Vale is now invoked through `mise exec "aqua:errata-ai/vale"` (the same pattern as `actionlint`/`zizmor`) instead of a bespoke `curl`/`tar` download. This removed the GNU-only `grep -oP` version extraction and `uname`/arch mapping that broke on macOS BSD grep, and the prose step now skips paths a PR deletes. See the resolved review threads for CRF-17/19/20/21/22. A sample of what this check does is as follows: <img width="1443" height="1293" alt="image" src="https://github.com/user-attachments/assets/cf68dbf9-d9df-49ba-8dbf-200875bc289e" /> ## What changes - `.vale.ini` at the repo root: Google base + Coder (custom, empty in v1) + curated write-good. `alex` rules are pulled in a la carte. Inline comments justify every enable/disable. - `mise.toml`: pin Vale `3.7.1` via aqua. `mise.lock`: lock that pin across all platforms so `mise install --locked` (used by `build_image`) resolves it. - `Makefile`: a `docs/.style/.vale-synced` sentinel that gates `vale sync`, and a `lint/prose` target that runs `vale --no-exit`. Both invoke Vale via `mise exec "aqua:errata-ai/vale" -- vale ...`, so mise owns the version and the OS/arch download (no hand-rolled install path). - `.github/workflows/ci.yaml`: append Vale steps to the existing required `lint-docs` job: `Detect changed Markdown`, `Restore Vale styles`, `Prepare Vale styles` (`make docs/.style/.vale-synced`), `Vale prose lint`, and a default-branch-only `Save Vale styles`. They lint only changed Markdown under `docs/` that still exists on disk, with a problem matcher for inline PR annotations. - `.github/vale-problem-matcher.json`: parses `vale --output=line` so alerts surface as annotations on the Files Changed tab. - `.gitignore` and the workflow cache `path:`: use `docs/.style/styles/*` plus a `!docs/.style/styles/Coder` negation so adding a package does not require parallel edits. - `.markdownlint-cli2.jsonc`: ignore the synced styles so `make lint/markdown` does not lint upstream READMEs. Scaffold prose under `docs/.style/` and `.claude/docs/DOCS_STYLE_GUIDE.md` / `AGENTS.md` come from the merged DOCS-180; this PR no longer touches them. Net diff against `main` is the 8 Vale-wiring files only. ## Severity policy (v1) Rule severity reflects two things together: the rule's false-positive rate against real Coder docs and the gravity of the rule. Low FPs plus high gravity argues for `error`; lower gravity or more judgment calls argue for `warning` or `suggestion`. v1 lands most rules at `warning` and the wordiness rules at `suggestion`. A rule promotes to `error` only when (a) its false-positive rate against real content is effectively zero and (b) the existing-content violation count for that rule is also zero. Vale exits non-zero only on error-level alerts regardless of `MinAlertLevel`; the Makefile and CI invoke Vale with `--no-exit` so the baseline error count from un-overridden Google rules does not fail the build while real failures (bad config, missing files) still propagate. ## CI integration Vale runs as steps appended to the required `lint-docs` job in `ci.yaml`, gated on changed Markdown: 1. **`Detect changed Markdown`** (`tj-actions/changed-files`) scopes to changed `**.md`; the prose step re-filters to `docs/` (the `docs/**.md` glob silently skips dot-prefixed dirs and would miss `docs/.style/style-guide.md`). 2. **`Restore Vale styles`** (`actions/cache/restore`), keyed off `hashFiles('.vale.ini', 'mise.toml', 'docs/.style/styles/Coder/**')`. mise manages the Vale binary, so only the synced styles are cached. 3. **`Prepare Vale styles`** runs `make docs/.style/.vale-synced` (`mise exec ... vale sync`). 4. **`Vale prose lint`** filters the changed set to `docs/` paths still present on disk, then runs `mise exec ... vale --no-exit --output=line`, emitting inline annotations via the problem matcher. 5. **`Save Vale styles`** writes the cache, gated to `refs/heads/main` only so PR runs cannot poison the cache other branches restore from (the zizmor `cache-poisoning` concern). **Every Vale step is `continue-on-error: true`.** This is a deliberate change from the original standalone-workflow design: now that Vale lives inside the *required* `lint-docs` job, a transient `vale sync` network failure (or first-use `mise` install blip) would otherwise block merges. `continue-on-error` keeps Vale advisory, so only the markdownlint / table-formatter checks above (`pnpm check-docs`) remain merge-blocking. `vale --no-exit` additionally keeps the baseline error count from un-overridden Google rules from failing the step. ## Verification - `actionlint` clean on `ci.yaml` (local + `make lint/actions/actionlint`); `zizmor --persona regular` reports no findings. - `make lint/prose` on the full `docs/` corpus: ~406 errors, ~5,346 warnings, ~7,928 suggestions across 461 files, exit 0 (`--no-exit`), Vale `3.7.1` installed by mise. - Net diff vs `main` is the 8 Vale-wiring files only; the `docs/.style/` scaffold already matches `main`. <details> <summary>Implementation plan and decision log</summary> ### Why this rule set The Vale evaluation against the full docs corpus (measured 2026-05-18) produced ~43,940 raw violations across six candidate base styles. The selection here drops Microsoft and RedHat (overlap with Google, and RedHat's Spacing rule hammers technical IDs), and proselint (Annotations rule treats `> [!NOTE]` admonitions as TODO markers). Within the kept styles: - **Google** is the base. Disables: `EmDash` (conflicts with `make lint/emdash`), `Latin` (i.e./e.g. are fine for our audience), `Spacing` (4,500 errors on `codersdk.SomeType` patterns in the auto-generated API reference). Softened: `Parens` to `suggestion`, `WordList` to `warning`. - **write-good** is the base, with `Passive` and `E-Prime` off. `TooWordy` and `ThereIs` are suggestions; `Weasel` is a warning. - **alex** is cherry-picked (not in `BasedOnStyles`): `Ablist`, `Condescending`, `LGBTQ`, `ProfanityLikely`, `Race`, `Suicide` at warning. The `ProfanityMaybe`/`ProfanityUnlikely` rules trip on `execute`, `kill`, `failed`, and `attack`, which read as technical vocabulary in our context. - **Coder** is in `BasedOnStyles` but the directory is empty in v1. Rules land through the per-rule tickets in the [Docs style guide](https://linear.app/codercom/project/docs-style-guide-7828445b9afc) project. ### Why `mise exec` instead of a download block Vale is pinned in `mise.toml` like `actionlint` and `zizmor`, so invoking it via `mise exec "aqua:errata-ai/vale" -- vale ...` makes the pin the single source of truth and lets mise handle the OS/arch-specific download. This replaced an earlier ~30-line `curl`/`tar` block whose GNU-only `grep -oP ...\K` version extraction returned empty on macOS BSD grep. Note: the bare `vale` short name in `mise exec` ignores the pin and resolves to the latest release, so the full aqua key is required. ### Why `vale sync` instead of vendoring The three style packages weigh ~272 KB combined, so vendoring is cheap. But Vale's ecosystem treats `Packages = ` + `vale sync` as canonical, the upstream LICENSE files are not in the package tarballs (would need to be added manually), and the CI cache makes the sync nearly free after the first run. Sticking with the canonical pattern keeps the repo lean and the upgrade path obvious. ### Why `lint/prose` is not in `lint:` or `lint-light:` Vale on the full docs corpus takes ~20s on cold caches. Forcing every pre-commit through that would be aggressive for a feature that ships as warnings. `make lint/typos` follows the same pattern (it is in `lint-light` but not `lint`; CI invokes it directly). v1 keeps Vale opt-in locally and CI-only by default; promote to `lint:` once the rule set stabilizes. ### Exit-code handling Two mechanisms combine, and the choice changed when the step moved into the required `lint-docs` job: - `vale --no-exit` suppresses Vale's non-zero exit on alerts, so the baseline error-level violations from un-overridden Google rules do not fail the step while the cleanup PRs land. Real failures (config invalid, file missing) still exit non-zero. - `continue-on-error: true` on every Vale step. Because the steps now run inside the *required* `lint-docs` job, a `vale sync` download/network blip must not block merges. The original (standalone, non-required) design rejected `continue-on-error` for showing a misleading yellow badge; in a required job that tradeoff flips, and advisory-yellow is strictly preferable to merge-blocking-red on an infrastructure flake. `|| true` in the Makefile was also rejected: it swallows missing-config failures indiscriminately. ### Pre-mortem - **Generated docs noise**: `docs/reference/` is dominated by auto-generated content (clidocgen, apidocgen, auditdocgen, metricsdocgen). The architectural decision is to fix the generators, not exclude paths in Vale. Google.Spacing is the only rule silenced specifically to defer the generator fix; everything else surfaces as warnings. - **First-run cost**: `mise` installs the pinned Vale (a single small binary) and `vale sync` pulls the style packages on a cold run. The Actions cache keyed off `hashFiles('.vale.ini', 'mise.toml', 'docs/.style/styles/Coder/**')` makes subsequent runs near-instant; the `Coder/**` hash is defense-in-depth against [actions/toolkit#713](https://github.com/actions/toolkit/issues/713) so a future cache release that regresses path-negation cannot serve a stale `Coder/` from cache. - **Required-job blast radius**: moving Vale into the required `lint-docs` job means any Vale step failure would gate merges. Mitigated by `continue-on-error` on all Vale steps plus a clean skip when no changed `docs/` Markdown remains on disk, so only `pnpm check-docs` stays blocking. - **Cross-platform install**: handled by mise (aqua backend) rather than a hand-rolled `uname`/arch map, which removes the macOS BSD-grep break the review flagged. - **Deleted files**: `all_changed_files` is ACMRD and lists paths a PR removes; the prose step filters to files still present on disk so Vale does not error on a missing file. - **Local-vs-CI parity**: CI lints changed files only; local `make lint/prose` lints the full tree. This mirrors `make lint/markdown` (full tree) vs the changed-files CI step. Acceptable for v1. </details> --- *Filed via [Coder Agents](https://coder.com/docs/ai-coder/agents) on Nick's behalf.*
12 lines
321 B
JSON
12 lines
321 B
JSON
{
|
|
"ignores": [
|
|
"PLAN.md",
|
|
// Synced Vale packages (pulled by `vale sync`). Keep aligned with the
|
|
// Packages directive in .vale.ini so markdownlint never lints upstream
|
|
// style files we did not author.
|
|
"docs/.style/styles/Google/**",
|
|
"docs/.style/styles/alex/**",
|
|
"docs/.style/styles/write-good/**"
|
|
],
|
|
}
|