Skip to content

feat: wire Vale prose linter into docs CI#25467

Merged
nickvigilante merged 13 commits into
mainfrom
vigilante/docs-40-add-vale-to-docs-ci-with-starter-microsoftgoogle-style-coder
Jun 22, 2026
Merged

feat: wire Vale prose linter into docs CI#25467
nickvigilante merged 13 commits into
mainfrom
vigilante/docs-40-add-vale-to-docs-ci-with-starter-microsoftgoogle-style-coder

Conversation

@nickvigilante

@nickvigilante nickvigilante commented May 18, 2026

Copy link
Copy Markdown
Contributor

Wires Vale into docs CI as an advisory (non-blocking) prose-lint step. Closes 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). 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 (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:

image

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.
Implementation plan and decision log

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

Filed via Coder Agents on Nick's behalf.

Adds a private contributor-tooling directory at docs/.style/ that holds:
- README.md explaining the convention and the no-deploy guarantee
- style-guide.md as a scaffold for the canonical prose style guide
- styles/Coder/ as the home for custom Vale rules (filled by follow-ups)

Defense-in-depth tweaks to .github/workflows/deploy-docs.yaml exclude
docs/.style/** from the push trigger and from the surgical-reindex git
diff. coder.com/docs route discovery is already manifest-driven, so
nothing under docs/.style/ becomes a route or an Algolia record.

Also:
- .github/.linkspector.yml: skip external-link checks under docs/.style/
- AGENTS.md: point agents at the new style guide
- .claude/docs/DOCS_STYLE_GUIDE.md: cross-link to the canonical prose
  guide; this file remains the structure/research companion

The Vale configuration that consumes docs/.style/styles/ lands in a
follow-up PR (DOCS-40).

Closes DOCS-180.
Lands the Vale prose linter as a non-blocking docs CI step. Builds on the
scaffold from DOCS-180 (#25466):

- .vale.ini at the repo root configures Google + curated write-good
  + cherry-picked alex rules. Disables Google.Spacing (false positives
  on codersdk type names in the auto-generated API reference),
  Google.EmDash (conflicts with our em-dash ban), Google.Latin (i.e.
  and e.g. are fine), write-good.Passive and write-good.E-Prime
  (judgment-heavy).
- mise.toml pins Vale 3.7.1 via aqua.
- Makefile adds build/vale-$VERSION install, docs/.style/.vale-synced
  sentinel that runs 'vale sync' once per .vale.ini change, and a
  lint/prose target wrapped in '|| true' for v1 non-blocking severity.
- .github/workflows/docs-ci.yaml adds a 'prose' step that lints only
  the changed Markdown files under docs/, with continue-on-error: true
  and a cache for the synced styles and binary.
- .gitignore excludes the synced upstream styles and the sentinel.
- .markdownlint-cli2.jsonc ignores the synced styles so local
  markdownlint runs do not lint upstream READMEs.
- docs/.style/README.md and style-guide.md document how to run Vale
  locally and what the active rule set is.

Severity policy (v1): every rule lands at 'warning'. CI is non-blocking
through continue-on-error: true. A rule promotes to 'error' only when
(a) it is objectively correct and (b) the existing-content violation
count reaches zero. Judgment rules stay at 'suggestion'.

Local run on the full docs corpus produces 0 blocking failures, 391
errors, 5202 warnings, 7682 suggestions across 458 files in ~20s.

Refs DOCS-40.
@github-actions

github-actions Bot commented May 18, 2026

Copy link
Copy Markdown

Docs preview

📖 View docs preview for docs/about/contributing/documentation.md

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

This PR adds a new docs-ci.yaml step, a new Makefile target, and a new mise.toml pin, so it is squarely "CI changes in a meaningful way." Focus areas:

  1. .github/workflows/docs-ci.yaml — the new prose step relies on continue-on-error: true being honored even when Vale exits non-zero. Confirm the cache key (hashFiles('.vale.ini', 'mise.toml')) is the right invalidation surface.
  2. Makefile lint/prose and the docs/.style/.vale-synced sentinel pattern. The find build/ -maxdepth 1 -name 'vale-*' -type f -print -quit in CI replaced a ls | head that shellcheck flagged as SC2012; sanity-check the CI invocation.
  3. .vale.ini rule-by-rule justifications. Anything missing from the disables list that we should also silence at v1?
  4. The || true + continue-on-error: true double-layer for v1 non-blocking; both come off when rules promote to error. Reasonable progression?

Base branch is vigilante/docs-180-... (PR 1 #25466), not main. Will rebase onto main once that one merges.

Filed via Coder Agents on Nick's behalf.

… filter

The 'docs/**.md' glob in tj-actions/changed-files skips dot-prefixed
directories by default, so the changed-md-docs filter silently dropped
docs/.style/README.md and docs/.style/style-guide.md. The Vale prose
step never fired on PR #25467 as a result.

Drop the second changed-files step and post-filter the changed-md
output in shell. grep '^docs/' keeps only docs paths; grep -v
'^docs/.style/styles/' excludes the synced upstream packages. The
early exit handles PRs that only touched non-docs markdown.

Refs DOCS-40.
Vale's release archive places 'vale' at the archive root with no
leading ./ (unlike typos), so 'tar -xzf - ./vale' matched nothing and
produced 'tar: ./vale: Not found in archive' on CI. Switch to 'vale'.

The local Makefile invocation worked before because build/vale-3.7.1
was already present from the spike artifacts. CI hit the cold path
and exposed the bug.

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Well-structured linter integration. The inline comments in .vale.ini justifying each enable/disable decision set a high bar for configuration documentation. The Makefile patterns mirror lint/typos cleanly, the CI caching is thoughtful, and the decision log in the PR description is unusually thorough.

As Hisoka put it: "Two config points that look independent, secretly attached through Vale's exit-code semantics. Whoever follows the documented path gets a surprise."

Severity breakdown: 4 P2, 5 P3, 1 P4, 2 Nit.

The P2 findings cluster around two themes: (1) Vale's exit-code behavior is different from what the PR documents (only errors cause non-zero exit, not warnings/suggestions), which inverts the described promotion path; and (2) operational blind spots where continue-on-error swallows infrastructure failures alongside lint warnings.

The P3 findings are mostly documentation accuracy issues and a maintenance friction point (package names hardcoded in 4 files that could use wildcards).

🤖 This review was automatically generated with Coder Agents.

Comment thread docs/.style/README.md Outdated
Comment thread docs/.style/style-guide.md Outdated
Comment thread .github/workflows/docs-ci.yaml Outdated
Comment thread .vale.ini Outdated
Comment thread .vale.ini Outdated
Comment thread docs/.style/style-guide.md Outdated
Comment thread .github/workflows/docs-ci.yaml Outdated
Comment thread docs/.style/style-guide.md Outdated
Comment thread Makefile Outdated
Comment thread .vale.ini Outdated
Resolves the 12 findings from review id 4314529409 on PR #25467.

DEREM-1 (P2): fix `./build/vale-*/vale` glob in docs/.style/README.md;
  the binary is the file itself, not a directory.
DEREM-2 (P3): drop the inaccurate `make lint` claim in style-guide.md;
  point readers at README's Running Vale section instead.
DEREM-3 (P2): split the prose step into `Prepare Vale styles` (no
  continue-on-error) and `prose` (no continue-on-error; --no-exit), so
  the job fails on sync failures but lints non-blocking.
DEREM-4 (P2): replace `|| true` with Vale's native --no-exit in the
  Makefile and rewrite the severity-policy comment to match the
  measured exit-code semantics (Vale exits non-zero only on error
  alerts, regardless of MinAlertLevel).
DEREM-5 (P2): add `Coder` to BasedOnStyles so the empty starter style
  is loaded and ready to receive rule files.
DEREM-6 (P3): replace hardcoded per-package paths in .gitignore and the
  workflow cache step with `styles/*` plus a `!styles/Coder` negation,
  reducing the places future packages need to be listed.
DEREM-7 (P3): already addressed by c3e9e2f (prose filter matches
  docs/.style paths) and 6054d9e (tar layout).
DEREM-8 (P3): remove the duplicate Running Vale block in style-guide.md
  and cross-reference README.md as the single source.
DEREM-9 (P3): add .github/vale-problem-matcher.json and wire the prose
  step with `::add-matcher::`/`--output=line`/`::remove-matcher::` so
  alerts surface as inline PR annotations.
DEREM-10 (P4): trim the Active rule set section in style-guide.md to a
  policy summary plus a pointer to .vale.ini.
DEREM-11 (Nit): rephrase the Makefile sync-sentinel comment to describe
  Make's behavior instead of suggesting `touch`.
DEREM-12 (Nit): swap "(Vale spike, 2026-05-18)" for the more neutral
  "(measured 2026-05-18)" in the Google.Spacing rationale.

Two-layer defense on the cache step protects the hand-authored
Coder rules: the negation excludes styles/Coder from the cached paths,
and the cache key hashes styles/Coder/** so any rule change invalidates
the cache even if the negation behavior ever regresses
(actions/toolkit#713, actions/cache#494).

Verification:
- make lint/actions: clean (zizmor, actionlint).
- make fmt/markdown: no changes.
- make lint/markdown: 0 errors across 463 files.
- make lint/prose: exit 0; baseline 391 errors / 5201 warnings / 7673
  suggestions across 458 files (consistent with the pre-review run).

Copy link
Copy Markdown
Contributor Author

Review iteration: addressed all 12 findings

Commit e49b5ccedf resolves the 12 findings from review #4314529409. Each comment has an inline reply with the per-finding fix.

ID Severity File(s) Resolution
DEREM-1 P2 docs/.style/README.md Fixed glob: ./build/vale-*/vale./build/vale-*.
DEREM-2 P3 docs/.style/style-guide.md Removed false make lint claim; section now cross-references README.
DEREM-3 P2 .github/workflows/docs-ci.yaml Split into Prepare Vale styles (no continue-on-error) + prose (no continue-on-error, --no-exit). Infra failures break CI; alert exits don't.
DEREM-4 P2 .vale.ini, Makefile, docs/.style/style-guide.md Replaced || true with --no-exit; rewrote severity policy to reflect Vale's real exit-code model.
DEREM-5 P2 .vale.ini BasedOnStyles = Google, write-good, Coder. Empty Coder/ directory is now loaded.
DEREM-6 P3 .gitignore, docs-ci.yaml cache Wildcard + !Coder negation; defense-in-depth Coder/** hash in cache key.
DEREM-7 P3 docs-ci.yaml Already fixed by earlier commit c3e9e2ffd8.
DEREM-8 P3 docs/.style/style-guide.md Duplicate Running Vale section removed; now a cross-reference.
DEREM-9 P3 .github/vale-problem-matcher.json (new), docs-ci.yaml Problem matcher + --output=line for inline PR annotations.
DEREM-10 P4 docs/.style/style-guide.md Active rule set trimmed to summary + pointer to .vale.ini.
DEREM-11 Nit Makefile "Touch any time…" → "Make rebuilds this target when .vale.ini changes."
DEREM-12 Nit .vale.ini "(Vale spike, 2026-05-18)" → "(measured 2026-05-18)".

Verification

  • make lint/actions: clean (zizmor + actionlint).
  • make fmt/markdown: no changes.
  • make lint/markdown: 0 errors across 463 files.
  • make lint/prose: exit 0; baseline 391 errors / 5201 warnings / 7673 suggestions across 458 files (consistent with the pre-review measurement).
  • make pre-commit: passed (23s).

Follow-ups (out of scope for this PR)

  • DOCS-185: investigate Vale sequence and script rule types.
  • DOCS-186: redirect docs/about/contributing/documentation.md to docs/.style/style-guide.md (from the DEREM-5 follow-up Mafuuu raised).

Filed via Coder Agents on Nick's behalf.

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Filed via Coder Agents on Nick's behalf.

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All 12 R1 findings addressed in e49b5cc. The fixes are thorough: the exit-code model is correct in .vale.ini, Makefile, and style-guide.md; the CI step split cleanly separates infra failures from lint warnings; the problem matcher surfaces annotations inline; the wildcard gitignore and cross-referenced docs eliminate maintenance coupling.

Two locations still carry the pre-correction --no-exit explanation (DEREM-14, DEREM-15), and the CI shell filter has a semantic gap (DEREM-13). Severity: 2 P3, 1 Nit.

As Kite noted: "The block comment explaining Vale's CI behavior is unusually thorough. It documents the split-step rationale, the --no-exit semantics, the problem matcher lifecycle, and the cache negation defense-in-depth strategy with specific issue citations."

Process note: the PR description still describes the removed || true and continue-on-error: true mechanisms. The code comments are all correct, but the description will persist as the merge commit summary. Consider updating the description to reflect the current --no-exit + step-split architecture before merge.

🤖 This review was automatically generated with Coder Agents.

Comment thread .github/workflows/docs-ci.yaml Outdated
Comment thread docs/.style/README.md Outdated
Comment thread .github/workflows/docs-ci.yaml Outdated
Three follow-ups from review id 4320840969.

DEREM-13 (P3): drop the redundant `grep -v '^docs/\.style/styles/'`
filter from the prose step. Synced upstream packages are gitignored, so
tj-actions/changed-files never lists them; the filter only ever rejects
tracked Coder/**.md files, which is the opposite of what we want. The
inline comment now documents why no second filter exists.

DEREM-14 (P3): fix the README's --no-exit explanation. The DEREM-4 fix
chain reached .vale.ini, the Makefile, and style-guide.md but missed
docs/.style/README.md. The README now matches the rest: --no-exit
suppresses the exit from the baseline error count produced by
un-overridden Google error-level rules, not from warnings/suggestions
(which never trigger non-zero exit regardless of --no-exit).

DEREM-15 (Nit): same model fix in the docs-ci.yaml prose-step comment.

Verification: make lint/actions, make fmt/markdown, make lint/markdown,
make lint/prose all clean. Vale baseline unchanged (391 errors / 5201
warnings / ~7673 suggestions across 458 files).

Copy link
Copy Markdown
Contributor Author

R2 follow-ups addressed

Commit 0048444e99 resolves the 3 findings from review #4320840969.

ID Severity File Resolution
DEREM-13 P3 .github/workflows/docs-ci.yaml Removed broken grep -v filter; gitignore is single source of truth for synced packages. Coder/**.md now lints.
DEREM-14 P3 docs/.style/README.md Fixed --no-exit model to match the rest of the fix chain.
DEREM-15 Nit .github/workflows/docs-ci.yaml Same model fix in the prose-step comment block.

Process note response

The PR description was already rewritten earlier this session to remove the stale || true / continue-on-error: true / severity-model claims. The bot reviewed an older description snapshot. Current description correctly describes --no-exit and the step split.

Verification

  • make lint/actions: clean (zizmor + actionlint).
  • make fmt/markdown: no changes.
  • make lint/markdown: 0 errors across 463 files.
  • make lint/prose: exit 0; baseline 391 errors / 5201 warnings / 7673 suggestions across 458 files.
  • make pre-commit-light: passed (6s).

Filed via Coder Agents on Nick's behalf.

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Filed via Coder Agents on Nick's behalf.

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All 15 findings from R1 and R2 addressed and verified by Netero + panel. The fixes are thorough and consistent: the --no-exit model is now correct across all five locations, the CI step split properly isolates infra failures, the problem matcher surfaces annotations inline, and the PR description matches the implementation.

One new Nit (DEREM-16) on the README escape-hatch command, otherwise clean.

As Hisoka put it: "Three rounds. Fifteen fixes. I pulled every thread I could find across the diff... Every thread either held or was guarded by a layer the author already placed. The code earned the silence."

🤖 This review was automatically generated with Coder Agents.

Comment thread docs/.style/README.md Outdated
DEREM-16 (Nit) from review id 4321386259. `./build/vale-* docs/` is fine
when only one vale-X.Y.Z binary exists in build/, but if a developer
bumps the version in mise.toml without running `make clean`, multiple
binaries coexist and the glob expands to multiple positional args,
breaking the command. Low-risk path because three conditions have to
align (version bump + no clean + manual invocation), but worth a sentence
so a developer who hits it knows to `make clean`.
@github-actions github-actions Bot added the stale This issue is like stale bread. label Jun 4, 2026
@github-actions github-actions Bot closed this Jun 8, 2026
@nickvigilante nickvigilante removed the stale This issue is like stale bread. label Jun 12, 2026
@nickvigilante nickvigilante reopened this Jun 12, 2026
@nickvigilante nickvigilante force-pushed the vigilante/docs-180-build-the-docs-style-guide-scaffold-in-docsstyle-private-to branch 2 times, most recently from c184080 to 3e26bed Compare June 17, 2026 12:52
Base automatically changed from vigilante/docs-180-build-the-docs-style-guide-scaffold-in-docsstyle-private-to to main June 17, 2026 13:19
…tarter-microsoftgoogle-style-coder

Reconcile Vale CI wiring with main's refactored docs lint:

- main (#25608) deleted the standalone docs-ci.yaml and moved docs lint into
  the required lint-docs job in ci.yaml. Integrate Vale there (advisory,
  continue-on-error so a vale sync blip never blocks the required job)
  instead of resurrecting docs-ci.yaml.
- Delete docs-ci.yaml; keep .vale.ini, Makefile vale targets, mise.toml,
  vale-problem-matcher.json, .linkspector.yml, .markdownlint-cli2.jsonc,
  .gitignore vale entries.
- deploy-docs.yaml: keep main's release(published) trigger plus this branch's
  docs/.style/** reindex exclusions.
- Scaffold prose (docs/.style/*, .claude/docs/DOCS_STYLE_GUIDE.md, AGENTS.md):
  take the merged DOCS-180 canonical versions.

Generated with Coder Agents on Nick's behalf.
@nickvigilante

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

Context for the reviewer: this branch was rebased onto main after the scaffold (#25466) merged. main had since deleted the standalone docs-ci.yaml and moved docs lint into the required lint-docs job in ci.yaml (#25608), so Vale is now integrated into that job as four continue-on-error (advisory) steps rather than a standalone workflow. Net diff vs main is the 7 Vale-wiring files only. Please pay particular attention to the ci.yaml lint-docs changes and the advisory/non-blocking semantics.

Requested by Coder Agents on behalf of @nickvigilante.

@coder-agents-review

coder-agents-review Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Chat: Review posted | View chat
Requested: 2026-06-17 14:36 UTC by @nickvigilante
Spend: $89.98 / $100.00

Review history
  • R4 (2026-06-17): 10 reviewers, 6 Nit, 5 P2, 9 P3, 1 P4, COMMENT. Review
  • R5 (2026-06-17), 6 Nit, 5 P2, 9 P3, 1 P4, COMMENT. Review
  • R6 (2026-06-17): 5 reviewers, 7 Nit, 5 P2, 10 P3, 1 P4, APPROVE. Review

deep-review v0.7.1 | Round 6 | 182bdc8..30072f1

Last posted: Round 6, 23 findings (5 P2, 10 P3, 1 P4, 7 Nit), APPROVE. Review

Finding inventory

Findings

# Sev Status Location Summary Round Reviewer Posted
DEREM-1 P2 Author fixed (e49b5cc) docs/.style/README.md:85 Broken shell glob: ./build/vale-*/vale treats binary as directory R1 Netero P3, Mafu-san P2 Yes
DEREM-2 P3 Author fixed (e49b5cc) docs/.style/style-guide.md:95 False claim: ` trueprevents breakingmake lint` but lint/prose is not a dependency of lint: R1
DEREM-3 P2 Author fixed (e49b5cc) .github/workflows/docs-ci.yaml:108 continue-on-error blankets infra setup + lint in one step; download/sync failure invisible R1 Hisoka P2 Yes
DEREM-4 P2 Author fixed (e49b5cc) .vale.ini:8 Wrong exit-code model: Vale only exits non-zero on errors, not warnings; promotion path is backwards R1 Pariston P2, Leorio P3, Razor P3, Gon P3, Mafuuu Nit Yes
DEREM-5 P2 Author fixed (e49b5cc) .vale.ini:27 BasedOnStyles omits Coder; parent PR README claims it exists; style-guide.md lists as active R1 Mafuuu P2, Leorio P3 Yes
DEREM-6 P3 Author fixed (e49b5cc) .gitignore:21 Package names hardcoded in 4 files; wildcard patterns reduce to 2 R1 Kite P3, Zoro P3 Yes
DEREM-7 P3 Author fixed (c3e9e2f) .github/workflows/docs-ci.yaml:74 CI comment claims .vale.ini scopes to docs/; config uses [*.md] and scopes to all markdown R1 Razor P3 Yes
DEREM-8 P3 Author fixed (e49b5cc) docs/.style/style-guide.md:91 Duplicated "Running Vale locally" with divergent content; has already drifted within this PR R1 Robin Nit, Razor Nit, Zoro P3 Yes
DEREM-9 P3 Author fixed (e49b5cc) .github/workflows/docs-ci.yaml:113 CI output not surfaced as annotations; no problem matcher means warnings nobody sees R1 Luffy P3 Yes
DEREM-10 P4 Author fixed (e49b5cc) docs/.style/style-guide.md:109 Active rule set section duplicates .vale.ini comments; will drift R1 Robin P3, Zoro P4 Yes
DEREM-11 Nit Author fixed (e49b5cc) Makefile:875 "Touch any time" comment misleading; Make handles this automatically R1 Gon Nit, Leorio P3 Yes
DEREM-12 Nit Author fixed (e49b5cc) .vale.ini:48 "Vale spike" jargon; "(measured 2026-05-18)" is clearer R1 Gon Nit Yes
DEREM-13 P3 Author fixed (0048444) .github/workflows/docs-ci.yaml:131 grep -v excludes tracked Coder files; synced packages already gitignored R2 Netero Yes
DEREM-14 P3 Author fixed (0048444) docs/.style/README.md:80 --no-exit explanation wrong: attributes to warning/suggestion suppression; DEREM-4 fix missed this file R2 Zoro P3, Hisoka Nit Yes
DEREM-15 Nit Author fixed (0048444) .github/workflows/docs-ci.yaml:89 Same wrong --no-exit explanation as DEREM-14 in CI comment R2 Zoro P3, Hisoka Nit Yes
DEREM-16 Nit Author fixed (d394b14) docs/.style/README.md:87 Shell glob ./build/vale-* expands to multiple args with stale binaries in build/ R3 Mafuuu P3 Yes
CRF-17 P2 Author fixed (30072f1) Makefile:833 grep -oP with PCRE \K is GNU-only; breaks VALE_VERSION on macOS R4 Netero P2, Mafu-san P2 Yes
CRF-18 P3 Author fixed (4b8f87c) .github/workflows/ci.yaml:189 actions/cache pinned to v4.3.0 while rest of repo uses v5.0.5 R4 Netero P3 Yes
CRF-19 P3 Author fixed (30072f1) .github/workflows/ci.yaml:209 all_changed_files includes deleted paths; Vale errors on missing files; advisory signal degrades R4 Hisoka P3 Yes
CRF-20 P3 Author fixed (30072f1) Makefile:833 Manual binary download duplicates mise; eliminates CRF-17 and follows actionlint/zizmor pattern R4 Zoro P3, Pariston Note Yes
CRF-21 Nit Author fixed (30072f1) .markdownlint-cli2.jsonc:4 Hardcoded package names need JSONC comment documenting coupling to .vale.ini Packages R4 Leorio P3 Yes
CRF-22 Nit Author fixed (30072f1) .vale.ini:56 k.R example opaque; use a real Coder example like codersdk.SomeType R4 Leorio Nit Yes

Contested and acknowledged

(none)

Round log

Round 1

Panel. Netero: 2 P3 (folded into panel findings). Panel: 4 P2, 5 P3, 1 P4, 2 Nit. 0 dropped. Reviewed against cb723c3..4cb4d63.

Round 2

Panel. DEREM-1 through DEREM-12 all addressed. 2 P3, 1 Nit new. Reviewed against cb723c3..e49b5cc.

Round 3

Panel. DEREM-13 through DEREM-15 all addressed. 1 Nit new. Reviewed against cb723c3..0048444.

Round 4

Panel (post-rebase). DEREM-16 addressed. PR restructured: Vale moved from standalone docs-ci.yaml to required lint-docs job in ci.yaml. 1 P2, 3 P3, 2 Nit new. Reviewed against 182bdc8..55e7ab0.

Round 5

BLOCKED. CRF-18 addressed. 5 silent (CRF-17, CRF-19, CRF-20, CRF-21, CRF-22). No review.

Round 6

Panel. CRF-17 through CRF-22 all addressed. 1 P3, 1 Nit new. 1 dropped (Mafuuu P3 factually wrong: Vale --output=line group 4 is rule name, not severity). Reviewed against 182bdc8..30072f1.
| CRF-23 | P3 | Open | .vale.ini:21 | Stale "missing binary" in comment after mise exec eliminated that failure mode | R6 | Mafu-san P3 | Yes |
| CRF-24 | Nit | Open | .github/workflows/ci.yaml:183 | Comment says docs/**.md skips dot-dirs; real issue is picomatch single-segment matching | R6 | Hisoka Nit | Yes |

About deep-review

CRF = Coder Review Finding (P0-P4, Nit, Note)

Reviewer Focus
Bisky tests
Chopper ops/errors
Churn-guard change verification
Ging language modernization
Gon naming
Hisoka edge cases
Killua perf
Kite change integrity
Knov contracts
Knuckle SQL
Kurapika security
Law decomposition
Leorio docs
Luffy product
Mafu-san process
Mafuuu contracts
Melody dispatch/pairing
Meruem structural
Nami frontend
Netero mechanical checks
Pariston premise testing
Pen-botter product gaps
Razor verification
Robin duplication
Ryosuke Go arch
Takumi concurrency
Zoro shape

🤖 Managed by Coder Agents.

…ning

Merging main surfaced two CI failures from wiring Vale into the required lint-docs job:

- mise install --locked failed because aqua:errata-ai/vale@3.7.1 was in mise.toml but missing from mise.lock. Added all 11 platform entries via mise lock.
- zizmor flagged the Vale styles cache (actions/cache) as a cache-poisoning risk. Split it into actions/cache/restore plus an actions/cache/save gated to pushes on main, matching the existing golangci-lint cache pattern in this workflow.

Generated with Coder Agents on Nick's behalf.

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Post-rebase review. The restructuring from standalone docs-ci.yaml to the required lint-docs job in ci.yaml is well-reasoned: continue-on-error: true on every Vale step keeps the advisory linter from blocking merges, while the split-step design preserves diagnostic visibility. The PR description was rewritten to match the new integration point, and all 16 prior findings remain addressed in the rebased code.

Two themes in the new findings: (1) the Makefile's grep -oP for version extraction is GNU-only and silently breaks Vale on macOS (CRF-17), and (2) the CI file list includes deleted paths that cause Vale to error on missing files, degrading the advisory signal (CRF-19). Both have one-line fixes.

As Hisoka put it: "The advisory system trains its users to stop reading it."

Severity: 1 P2, 3 P3, 2 Nit.

CRF-20 proposes a structural alternative: replacing the ~30-line binary-download block with mise exec vale -- (8 lines), following the pattern actionlint and zizmor already use in this Makefile. This would also eliminate CRF-17 as a side effect. Worth considering.

🤖 This review was automatically generated with Coder Agents.

Comment thread Makefile Outdated
Comment thread .github/workflows/ci.yaml Outdated
Comment thread .github/workflows/ci.yaml
Comment thread Makefile Outdated
Comment thread .markdownlint-cli2.jsonc
Comment thread .vale.ini Outdated
@nickvigilante

Copy link
Copy Markdown
Contributor Author

/coder-agents-review

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Five of six R4 findings have no author response or code change. CRF-18 (actions/cache version) was addressed in 4b8f87c. Further review is blocked until the remaining findings are addressed or acknowledged.

Open findings requiring a response:

  1. CRF-17 (P2) Makefile:833grep -oP is GNU-only; breaks VALE_VERSION extraction on macOS. Fix: use POSIX sed or awk, or adopt CRF-20's mise exec approach.
  2. CRF-19 (P3) ci.yaml:209all_changed_files includes deleted paths; Vale errors on missing files. Fix: use all_modified_files.
  3. CRF-20 (P3) Makefile:833 — ~30-line binary download block is the sole surviving instance of this pattern; actionlint and zizmor already use mise exec. Adopting mise exec vale -- eliminates CRF-17 as a side effect.
  4. CRF-21 (Nit) .markdownlint-cli2.jsonc:4 — add a JSONC comment documenting the coupling to .vale.ini Packages.
  5. CRF-22 (Nit) .vale.ini:56 — replace opaque k.R example with a real codebase example.

For each: push a fix, acknowledge, contest, or file a ticket.

🤖 This review was automatically generated with Coder Agents.

Resolves DOCS-40 review findings CRF-17/19/20/21/22:

- Makefile: invoke Vale via `mise exec "aqua:errata-ai/vale"` like
  actionlint/zizmor instead of the bespoke curl/tar download. This
  removes the GNU-only `grep -oP ...\K` version extraction that
  silently returned empty on macOS BSD grep, producing a malformed
  download URL (CRF-17). mise.toml stays the single version source of
  truth; mise handles the OS/arch build (CRF-20).
- ci.yaml: filter the changed-Markdown list to docs/ paths still
  present on disk so Vale never errors on files this PR deleted
  (CRF-19); run Vale through `mise exec`; drop the now-unused
  build/vale-* cache path; set MISE_YES so first-use auto-install is
  non-interactive. All Vale steps remain continue-on-error (advisory).
- .markdownlint-cli2.jsonc: document the coupling with .vale.ini's
  Packages directive (CRF-21).
- .vale.ini: replace the unreal `k.R` example with a real codersdk
  type name (CRF-22).

CRF-18 was already satisfied: both Vale cache steps pin
actions/cache@v5.0.5, matching the repo standard.

Validated locally: make lint/prose (mise-installed Vale 3.7.1, exit 0
via --no-exit), actionlint (exit 0), and zizmor (no findings) all pass.

Filed via Coder Agents on Nick's behalf.
@nickvigilante

Copy link
Copy Markdown
Contributor Author

Addressed post-rebase review (CRF-17 through CRF-22)

Pushed 30072f14a6 resolving the findings from review #4516496011. Per-thread replies are inline; summary:

  • CRF-17 (P2, macOS grep) + CRF-20 (P3, mise exec) — Replaced the bespoke curl/tar download (version extraction via GNU-only grep -oP ...\K, uname/arch mapping) with mise exec "aqua:errata-ai/vale" -- vale ..., matching the actionlint/zizmor targets. Removing the download path eliminates the macOS BSD-grep break entirely. The invocations use the full aqua key so the mise.toml pin (3.7.1) is honored — the bare vale short name resolves to the latest release (3.15.1) and ignores the pin, which is a trap worth noting.
  • CRF-19 (P3, deleted files) — The prose step now keeps only docs/ paths still present on disk before invoking Vale, so files a PR deletes no longer make Vale error.
  • CRF-21 / CRF-22 (Nits) — Documented the markdownlint ↔ .vale.ini Packages coupling; replaced the unreal k.R example with a real type, codersdk.WorkspaceAgent.
  • CRF-18 (P3) — Already satisfied on the reviewed commit: both cache steps pin actions/cache@27d5ce7 # v5.0.5. No change needed.

Validated locally: make lint/prose (mise-installed Vale 3.7.1, exit 0 via --no-exit), actionlint (exit 0), and zizmor --persona regular (no findings). Every Vale step remains continue-on-error (advisory).

/coder-agents-review

Filed via Coder Agents on Nick's behalf.

@coder-agents-review coder-agents-review Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

All 6 R4 findings addressed in 30072f1. The mise exec migration (CRF-17/CRF-20) is clean: 30 lines of platform-specific binary management replaced by 8 lines of mise exec, matching the established actionlint/zizmor pattern. Deleted-file filtering (CRF-19), coupling documentation (CRF-21), and the opaque example fix (CRF-22) are all verified correct.

Two minor items remain (CRF-23, CRF-24), neither blocking.

As Hisoka put it: "Six rounds in and this opponent has hardened. I pulled every thread I could find. Most held."

🤖 This review was automatically generated with Coder Agents.

Comment thread .vale.ini Outdated
Comment thread .github/workflows/ci.yaml Outdated
@nickvigilante nickvigilante marked this pull request as ready for review June 17, 2026 15:21
@nickvigilante nickvigilante requested a review from bpmct June 17, 2026 15:21
@coder-tasks

coder-tasks Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Documentation Check

Updates Needed

  • docs/.style/README.md - Lines 21-22 and 59 reference .vale.ini as being "added in a follow-up PR," but this PR lands .vale.ini. Update to present tense (e.g., "the repo-root .vale.ini points at..." and "Vale lints the entire docs/**/*.md set...").
  • docs/.style/style-guide.md - Line 79 says "The rationale for the cherry-picked base styles and the severity policy lives in the follow-up PR that lands .vale.ini." Since this IS that PR, update to reference the .vale.ini file directly (e.g., "The rationale ... is documented in .vale.ini comments and DOCS-40.").
  • docs/about/contributing/documentation.md - The Tools section lists alex.js, Grammarly, and Hemingway but does not mention Vale or make lint/prose, which is now integrated into CI. Consider adding Vale with a note that it runs automatically in CI and can be run locally via make lint/prose.

Automated review via Coder Agents

- .vale.ini: drop the stale "missing binary" failure mode from the
  exit-code comment; mise auto-installs Vale, so that mode no longer
  exists (CRF-23).
- ci.yaml: correct the **.md glob rationale -- the action's globber
  collapses a `**` adjacent to `.md` to a single segment, so docs/**.md
  misses nested pages; the prior "skips dot-prefixed dirs" reason was
  inaccurate (CRF-24).

Comment-only; no behavior change. Both are non-blocking nits from the
Coder Agents R6 review (which APPROVED the PR).

Filed via Coder Agents on Nick's behalf.
@nickvigilante

Copy link
Copy Markdown
Contributor Author

Addressed the two non-blocking R6 nits in 1659d509a7 (comment-only, no behavior change):

  • CRF-23 (.vale.ini) — Dropped the stale "missing binary" failure mode from the exit-code comment; with mise exec, mise auto-installs Vale so that mode no longer exists. Now consistent with the Makefile comment.
  • CRF-24 (ci.yaml) — Corrected the **.md glob rationale: the action's globber collapses a ** adjacent to .md to a single segment, so docs/**.md only matches top-level docs/*.md and misses nested pages like docs/.style/style-guide.md. The prior "skips dot-prefixed dirs" explanation was inaccurate.

Thanks for the thorough six-round pass. Not re-triggering a review for these comment-only fixes.

Filed via Coder Agents on Nick's behalf.

Comment thread .vale.ini
Comment on lines +24 to +26
# The styles themselves live under docs/.style/styles/ after `vale sync`,
# which the Makefile target invokes once per .vale.ini change. They are
# gitignored to keep the repo lean.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Given the only style guide we have is in docs/ is there any reason why we have the .vale.ini the root and not in ./docs? The only reason I ask is it can be visually pleasing to have a simple repo structure (and less scroll for users to the README). Plus - then all files and rules for the docs only exist in docs versus it being scattered in docs and the root

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This is a huge nit and see our root is already messy so feel free to ignore.

@bpmct bpmct left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Approving, but: Will this cause all of our existing PRs/builds to fail for engineers in CI since I'd assume we're not compliant?

The Doc-Check bot flagged three items on the rebased PR:

1. docs/.style/README.md referenced .vale.ini as 'added in a follow-up
   PR' on two lines, but this PR lands .vale.ini. Switched both to
   present tense and pointed at `make lint/prose` for local reproduction.
2. docs/.style/style-guide.md said the Vale rule-set rationale 'lives in
   the follow-up PR that lands .vale.ini'. Replaced with a pointer to
   the .vale.ini inline comments and `make lint/prose`.
3. docs/about/contributing/documentation.md's Tools section listed
   alex.js, Grammarly, and Hemingway but had no mention of Vale, even
   though Vale now runs on every doc PR. Added a paragraph explaining
   that Vale runs in CI on docs/, points at .vale.ini and the style
   guide, and runs locally via `make lint/prose`. Kept the existing
   external-tools bullet list with a slightly tightened lead-in that
   notes why those tools aren't a substitute (not tuned for Coder's
   voice).

The third edit is also a natural illustration of Vale's non-blocking
posture on this PR: the file already produces 9 warnings + 22
suggestions against the curated rule set, and the new paragraph adds
a few more. CI's prose step lints the changed Markdown files and surfaces
those as inline PR annotations, but the job still exits 0 because none
of them are error-level.
@datadog-coder

datadog-coder Bot commented Jun 22, 2026

Copy link
Copy Markdown

Pipelines

⚠️ Warnings

🚦 1 Pipeline job failed

weekly-docs | audit-docs-paths   View in Datadog   GitHub Actions

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 979137c | Docs | Give us feedback!

@nickvigilante

Copy link
Copy Markdown
Contributor Author

@bpmct

  1. This change will not block CI. It's just an information flag in the Files changed tab in a PR.
  2. About the location of .vale.ini, I agree the cleaner root folder would make sense, but all invocations of Vale would then require passing a custom --config flag, even when doing it by hand, so I'll log this as a separate issue to research further outside of the scope of this PR.

Thanks for the review!

@nickvigilante nickvigilante merged commit ee3572a into main Jun 22, 2026
36 of 40 checks passed
@nickvigilante nickvigilante deleted the vigilante/docs-40-add-vale-to-docs-ci-with-starter-microsoftgoogle-style-coder branch June 22, 2026 18:14
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants