refactor(cli): recipes are markdown, so Vale can tell prose from an example - #290
Conversation
… itself `pnpm cli check` reported 5 warnings. All five are cases where a rule fires on a word being QUOTED rather than used, and the fix differs by whether the quoted word is load-bearing. Four `no-hedging` hits, in `onboard.txt` and `route.txt`. The rule's own config already draws the distinction and excludes `create-vale-rule.txt` and `verify-rule.txt`, because those teach authoring through a worked example called `no-simply`: the token is the subject, appears ~34 times, and "rewording the example to dodge its own subject would make the recipe worse". That reasoning is right and those files stay excluded. It does not apply to these four. The same config predicted them, calling them "sample rule requests", and in each the hedging word is an arbitrary choice of illustration rather than the subject: onboard.txt one bullet among four sample rule requests route.txt:78 an example distinguishing prose evidence from tree evidence route.txt:197 a routing-table row route.txt:198 a routing-table row Each now names a different sample rule and teaches exactly what it taught before. `onboard.txt` names `comments-record-not-forecast`, a rule this repository actually ships, so the example stays as grounded as it was. Coverage is unchanged. The alternative was excluding two more recipes from the rule, which would have traded real coverage for a quiet run. Both recipes changed, so both bump their topic version, per the freshness signal #282 landed for. onboard v2 to v3, route v4 to v5. The fifth was `no-unrouted-cli-invocation` on a zod `.describe()` in `schemas/verify-test.ts` naming `@taskless/cli/reference.json`. That rule catches content the CLI EMITS to its own reader, where an unrewritten specifier sends a nightly's user to install the release instead. A schema description names a published artifact to a consumer reading the schema: it is a fact about the artifact, true on every build target, and routing it would rewrite an export path that does not differ between them. `packages/cli/src/ schemas/**` is now ignored by that rule, with the reasoning recorded beside the entry. pnpm cli check No issues found. 1318 tests pass, typecheck and lint clean, and the rule's own fixtures still verify and test green after the ignore.
…xample `pnpm cli check` reported 5 warnings, every one on a word being QUOTED rather than used. The first commit reworded four incidental examples. This replaces that workaround with the fix. Recipes were `.txt`. They have never been plain text: `route` alone carries 69 headings, tables, fenced blocks and emphases. The extension was the whole problem. Vale has no markdown parser for a `.txt`, which the rule config already recorded: These are `.txt`, which Vale treats as PLAIN TEXT: there is no markdown parser, so fenced blocks and code spans are not skipped the way they are in a `.md` file. Everything in the file is prose as far as the rule is concerned, including command examples. So `no-hedging` excluded `create-vale-rule` and `verify-rule` wholesale, giving up all coverage on two of the largest recipes to silence one worked example. Renaming to `.md` collapses that. Measured: lifting the exclusion cost 33 findings as `.txt` and 2 as `.md`, because the other 31 were inside fences and code spans that a markdown parser skips. The 2 that survive are genuine prose that quotes the word it teaches about, and each is now wrapped in `<!-- vale no-hedging.no-hedging = NO -->`, which works natively in markdown and needed no CLI change. Both files are back in scope. This was measured, not assumed. `CommentDelimiters` is the documented way to get directives into a format without comment syntax, and it does nothing here: the key reaches the assembled config and Vale ignores it, because it "applies to the formats that take BlockIgnores" and plain text takes none. Vale 3.19.0, three configurations, one sample with three hedging words and the middle one wrapped: .txt as plain text, CommentDelimiters set 3 hits, directive inert .txt mapped to markdown via [formats] 2 hits, region suppressed native .md with <!-- vale off --> 2 hits, region suppressed The mapping route would have needed a global `[formats]` section the config assembler does not emit. Being markdown needs nothing. `stripValeDirectives` removes the markers where the embedded files are read into the recipe maps, so all three render entry points and `getRawRecipe` are covered by one stroke. Anchored to the start of a line, so `create-vale-rule` can keep TEACHING the syntax without the lesson being stripped out of it. Five tests, including one asserting no served recipe carries a directive; removing the strip fails it by name. pnpm cli check No issues found. 1323 tests pass, typecheck and lint clean.
thecodedrift
changed the title
|
@claude /review |
|
Claude finished @thecodedrift's task in 10m 38s —— View job Review completeReview mode: incremental — read 0 prior review thread(s) before reviewing. The core mechanism ( Posted 6 inline comments:
Lower-confidence/not separately commented: |
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
Review findings on the markdown migration, and one of them was mine to answer for. RENAMING TO .md PUT 20 RECIPES INTO PRETTIER'S SCOPE, and prettier reflowed every one: 1298 insertions and 1175 deletions across a change that was supposed to be a rename. `create-vale-rule` moved enough that git stopped detecting it as a rename at all. These are hand-wrapped documents served verbatim to an agent, so their line breaks are content, and nothing asked for them to be rewritten. `.prettierignore` now excludes them, and every recipe is restored to its original bytes. The diff is a rename plus 10 changed lines. That reflow is also what broke the two directives the reviewer caught. Their `= YES` lines ended up followed by content indented past the code-block threshold, so the coverage they were supposed to restore was not restored. THE DIRECTIVES ARE GONE, AND THE MEASUREMENT IS WHY. A zone has to start at column 0, and both hits sit inside nested list items: at the continuation indent not recognised, words still reported column 0 around one bullet recognised, ends the list, 2 findings became 4 column 0 around whole steps works, and exempts 212 lines to save 2 words The last one is the honest cost: 176 lines of prose out of reach to keep two words of an example, in a change whose purpose is buying coverage back. Both examples now use words the rule does not carry, which costs three words and no coverage. `create-vale-rule` gains a section on exception zones carrying all of the above, since the next author will reach for one. It also states the trap this file walks into: a directive at column 0 in a recipe takes effect on the recipe and is stripped before serving, so a documented example has to be indented or inline. `stripValeDirectives` stays. No recipe carries a directive today, and the guard is for the hazard this change creates rather than for a current use: `.md` makes `<!-- vale ... -->` a natural thing to reach for, and without the strip one would ship silently to every agent reading that recipe. Also from the review: a JSDoc glued to the previous closing brace by my own edit, and five stale `.txt` filenames in test comments. pnpm cli check No issues found. 1323 tests pass, typecheck and lint clean, and prettier reports no recipe as needing formatting.
Three more review findings. Two rule configs still said "These are `.txt`, which Vale treats as PLAIN TEXT" directly above a `*.md` glob. I updated that comment in `no-hedging/.vale.ini` and not in `no-em-dashes/.vale.ini` or `no-blocklist-phrases/.vale.ini`, so two of the three now contradicted the line under them. They describe what a `.md` gets instead, and why the old arrangement forced whole-file exemptions. The leak test iterated `canonicalRecipeTopics()`, which lists canonical names only, so a directive left in a `.anonymous` recipe would not have been asserted against. That variant is served by the same path. It now checks both. The `no-unrouted-cli-invocation` exemption named `schemas/**`. A whole directory is wider than the reason for it: a description that told a reader to RUN something is exactly what the rule is for, and the blanket form would have exempted it silently. `verify-test.ts` and `index.ts` are the only files under `schemas/` naming the package, so both are listed and the next one is a decision rather than something inherited. pnpm cli check No issues found. 1323 tests pass, typecheck and lint clean.
All 13 addressed across The indented-code finding was the important one, and tracing it turned up the actual cause: renaming to The directives themselves are gone. A zone has to start at column 0 and both hits sit inside nested list items. Measured three placements: at the continuation indent the directive is not recognised at all; at column 0 around one bullet it ends the list and two findings became four; at column 0 around the enclosing steps it works and exempts 212 lines to keep two words of an example. Rewriting the examples to use words the rule does not carry costs three words and no coverage, so that is what shipped. Two of your findings I had half-done and did not notice: the On
— AI Coding Agent |
1 participant
Makes
pnpm cli checkreportNo issues found.on this repository, so 0.11.1 is the first release where taskless checks taskless with nothing outstanding.The recipes were never plain text
routealone carries 69 headings, tables, fenced blocks and emphases. They are markdown documents with a.txtextension, and that extension was the problem. The rule config already said so:So
no-hedgingexcludedcreate-vale-ruleandverify-rulewholesale, giving up all coverage on two of the largest recipes to silence one worked example.What the rename buys
.txt.mdThe other 31 were inside fences and code spans that a markdown parser skips. Both files are back in scope. The 2 that remain are prose quoting the word it teaches about, and both examples were rewritten to use words the rule does not carry: three words changed, no coverage given up.
Two things this turned up the hard way
Renaming put the recipes into prettier's scope. Prettier reflowed all 20, 1298 insertions against 1175 deletions on a change that was supposed to be a rename, and
create-vale-rulemoved enough that git stopped detecting it as one. These are hand-wrapped documents served verbatim to an agent, so their line breaks are content..prettierignorenow excludes them and every recipe is restored to its original bytes; the diff is a rename plus 10 changed lines.Exception zones work, but only at column 0. Measured on Vale 3.19.0:
Both remaining hits sit inside nested list items, so the smallest zone available covers a whole numbered step: 176 lines of prose out of reach to preserve two example words, in a change whose purpose is buying coverage back. Rewriting the examples was the better trade.
create-vale-rulegains a section carrying all of this, including the trap that a directive at column 0 in a recipe takes effect on the recipe and is stripped before serving, so a documented example has to be indented.The strip
stripValeDirectivesruns where the embedded files are read into the recipe maps, so all three render entry points andgetRawRecipeare covered once rather than three times.No recipe carries a directive today. The guard is for the hazard this change creates:
.mdmakes<!-- vale ... -->a natural thing to reach for, and without the strip one would ship silently to every agent reading that recipe. It is anchored to the start of a line socreate-vale-rulecan keep teaching the syntax, and the tests cover both the canonical and.anonymousvariants of every topic. Mutation-checked: removing the strip fails the leak test by name.Scope
20 recipes renamed, the
import.meta.globand its stem regex, 4 rule config globs and the comments describing them, the tests that discover recipes by extension, and stale.txtfilenames in comments. No published contract depends on the extension:getPrompt(topic)andagent <topic>are keyed on topic, and the files are embedded at build time.Verification
1323 tests pass,
pnpm typecheckclean,pnpm lintclean, and prettier reports no recipe as needing formatting.Closes #291.