Skip to content

fix(vale): adopt Vale 3.26.0 - #607

Merged
theCodeDrift merged 3 commits into
mainfrom
vendor/vale/3.26.0-fixes
Nov 3, 2026
Merged

theCodeDrift merged 3 commits into
mainfrom
vendor/vale/3.26.0-fixes

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

Lands Vale 3.26.0 laterally: this branch carries #589's bump commit plus the fixes it needs, targeting main.

What 3.26.0 broke

One unreadable file took every Vale finding down again (#300 by a new route). 3.26.0 (upstream b5b8e429) no longer stops at the first document it cannot read. It lints the rest, exits 2, and writes one JSON error object per unreadable document to stderr. run.ts parsed stderr as one JSON document, so with two or more broken files the parse failed, the narrowing lost every Path, and the run failed with every finding gone. Measured on both binaries with two files whose front matter is invalid YAML. vale-run.test.ts caught it; the contract suites did not, so #589's report read green.

  • run.ts reads stderr as a stream of error objects and excludes every unreadable target file in one retry. An error that does not name a target file still fails the run, as before.
  • vale-vendor-contract.test.ts pins the new stderr shape, so a future change to it shows up in the upgrade report.

What 3.26.0 learned

internal/core/format.go now routes .mjs/.cjs to the JavaScript reader and .mts/.cts to TypeScript, so all four moved from plaintext to the comment tier. They are new rows in VALE_FORMAT_TIERS, probed by the contract suite.

The four readers #589's report listed (built.go, code_js.go, code_ts.go, native_stubs_js.go) are not formats: they split existing handlers by build tag for the new WebAssembly build. The report missed the real change because it was a two-line edit to format.go, so vale-upgrade-report.cjs now prints that file's patch whenever it changes.

Ledger and release note

update.md gains a "Migrating to 0.13.5" entry with three changes measured on both binaries: the four comment-only extensions, a phrase matching across a line wrap, and the built-in dictionary knowing words like OAuth and npm. The changeset says the same.

Checked and needing nothing

  • A missing converter still exits 2; check already sets converter files aside before calling Vale.
  • .config/vale.ini discovery does not apply, since check passes --config.

Closes #589

@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@github-actions

github-actions Bot commented Nov 3, 2026 •

Copy link
Copy Markdown
Contributor

Claude finished @theCodeDrift's task in 3m 1s —— View job


Review: fix(vale): adopt Vale 3.26.0

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contains no reviews, threads, or comments besides the @claude /review trigger itself, so this is the first review of this PR.

  • Read .prior-review.json
  • Fetch PR diff and metadata
  • Review run.ts stderr-parsing fix
  • Review vale-upgrade-report.cjs diff-printing change
  • Review capabilities.ts format tier additions
  • Review test files (vale-vendor-contract.test.ts, vale-upgrade-report.test.cjs, reconcile-marker.test.ts)
  • Review ledger/changeset docs
  • Post findings

packages/cli/src/rules/vale/run.ts

Traced splitJsonObjects carefully (brace-depth tracking outside strings, escape handling, the "anything non-whitespace at depth 0 that isn't { aborts the whole parse" rule) and the retry loop in runVale. Both look correct:

  • splitJsonObjects correctly treats braces/quotes inside a string as inert, requires every object to close cleanly, and returns undefined (not a partial list) for anything that isn't a clean sequence of top-level objects separated only by whitespace — matching the "all or nothing" contract the docstring promises.
  • The two-pass retry logic (first pass requires every error to resolve to a not-yet-excluded target file or the whole attempt is reported as blocking-failed; second pass dedupes a file reported twice in the same attempt via excludedTargets.has before pushing a finding) handles the duplicate-path case correctly and preserves the "bounded by construction" termination argument from the comment.
  • Confirmed packages/cli/test/vale-run.test.ts (unchanged by this PR, not in the diff) already has a "drops more than one bad file, one finding per file" test that exercises exactly the two-bad-file scenario this fix addresses against the real vendored binary — so the fix is exercised even though that test file isn't part of the diff.

No correctness issues found in this file.

.github/scripts/vale-upgrade-report.cjs

ROUTING_FILE/formatRouting addition is straightforward and the new tests (routing: a changed format.go prints its patch, routing: an unchanged format.go is said) match the implementation, including the readers.routing default (undefined) from the existing readers() test helper. Only a very minor observation, not a blocker: the COMPARE_FILE_CAP completeness caveat text only mentions the added-readers list ("may be incomplete... check the full diff"), even though a truncated compare response could in principle also cause format.go's patch to be missed. Given format.go is a tiny, almost-always-early file in a Vale diff, this is unlikely to matter in practice.

packages/cli/src/rules/capabilities.ts

New .cjs/.cts/.mjs/.mts → comment rows and the accompanying "3.24.0 → 3.26.0" narrative are internally consistent with the rest of the table's history and with the PR description (readers added under internal/lint/ are WebAssembly build splits, not new formats; the real routing change is a two-line format.go edit). Nothing to flag.

Tests

  • vale-vendor-contract.test.ts: new test pins the real stderr shape (one JSON object per unreadable document, exit 2) with two bad files — directly guards the regression this PR fixes.
  • vale-upgrade-report.test.cjs, reconcile-marker.test.ts: straightforward coverage additions matching the new behavior, no issues.

Summary

Found nothing NEW to flag — no bugs in the stderr-parsing/retry rewrite, the format-tier additions, or the upgrade-report script changes. This is a clean, well-documented fix.

github-actions Bot and others added 3 commits November 3, 2026 13:20
Vale 3.26.0 lints past a document it cannot read and writes one error
object per such document to stderr. run.ts parsed stderr as a single
JSON document, so two unreadable files lost every Path and the run
failed with every finding gone (#300 by a new route). Read
stderr as a stream of error objects and exclude every unreadable target
in one retry; pin the new shape in the vendor contract.

format.go now routes .mjs/.cjs to the JavaScript reader and .mts/.cts
to TypeScript, so all four are comment-tier rows. The upgrade report
listed four added readers that are WebAssembly build splits and missed
this edit, so it now prints the format.go patch whenever it changes.

Adds the 0.13.5 update ledger entry and grows the changeset.
@theCodeDrift
theCodeDrift force-pushed the vendor/vale/3.26.0-fixes branch from 3e33a3b to 418178f Compare November 3, 2026 21:21
@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @github-actions — "Claude finished @theCodeDrift's task in 3m 1s — Review: fix(vale): adopt Vale 3.26.0…"
#607 (comment)

Declined the compare-cap observation on vale-upgrade-report.cjs: a Vale bump has not come near the 300-file listing cap, so the "Unchanged" routing line stays as is. No other findings to act on.

— AI Coding Agent

@theCodeDrift
theCodeDrift merged commit 8951788 into main Nov 3, 2026
4 checks passed
@theCodeDrift
theCodeDrift deleted the vendor/vale/3.26.0-fixes branch November 3, 2026 21:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant