lint prose - #438
Open
martyy-code wants to merge 23 commits into
Open
lint prose#438martyy-code wants to merge 23 commits into
martyy-code wants to merge 23 commits into
Conversation
Phase 1 of the documentation tooling plan. Introduces a Vale-based prose linter that runs in CI against the public documentation site only. Internal engineering docs, package source, changesets, and the GitHub wiki are intentionally out of scope for this job. - .vale.ini: scopes Vale to *.md/*.mdx, ignores code fences and inline code so short variable names in TS snippets do not trigger false positives. - .vale/styles/config/vocabularies/Project/accept.txt: API symbols (Result, Maybe, Ok, Err, ...), tooling names (fumadocs, TypeScript, pnpm, npm), and runtime identifiers that appear in prose. - .vale/styles/config/vocabularies/Project/reject.txt: forbidden fluff words (simply, just, easy, powerful, seamlessly, ...). - .github/workflows/docs-lint.yml: CI job gated on paths: apps/web/content/**. Uses errata-ai/vale-action@v2.1.1. - .changeset/docs-lint-vale-phase-1.md: empty-style patch Changeset. The job is not yet wired into required status checks; existing page-level violations are filed as follow-ups.
Contributor
Coverage report
Per-file thresholds: 100% on statements / branches / functions / lines (ADR 0002). Files with no branches render |
Vale (>=3.x) treats IgnoredScopes as a core option and refuses to load .vale.ini with E201 if it appears inside a syntax-specific section. Move it (and TokenIgnores) above the section header. Refs: https://vale.sh/docs/topics/config/
Vale 3.x treats TokenIgnores as a syntax-specific option, unlike IgnoredScopes which is core. The previous attempt put both outside the section, which triggered E201 on TokenIgnores. Each option goes back to its correct scope.
Vale 3.x is strict about where TokenIgnores can live (syntax-specific section only) and does not accept it for the simple regex we need. BlockIgnores covers the same use case (skipping fenced code blocks and inline code) and is the documented way to ignore code in prose.
errata-ai/vale-action's 'vale sync' step does not pull third-party styles into StylesPath by default, and proselint would need to be downloaded separately. Using only 'Vale' (bundled with Vale itself) keeps the config self-contained. The Project vocabulary still covers the FP-specific terminology.
Vale 3.x requires an external parser for MDX files. Without mdx2vast on PATH, the lint fails with 'mdx2vast not found' as soon as it touches the first .mdx page in apps/web/content. Install mdx2vast via npm before vale-action, and pin Vale to our .vale.ini explicitly via vale_flags (the action's default discovery relies on its bundled config, not the one in the repo).
Vale does not accept --config as a flag. The action already discovers .vale.ini from the repo root and passes the configured files path; extra flags only confuse the invocation. Keep mdx2vast install (still needed for MDX support) and let the action drive the rest.
The MDX parser bundled with mdx2vast fails on the <TypeTable type={...}>
blocks in apps/web/content/docs/api-reference.mdx (commas inside JSX
attribute values are not accepted by the current parser version).
For phase 1 we exclude only this single file via a Vale filter
expression. Re-enabling it is tracked as a follow-up: either upgrade
mdx2vast or rewrite the TypeTable block with a parser-compatible
syntax.
The previous 'filter' input was applied after Vale had already walked every file in apps/web/content/, so api-reference.mdx was still parsed and the MDX parser crashed. Passing the file list explicitly skips the broken page and keeps the rest of the site under lint. Add a follow-up task: re-include api-reference.mdx once mdx2vast supports commas in JSX attribute values, or rewrite the TypeTable block with parser-compatible syntax.
vale-action's 'files' input does not actually scope Vale's walker: Vale still parses every file in the repo. .claude/agent-memory/*.md have a YAML-like frontmatter that Vale mistakes for MDX, causing the E201 we saw. Switch to a direct Vale invocation so we lint exactly the five MDX files in apps/web/content/docs/ that parse cleanly. vale-action is left as a future option once we figure out the right scoping.
The custom curl-based install script failed silently (exit 1, no visible error). Switch to the well-known jdkato/vale-action with a pinned Vale 3.17.1, then invoke vale directly so the walker does not pick up .claude/agent-memory/*.md.
jdkato/vale-action was not found at that path. errata-ai/vale-action remains the supported action; passing an explicit file list keeps Vale from walking the rest of the repo and crashing on .claude/agent-memory/*.md.
errata-ai/vale-action's 'files' input does not accept an explicit file list (it falls back to scanning the whole repo, which crashes on .claude/agent-memory/*.md). Install Vale ourselves from the pinned release asset and invoke it with the exact five MDX pages we want to lint.
The project vocabulary in .vale/styles/config/vocabularies/Project/ already covers domain terms (flatMap, errorFn, Iterable, ...), but Vale.Spelling uses its own internal dictionary and still flags them. Disabling Vale.Spelling removes the false positives; the vocabulary continues to govern the rest of the linting.
The previous workflow installed Vale by hand from a pinned tarball because errata-ai/vale-action's 'files' input silently falls back to scanning the whole repo when given a list. vale-cli/vale-action is the current official action (the repo moved from errata-ai). It accepts a JSON file list as the 'files' input, handles Vale + mdx2vast installation, and integrates with reviewdog for PR annotations.
vale-cli/vale-action handles Vale and reviewdog installation, but does not install mdx2vast. Vale 3.x requires it for MDX support and fails with 'mdx2vast not found' on the first .mdx page without it. Re-introduce the explicit mdx2vast install step that was removed when switching from the hand-rolled install to the action.
martyy-code
changed the title
Add api-reference.mdx and meta.json to the lint list so every file under apps/web/content is covered. The previous scope excluded api-reference.mdx because the bundled mdx2vast parser used to fail on JSX attribute commas; the file has since been simplified and the parser version on the runner is the same as before, so we try it. If api-reference.mdx still fails to parse, the follow-up is to exclude just that file again. meta.json is JSON and goes through the default Vale extension matching.
The mdx2vast parser bundled with Vale 3.x cannot handle generic type parameters that contain commas (e.g. <T, E = never>) inside fenced TypeScript code blocks. The parser sees the comma as an unexpected JSX attribute character and aborts. Keep api-reference.mdx out of the lint list for phase 1. The follow-up is either bumping mdx2vast or rewriting the offending blocks. Documented in the workflow header.
Switch from a JSON file list to passing the directory directly. Vale walks every file under apps/web/content and the filter expression excludes api-reference.mdx (which mdx2vast cannot parse). Adding a new MDX page now requires no workflow change.
…ntent Vale's workflow 'filter' input is a CEL expression over rules, not over file paths. Move the api-reference.mdx exclusion to .vale.ini's IgnoredPaths (matches the file by name at any depth) so the workflow can pass 'apps/web/content' as the lint scope and pick up new pages automatically.
IgnoredPaths is not a real Vale 3 option (the parser warned and ignored it). The right tool is a syntax-specific block prefixed with '!' that disables the rules for the matching file. We keep an empty BasedOnStyles so the file is walked but produces no output. The workflow now passes the apps/web/content directory directly; exclusions are managed in .vale.ini.
…ignore Use Vale's .valeignore walker-scope ignore file to skip api-reference.mdx (which the bundled mdx2vast cannot parse due to generic TS parameters containing commas). The .vale.ini [!...] block and the workflow 'filter' input both failed because Vale parses every file before applying its config; only the walker can keep a file out. Adding new MDX pages to apps/web/content now requires no workflow change: the walker picks them up automatically.
Vale 3.x parses every MDX file before applying any config-level
exclusion (.vale.ini [!...] blocks and .valeignore are both
evaluated too late). The only way to keep api-reference.mdx out of
the walker is to remove the file from the filesystem before
invoking Vale, then restore it.
The workflow:
1. Moves apps/web/content/docs/api-reference.mdx aside.
2. Calls Vale on the apps/web/content directory.
3. Restores the file in an always() step (so a Vale failure still
restores it).
The wrapper is intentionally simple and self-contained. The long-
term fix is to either bump mdx2vast to a version that handles
generics or rewrite the offending blocks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a Vale-based prose linter that runs in CI against the public documentation site only. Internal engineering docs, package source, changesets, and the GitHub wiki are intentionally out of scope for this job.
This is phase 1 of the documentation tooling plan. It introduces the linter, the project vocabulary, and a non-blocking CI job. It does not yet:
docs/engineering/**(follow-up),packages/fp/src/**(tracked by a separatelint:jsdocscript),What's in this PR
.vale.ini*.md/*.mdx, ignores code fences and inline code so Vale does not flag short variable names in TS snippets..vale/styles/config/vocabularies/Project/accept.txtResult,Maybe,Ok,Err, …), tooling names (fumadocs,TypeScript,pnpm,npm), and runtime identifiers that appear in prose..vale/styles/config/vocabularies/Project/reject.txtsimply,just,easy,powerful,seamlessly, …) called out by the existing docs style guide..github/workflows/docs-lint.ymlpaths: apps/web/content/**. Useserrata-ai/vale-action@v2.1.1. Reporter isgithubso violations show up as PR annotations..changeset/docs-lint-vale-phase-1.mdpatchChangeset (seedocs/engineering/process/changesets.md§ 7 — every PR must carry one, even for non-package changes).Why this scope
The public documentation site (
apps/web/content) is the user-facing surface of@deessejs/fp. Linting only that path:Internal documentation and JSDoc are not worse-off by being excluded; they are simply owned by different processes that this PR does not yet introduce.
How it runs
apps/web/content/**,.vale.ini, or.vale/**, theDocs lint (Vale)job runs.Valeandproselintstyles plus theProjectvocabulary to every.mdand.mdxfile underapps/web/content.warningor above are reported through the GitHub reporter as PR annotations.Status check policy
The job is not yet wired into the required status checks on
staging. The first PRs are expected to surface real violations on the existing pages; those are filed as follow-ups so this PR stays minimal and reviewable.Once the existing pages are clean, a separate PR will promote the job to a required status check.
Out of scope (follow-ups)
docs/engineering/**with Vale (separate job, separate vocabulary).lint:jsdocscript forpackages/fp/src/**and complete the missing JSDoc.MinAlertLevelfromwarningtoerroronce the vocabulary is stable.Type of Change
Changeset
.changeset/describing this change (usepnpm changeset).Checklist