Skip to content

feat(verify): warn on multi-entry raw and pin Vale's raw and front-matter behavior - #380

Merged
theCodeDrift merged 3 commits into
mainfrom
feat/vale-raw-authoring-checks
Sep 22, 2026
Merged

theCodeDrift merged 3 commits into
mainfrom
feat/vale-raw-authoring-checks

Conversation

@theCodeDrift

Copy link
Copy Markdown
Member

What

The code halves of #360 and #361. The recipe prose lands in a parallel PR; this one pins the underlying Vale behavior on the vendored binary and adds the verify advisory #360 asks for.

  • Vendor contract pins (packages/cli/test/vale-vendor-contract.test.ts, three new describe blocks under "Vale vendor contract"): how an existence rule's raw list is read, which key nonword governs, and which scopes see YAML front matter.
  • verify advisory (packages/cli/src/schemas/vale-rule.ts, wired in packages/cli/src/rules/inspect.ts): validateValeRule now returns advisories: string[] beside valid/errors, and verifyOneRule joins them with the config layer's advisories onto the rule's notice, in verify text output and --json.
  • Changeset .changeset/vale-raw-authoring.md, patch.

The exact advisory:

<id>: raw has N entries; Vale joins them into one pattern with no separator, so the second never matches on its own. Write one entry with (a|b) unless the join is intended.

Measured

Every row below was run on the vendored Vale 3.22.0 before it was written down, and each is now a test.

Pin Rule Fixture Observed
raw joins raw: ["\bstops being\b", "[^.]{0,20}\band becomes\b"] It and becomes fun. (second entry alone) no finding
same It stops being fun. (first entry alone) no finding
same It stops being dull and becomes fun. one finding, match stops being dull and becomes
raw: [alpha, bravo, charlie] bravo alone. alphabravocharlie together. one finding, match alphabravocharlie
alternation raw: ["(\bstops being\b|[^.]{0,20}\band becomes\b)"] each of the two lines above fires on both
nonword is for tokens tokens: ["—"] This is a sentence — with an em dash. no finding
tokens: ["—"], nonword: true same fires
nonword does not touch raw raw: ["—"], with and without nonword same fires both ways
raw: ["(The|That|This) is the twist\."], with and without nonword That is the twist. Yes. fires both ways
front matter scope: raw, token taskless ---\ntarget: taskless\n---\n\nSee taskless in the body. lines 2 and 5
scope: text, and no scope at all same lines 2 and 5
scope: paragraph, scope: sentence same line 5 only
scope: frontmatter.target same line 2 only
scope: frontmatter.title same no finding
scope: frontmatter same line 2 only
require a field occurrence, min: 1, scope: raw, token: '(?m)^description: .+$' front matter without description: one finding, line 1
same front matter with description: no finding
same no front matter at all one finding, line 1

Two things the issues did not say, worth carrying into the recipe PR:

  • nonword: true is a tokens concern. Vale wraps the pattern in \b…\b only when the rule has tokens and nonword is unset; raw is inserted verbatim. The "Right" example in create-vale-rule: raw entries concatenate, they do not alternate #360 carries nonword: true beside a raw list, where it is harmless and does nothing.
  • The default scope reads front matter too. scope: raw is not what lets a casing rule into target: taskless; a rule with no scope key, and scope: text, see it as well. paragraph and sentence do not. So "drop scope: raw" is not a fix on its own.

Why advisory, not rejection

A multi-entry raw has a legitimate reading: it is how Vale lets an author split one long pattern across lines, and the join is exactly what such an author wants. The trap is the other reading, where the entries were meant as alternatives, and the schema cannot tell the two apart. So it is said rather than refused, the same line vale-config.ts holds for a repeated key or a [*] matcher: ok stays true, errors stays empty, and the message names the fix for the reading that is usually meant.

The corpus in vale-corpus.ts has three verdicts (accepted, ignored, rejected) and no "accepted, with a note", so the advisory is not a corpus row; its unit tests sit in vale-schema-contract.test.ts beside the other tests that ask the schema rather than the binary.

How verified

  • pnpm build, pnpm typecheck, pnpm lint, pnpm --filter @taskless/cli test: 98 files, 1645 tests, all passing.
  • New tests: 12 vendor-contract pins (all measured first by hand on the binary, see the table), 7 schema unit tests for the advisory (two entries, three entries, one entry, tokens list, Raw: casing, beside a rejection, empty list), 3 CLI-level tests in verify-test-commands.test.ts (notice present on two entries in text and --json, absent on one entry, joined with a config advisory).
  • Untouched on purpose: packages/cli/src/agent/create-vale-rule.md (the recipe PR owns it).

Refs #360
Refs #361

@theCodeDrift

Copy link
Copy Markdown
Member Author

@claude /review

@claude

claude Bot commented Sep 22, 2026 •

Copy link
Copy Markdown

Claude finished @theCodeDrift's task in 11m 36s —— View job


Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json contained no reviewThreads, reviews, or comments, so this is the first review of this PR; nothing was skipped as already-addressed.

  • Read .prior-review.json (empty — first review)
  • Fetch PR diff and metadata (gh pr diff 380, gh pr view 380)
  • Read changed files in full, plus surrounding context (inspect.ts, vale-rule.ts, vale-config.ts, commands/verify.ts, dispatch.ts, vale/run.ts)
  • Correctness / cleanup / altitude / conventions analysis (8 finder angles)
  • Verify candidate findings
  • Post inline comments
  • Post final summary

Findings

[New] The PR's design is sound: raw really does concatenate with no separator on the vendored binary (matches the measured table in the PR body), the advisory is correctly scoped to existence rules with raw.length > 1 (confirmed against the generated VALE_CHECK_FIELDS vocabulary — raw belongs only to existence), and it correctly rides on notice rather than errors per the "said, not refused" precedent already established for the config layer. The 12 new vendor-contract pins, 7 schema tests, and 3 CLI tests are well targeted and I didn't find fabrication or escaping errors in the new YAML-building test fixtures.

Four items posted inline, ranked most notable first:

  1. packages/cli/src/rules/inspect.ts:290 — Once a style-layer advisory (e.g. the new multi-entry raw warning) and a config-layer advisory both fire for the same rule, advisories.join("\n") embeds a real newline into notice, but commands/verify.ts's text-mode renderer prints it as a single notice: ... line — the second advisory loses its prefix and indentation. This exact combined case was already latently possible before this PR (adviseValeRuleConfig alone can push >1 advisory), but this PR is what documents and tests "joined... in verify text output and --json," and its own CLI-level test for the combined case only asserts --json, leaving the text-mode gap untested. Same line also reimplements an existing joinNotices helper (rules/dispatch.ts:262) — now a third copy of the identical idiom (the second being rules/vale/run.ts:401-402).
  2. packages/cli/src/schemas/vale-rule.ts:831 — canonicalKeys is computed twice per validateValeRule call: once inside the schema pipeline (line 812, discarded by schemaLayer), once again in the new adviseValeRule. Harmless today, but a latent hazard if the pipeline ever grows a real pre-canonicalization step.
  3. packages/cli/src/schemas/vale-rule.ts:842-850 — The new advisory is a single ad hoc if, not integrated into the table-driven pattern the rest of the module uses, nor the single-walk approach adviseValeRuleConfig uses. Fine for one advisory; the next one will likely be copy-pasted rather than generalized.
  4. packages/cli/test/vale-vendor-contract.test.ts:167 — The new existenceOver test helper duplicates the pre-existing hedge helper's shape, and its extra string parameter silently requires a trailing \n from every caller.

No correctness bugs found in the core advisory logic, the canonicalKeys/literal-key handling, or the vendor-pin test assertions themselves. No CLAUDE.md/STYLEGUIDE-CODE.md violations found (import ordering, barrel exports, and type-export rules are all unaffected by the diff's hunks; the changeset's patch bump is correct for a 0.y.z package per CLAUDE.md's explicit guidance).

Comment thread packages/cli/src/rules/inspect.ts
Comment thread packages/cli/src/schemas/vale-rule.ts
Comment thread packages/cli/src/schemas/vale-rule.ts
Comment thread packages/cli/test/vale-vendor-contract.test.ts
@theCodeDrift

Copy link
Copy Markdown
Member Author

Re: @claude[bot] — "Claude finished @theCodeDrift's task in 11m 36s — Review complete…"
#380 (comment)

Read in full. The medium item (canonicalKeys computed twice) is answered on its thread: the advisory must read the raw input because it is specified to speak beside a rejection, where the pipeline yields no parsed value; canonicalKeys is pure and idempotent, so no divergence is possible today. The four low items (multi-line notice text rendering and joinNotices reuse, table-driven advisory shape, existenceOver helper ergonomics) are collected for the maintainer to decide on and left open.

— AI Coding Agent

@theCodeDrift
theCodeDrift force-pushed the feat/vale-raw-authoring-checks branch from e4951a1 to 84d3459 Compare September 22, 2026 04:18
@theCodeDrift
theCodeDrift merged commit 241e1c4 into main Sep 22, 2026
7 checks passed
@theCodeDrift
theCodeDrift deleted the feat/vale-raw-authoring-checks branch September 22, 2026 04:30
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