feat: derive the Vale rule schema from the vendored binary - #177
feat: derive the Vale rule schema from the vendored binary#177thecodedrift wants to merge 3 commits into
Conversation
`pnpm generate:vale-schema` runs the pinned Vale against rules it writes itself and emits `src/generated/vale-vocabulary.ts`, alongside a divergence report. It replaces a transcription whose measurements were real and whose measuring script was discarded — leaving the answer, not the question. Every verdict comes from the process exit status and the structured JSON output, never from matching stdout against an error phrase. A Go panic contains no `has invalid keys` string, so a phrase-grep scores a crash as a clean run, which is how a tokenless `sequence` rule once looked like a check that validates nothing. A run's outcome is a closed set — clean, diagnostic, panic, unrecognized — and the last is fatal at every call site. Two of the four vocabularies self-enumerate: an unknown `extends` or `level` makes the binary name its own accepted set. If either line stops matching, generation fails rather than emitting a short enum, because a truncated enum is stricter than the binary and strictness is the direction that blocks working rules. The other two are honest about their limit. `E201` names the key you got wrong and never the ones you could have used, and an unknown scope raises nothing at all, so field tables and scope operands are verified rather than discovered from a candidate list whose provenance is recorded. A scope verdict is three-valued, and a `scope: raw` reach probe must fire on every fixture so an operand cannot be dropped because its fixture was unreachable. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cEN93Acyp4zBwP3oDnyy1
The enums and field tables move out of `vale-rule.ts` and into the generated artifact; what stays is the part worth reading — the zod construction, the two-stage `.pipe()` that reproduces the binary's own order, the case-folding transform, the scope grammar, and the error messages that explain blast radius to an author. Generating those would mean maintaining a code emitter to reproduce prose. Three things stay hand-written on purpose, and each says why in place: - The fatal shapes. Every key in a tokenless `sequence` is a legal field of its check; it is the shape that panics, which a field table cannot express. - The scope grammar. `~` and `&` are operators around the operands, and rejecting `~fenced` is a business rule rather than a transcription. - The union members, spelled out so each `z.literal` survives into `ValeRule`. An import-time guard requires every derived check type to appear exactly once and to be classified strict or permissive. The artifact's version is asserted against `VALE_VERSION` as a conditional type rather than an `if`. Both are string literal types, so the compiler settles it and a mismatch is a build failure naming the line, instead of a throw in front of a user on whichever command loads the schema first. All 86 existing corpus rows pass unmodified — that is what proves generation reproduced the transcription. Two rows are added for ground the generation newly measured: `comment.line` and `comment.block` in the TypeScript tier, which is the only operand family whose membership depends on which parser the extension routes to. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cEN93Acyp4zBwP3oDnyy1
The `cli-rule-validation` requirement "The Vale rule schema is pinned to the vendored binary" was written for a hand-authored transcription. It becomes a derivation, and gains the three constraints that make one trustworthy: take every verdict from the exit status and the structured output rather than from an error phrase; fail rather than emit a partial enumeration, because a short enum is stricter than the binary; and report a divergence from Vale's documentation rather than resolving it silently. Delivery shape is a single PR — the generator, its artifact, and the schema's switch to importing it are only correct together, and there is no intermediate state that reaches production safely. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017cEN93Acyp4zBwP3oDnyy1
|
@claude /review |
|
Claude finished @thecodedrift's task in 12m 29s —— View job Review completeReview mode: incremental — read 0 prior review thread(s) before reviewing. Todo list
Overall assessmentThis is a well-executed, thoroughly documented change: the generator ( I did find one real correctness/altitude gap and a few lower-severity cleanup items, posted inline:
Minor, not inlined: the changeset, Nothing else rose to the level of a finding — the type-precision question around |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| const UNION_MEMBERS = [ | ||
| "existence", | ||
| "substitution", | ||
| "capitalization", | ||
| "occurrence", | ||
| "repetition", | ||
| "conditional", | ||
| "metric", | ||
| "readability", | ||
| "sequence", | ||
| "script", | ||
| "consistency", | ||
| "spelling", | ||
| ] as const; | ||
|
|
||
| { | ||
| const spelled = new Set<string>(UNION_MEMBERS); | ||
| const derived = new Set<string>(VALE_CHECK_TYPES); | ||
| const missing = [...derived].filter((name) => !spelled.has(name)); | ||
| const extra = [...spelled].filter((name) => !derived.has(name)); | ||
| const permissive = new Set<string>(VALE_PERMISSIVE_CHECKS); | ||
| const unclassified = [...derived].filter( | ||
| (name) => !permissive.has(name) && !(name in CHECK_FIELDS) | ||
| ); | ||
|
|
||
| if (missing.length > 0) { | ||
| throw new Error( | ||
| `Vale ${PINNED_VALE_VERSION} has a ${missing.join(", ")} check and the schema's ` + | ||
| `union does not, so every rule extending it would be rejected. Add ` + | ||
| `the member in src/schemas/vale-rule.ts.` | ||
| ); | ||
| } | ||
| if (extra.length > 0) { | ||
| throw new Error( | ||
| `the schema's union has a ${extra.join(", ")} member and Vale ` + | ||
| `${PINNED_VALE_VERSION} has no such check, so those rules would verify clean ` + | ||
| `and then fail the whole run. Remove the member in ` + | ||
| `src/schemas/vale-rule.ts.` | ||
| ); | ||
| } | ||
| if (unclassified.length > 0) { | ||
| throw new Error( | ||
| `${unclassified.join(", ")} is in neither the derived field tables nor ` + | ||
| `the permissive set, so there is nothing to build a union member from. ` + | ||
| `Re-run the generator: pnpm generate:vale-schema.` | ||
| ); | ||
| } | ||
| } |
There was a problem hiding this comment.
[New] The import-time guard checks the wrong list — it never validates the actual discriminatedUnion array.
UNION_MEMBERS (line 495) is referenced exactly twice in this file: its own declaration, and inside this guard block. It is never used to build valeBodySchema — the union at lines 544-560 is a separate, independently hand-written list of check(...)/permissiveCheck(...) calls. So this guard proves UNION_MEMBERS agrees with the derived vocabulary; it does not prove the union array does.
Concrete failure: Vale adds a 13th check type. pnpm generate:vale-schema updates VALE_CHECK_TYPES, and on next import missing is non-empty, throwing and naming the check — correctly prompting a fix. A maintainer adds the name to UNION_MEMBERS per the error message and stops there (an easy thing to miss, since it's a separate list from the actual union a few lines below). The guard now passes — spelled matches derived — but valeBodySchema's discriminatedUnion array still has only 12 members. A rule with the new extends value passes stage-1 header validation (it's in VALE_CHECK_TYPES) and then fails stage-2 with the generic "extends must be one of: ..." fallback, even though Vale itself runs the rule fine. That's exactly the "too-strict" failure this module says is the worse direction, and the doc comment above ("This block closes that gap at import... every derived check type must appear below exactly once") overstates what's actually checked.
In practice vale-schema-contract.test.ts's "is not too strict" test would catch this (given a corpus row for the new check), so it wouldn't reach production silently — but that's the test suite doing the job this comment credits to the import-time guard. Consider building the union array from UNION_MEMBERS/CHECK_FIELDS/VALE_PERMISSIVE_CHECKS instead of introducing a third freestanding name list, or asserting the constructed union's actual discriminant set against VALE_CHECK_TYPES after valeBodySchema is built.
| /** | ||
| * Read Vale's diagnostic object off stderr. | ||
| * | ||
| * Vale writes one JSON object per config error, pretty-printed and possibly | ||
| * several in a row. `JSON.parse` handles the single-object case; anything else | ||
| * returns `undefined` and becomes {@link ProbeOutcome} `unrecognized`, which | ||
| * every caller treats as fatal. That is the point: an unreadable diagnostic | ||
| * must never be mistaken for a clean run. | ||
| */ | ||
| function readDiagnostic(stderr: string): ValeDiagnostic | undefined { | ||
| const trimmed = stderr.trim(); | ||
| if (trimmed === "") return undefined; | ||
| let parsed: unknown; | ||
| try { | ||
| parsed = JSON.parse(trimmed); | ||
| } catch { | ||
| return undefined; | ||
| } | ||
| const first: unknown = Array.isArray(parsed) ? parsed[0] : parsed; | ||
| if (typeof first !== "object" || first === null) return undefined; | ||
| const candidate = first as Partial<ValeDiagnostic>; | ||
| if ( | ||
| typeof candidate.Text !== "string" || | ||
| typeof candidate.Code !== "string" | ||
| ) { | ||
| return undefined; | ||
| } | ||
| return candidate as ValeDiagnostic; | ||
| } |
There was a problem hiding this comment.
[New] ValeDiagnostic/readDiagnostic duplicate ValeConfigError/asValeConfigError in src/rules/vale/map.ts.
map.ts:44-68 already defines the same {Code, Text, ...} shape check (asValeConfigError), which this script could import — it already does the equivalent for findValeBinary/VALE_VERSION a few lines up (await import("../src/rules/vale/binary.js") / capabilities.js), so importing map.js the same way is an established, working pattern here. As written, a future change to Vale's diagnostic envelope (e.g. Span becoming optional, or a new discriminating field) has to be applied in both places, and nothing enforces that it is.
| * Run one rule over one document in a config isolated from everything else. | ||
| * | ||
| * `BasedOnStyles =` is load-bearing. Without it Vale loads its bundled styles | ||
| * and a control document can trip one of those, which a finding count cannot | ||
| * tell apart from the rule under test firing. | ||
| * | ||
| * `--no-exit` suppresses the non-zero status Vale returns merely for *finding* | ||
| * something, so a non-zero status here means the config itself failed. | ||
| */ | ||
| function probe( | ||
| rule: string, | ||
| document: string, | ||
| extension: string | ||
| ): ProbeOutcome { | ||
| const cwd = mkdtempSync(join(tmpdir(), "vale-derive-")); | ||
| try { | ||
| mkdirSync(join(cwd, "styles", "probe"), { recursive: true }); | ||
| writeFileSync(join(cwd, "styles", "probe", "probe.yml"), rule); | ||
| writeFileSync( | ||
| join(cwd, ".vale.ini"), | ||
| "StylesPath = styles\nMinAlertLevel = suggestion\n\n[*]\nBasedOnStyles =\nprobe.probe = YES\n" | ||
| ); |
There was a problem hiding this comment.
[New] probe()'s isolating config re-derives the recipe buildIsolatingConfig (src/rules/vale/verify.ts:40-50) already encodes.
Both write StylesPath + MinAlertLevel = suggestion + an empty BasedOnStyles = + a single <style>.<rule> = YES line, and buildIsolatingConfig's docstring specifically calls out why each of those three details is load-bearing. Here the same shape is re-derived by hand with a hardcoded probe.probe name. The two aren't a drop-in match (buildIsolatingConfig requires an absolute StylesPath tied to the .taskless/vale/rules/<id> layout, vs. this script's simpler styles/probe/probe.yml), so reuse would take some adapting, but a future load-bearing requirement discovered against the isolating recipe (e.g. another key Vale needs) would need to be applied in both places today.
| for (const check of CHECK_TYPES) { | ||
| // The sentinel is the one probe where a panic is *not* read as evidence. It | ||
| // decides whether the check validates keys at all, and a permissive verdict | ||
| // taken from a crash would make the schema loose for a check that is strict. | ||
| if ( | ||
| probeField(check, SENTINEL_FIELD, { panicIsEvidence: false }) === "member" | ||
| ) { | ||
| permissiveChecks.push(check); | ||
| continue; | ||
| } | ||
| const accepted = new Set([ | ||
| ...HEADER_FIELDS, | ||
| ...Object.keys(must(CHECK_BASES[check], `the base rule for ${check}`)), | ||
| ]); | ||
| for (const field of FIELD_CANDIDATES) { | ||
| if (accepted.has(field)) continue; | ||
| if (probeField(check, field, { panicIsEvidence: true }) === "member") { | ||
| accepted.add(field); | ||
| } | ||
| } | ||
| acceptedFields[check] = [...accepted].toSorted(); | ||
| console.log(` ${check}: ${String(accepted.size)} fields`); | ||
| } |
There was a problem hiding this comment.
[New] Efficiency: this probes one candidate field at a time (~60 candidates × 10 strict checks ≈ 600 subprocess spawns, each with its own mkdtempSync/writeFileSync/spawnSync/rmSync), but invalidKeys (line 488) already parses Vale's has invalid keys: 'a', 'b', … as a list.
A rule carrying every untested candidate at once would return every rejected key in a single diagnostic, so a "throw everything at it" pre-pass could resolve most candidates in one probe per check, falling back to one-at-a-time only to disambiguate the type-objection/panic cases. This is a dev-time generator script, not a hot path, so it's a wall-clock nicety rather than a correctness issue — flagging since pnpm generate:vale-schema wall time scales with subprocess count and this cuts it by roughly an order of magnitude.
Stack (root → tip):
What this is
Unit 3 of the
vale-authoring-polishstack, on top of #175. It replaces thehand-written Vale vocabulary with one derived from the vendored binary by a
checked-in generator, keeps zod as the validation layer, and ships the
divergences the derivation found as a report rather than dropping them.
Review #175 first — this rewrites the schema it introduced.
Why
#175 called its own schema what it was: a transcription. Twelve check types,
three levels, ten field tables and twenty-eight scope operands, read off a
binary by hand because Vale publishes no JSON Schema and its machine-readable
field knowledge sits behind a paid hosted MCP.
Every one of those values was measured. The measuring was done by a script
that was then thrown away. What survives is the answer, not the question —
the next person to raise
VALE_VERSIONgets a test failure naming a field andno way to reproduce the measurement it is failing against.
The measuring is also where the errors live, and one is already on the record:
A Go panic contains no
has invalid keysstring, so a phrase-grep scores acrash as a clean run. A method that can make that mistake is worth writing
down once, correctly.
The method rule
Every verdict comes from the process exit status and the structured JSON
output. Never from grepping stdout for an error phrase. A run's outcome is a
closed set with no escape hatch:
cleandiagnosticCodepanicunrecognizedunrecognizedis fatal at every call site. There is deliberately no branchthat folds an unfamiliar shape into "fine".
What is derived, and what is honestly not
'extends' key must be one of [...]'level' must be one of [...]has invalid keys: '<name>'— names the bad keyThe top two self-enumerate: hand the binary a sentinel and it names its own
accepted set. If either line stops matching, the generator errors rather than
emitting a short enum — a truncated enum is stricter than the binary, which
is the direction that blocks rules that would have worked.
The bottom two are verified, not discovered, and the artifact says so. The
candidate list is seeded from four sources, including names the binary rejects
(
prefixes,suffixes,ignorecaseoncapitalization) so that theirrejection stays a recorded finding rather than an omission, and every check's
fields are offered to every other check so the tables are a measured partition.
Membership is read as evidence about the key, since the probe value is
arbitrary. Two of the positive verdicts are inferences rather than readings —
"Vale objected to the value instead" and "Vale panicked, so it dereferenced
something" — and both are named in the report, because an inference nobody
can audit is a grep with better manners.
scopehas no oracle at all, so its verdict is three-valued and ascope: rawreach probe must fire on every fixture or the run is fatal. That guard is
not theoretical:
figure.captionmeasures silent when its fixture nests thecaption inside a
<figure>, and so doesscope: textover the same document,because Vale drops everything in that element. Without the guard the operand
would be dropped as one Vale ignores, and every rule using it would then fail
verify.The divergence report
src/generated/vale-vocabulary-report.md, emitted by the run:meta,meta.class.<kind>verifyrejects themfrontmatter,frontmatter.<key>consistency,spellingactionon all ten strict checksexpected a map, got 'bool'frontmatteris the standing proof of the limit above: nobody proposing fromthe documentation alone would have found it.
Three claims carried in from earlier measurement did not reproduce, and the
report is where that surfaced:
figure.captiondoes fire, on a bare<figcaption>. What never fires isthe nested-in-
<figure>form; the earlier note conflated the two.comment.blockfires, in.jsand.tsalike.comment.*does not need a.tsfixture specifically —.jsreaches thesame tier. Rows for both now exist, because "fires in a source file" and
"fires in this source file" are different claims.
What stays hand-written
sequenceis a legalfield of its check; it is the shape that panics, which a field table cannot
express.
~and&are operators around the operands, andrejecting
~fencedis a business rule rather than a transcription.z.literalsurvives intoValeRule. An import-time guard requires every derived check type to appearexactly once and to be classified strict or permissive.
if.Both sides are string literal types, so a mismatch is a build failure naming
the line instead of a throw in front of a user.
How it is checked
The corpus is the acceptance test. All 86 existing rows pass unmodified
against the generated schema — that is what proves generation reproduced the
transcription. No row was edited to fit the output. Two rows were added for
ground the generation newly measured (
comment.line/comment.blockin theTypeScript tier), for 88.
The generated tables matched the hand-written ones exactly: the same twelve
check types, the same three levels, the same nine common fields, all ten
per-check tables identical, the same twenty-eight scope operands and two
prefixes. Nothing in the vocabulary changed; only how it is produced.
Deleting both artifacts and re-running produces byte-identical files.
What it looks like
The second only appears once the first is fixed, which is the
.pipe()reproducing the binary's own order: with no valid header there is no field
table to check anything against.
Refs #171