Skip to content

Commit f4d82cb

Browse files
committed
fix(vale): measure tokens and swap, and correct the spec to match
The recipe scopes the lookaround/backreference claim to `tokens` and `swap`, but every measurement behind it went through `raw`. Measuring the other two against the vendored binary found two silent divergences: - a backreference does nothing as a `swap` key, where the identical pattern fires under `tokens` and `raw`, with nothing on stderr - the implicit `\b` on `tokens`/`swap` is appended after a trailing lookahead, so `foo(?=bar)` can never match there while `raw` fires Step 3 now carries both as caveats instead of a flat "they work", and the vendor-contract suite pins them. Also corrects `cli-vale-rule-engine`, which still described a `[.taskless/**]` matcher as acting "only under a bare vale invocation" — the claim #370/#371 removed from the recipe and the advisory — and states the imprecise topic span as the measured v1-through-v12.
1 parent bc16225 commit f4d82cb

8 files changed

Lines changed: 263 additions & 8 deletions

File tree

‎.changeset/vale-recipe-regex-and-fixtures.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,6 @@
22
"@taskless/cli": patch
33
---
44

5-
`agent create-vale-rule` (topic v13) corrects two claims that cost rule authors work. Vale patterns are not RE2-only: Vale compiles with Go's `regexp` and falls back to `regexp2`, so lookahead, lookbehind and backreferences all work, and the recipe no longer tells you to split a rule that one pattern expresses. `check` on a rule's `.tests/fail` bucket is a supported way to read a rendered message, because `.taskless/` is excluded from the whole-project walk only; when that bucket comes back empty, the cause named is the rule's own `[.taskless/**]` matcher rather than the config.
5+
`agent create-vale-rule` (topic v13) corrects two claims that cost rule authors work. Vale patterns are not RE2-only: Vale compiles with Go's `regexp` and falls back to `regexp2`, so lookahead, lookbehind and backreferences work, and the recipe no longer tells you to split a rule that one pattern expresses. Two silent limits are measured and documented alongside it: a backreference does nothing as a `swap` key, and the implicit word boundary on `tokens`/`swap` lands after a trailing lookahead, so that lookahead has to peek at a non-word character. `check` on a rule's `.tests/fail` bucket is a supported way to read a rendered message, because `.taskless/` is excluded from the whole-project walk only; when that bucket comes back empty, the recipe now sends you to the rule's own config, specifically a `[.taskless/**]` matcher, before the pattern.
66

77
`verify` carried the same imprecision and now states both halves: a `[.taskless/**]` matcher is unnecessary on a whole-project check, AND it silences the rule on a path you name, such as the rule's own fixture bucket. It was previously described as acting only under a bare `vale` invocation, which read as harmless. `agent update` (topic v10) is corrected to match.
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
schema: spec-driven
2+
created: 2026-09-22
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
## Why
2+
3+
taskless/cli#370 and #371 corrected the recipe and the `verify` advisory: a
4+
`[.taskless/**]` matcher is not harmless. `check` excludes `.taskless/` from a
5+
_whole-project walk_ only, so the matcher does nothing there — but on a path
6+
named explicitly, such as the rule's own `.tests/fail` bucket, the matcher is
7+
live and silences the rule. That is the shape that reproduces "`test` says the
8+
fixture fired, `check` on the same fixture says nothing".
9+
10+
The recipe, the `update` ledger and the advisory string now all say both halves.
11+
The standing spec does not. `cli-vale-rule-engine` still reads "`check` excludes
12+
that tree before Vale runs, so the matcher acts only under a bare `vale`
13+
invocation", and its scenario still requires `verify` to "report that `check`
14+
already excludes that tree" — the exact imprecise phrasing the code no longer
15+
ships. The spec is the source of truth for this capability, so leaving it
16+
disagreeing with the advisory it describes is how the next author reproduces
17+
#370 from the spec instead of the recipe.
18+
19+
This change carries no code. The implementation already landed in this PR; the
20+
spec is what is behind.
21+
22+
## What Changes
23+
24+
- **`cli-vale-rule-engine`** — the advisory bullet and the
25+
`.taskless/**` scenario state both halves of the behaviour: unnecessary on a
26+
whole-project check, AND silencing on a named path. The scenario also gains
27+
the `check`-notices half that the other advisory scenarios already carry, so
28+
the two advisory paths are specified alike.
29+
30+
No requirement is added or removed, and no behaviour changes: this is the spec
31+
catching up to an advisory string and a recipe that already shipped.
32+
33+
## Capabilities
34+
35+
### New Capabilities
36+
37+
None.
38+
39+
### Modified Capabilities
40+
41+
- `cli-vale-rule-engine`: "A rule's Vale config is validated against a schema
42+
before it is assembled" — the `.taskless/**` advisory bullet and its scenario
43+
are restated to match the shipped advisory.
44+
45+
## Impact
46+
47+
Documentation only. No source file, test, or public surface changes. The bump
48+
stays `patch` and rides the existing changeset for this PR; no second changeset
49+
is added.
50+
51+
## Delivery shape
52+
53+
**Single PR.** The spec correction is two edits inside one requirement and
54+
belongs with the code change that made the standing text wrong, which is this
55+
PR. It is the tip, so the change is archived here.
Lines changed: 84 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,84 @@
1+
## MODIFIED Requirements
2+
3+
### Requirement: A rule's Vale config is validated against a schema before it is assembled
4+
5+
The system SHALL parse each rule's `.vale.ini` into an ordered, lossless AST and validate that AST against a schema keyed by the rule's directory id, before the config is assembled into the run config and when the rule is verified. Validation SHALL be performed on the parsed structure, never by matching the file's text.
6+
7+
The schema SHALL reject a config that:
8+
9+
- assigns any property above its first matcher (`StylesPath`, `MinAlertLevel`, or anything else; Vale ignores such a line with a `W101` warning and the rule verifies clean while enabled nowhere)
10+
- declares a matcher without a `tskl) rule = <id>` breadcrumb naming this rule
11+
- assigns a key other than `<id>.<id>` (a `<style>.<check>` key naming any other rule is a cross-rule override)
12+
- assigns a value other than `YES` or `NO`
13+
- assigns `BasedOnStyles`, with any value, empty included (on Vale 3.22.0 an empty value clears every earlier matcher's settings for the file, which in the assembled config silences every other rule whose glob reaches it; a named style loads alongside every overlapping rule; and no rule config ever needed either, since no bundled style loads unless a run-level `BasedOnStyles` names one)
14+
- declares no matcher
15+
- never assigns `<id>.<id> = YES` in its final per-matcher verdicts (matchers with the same glob are folded, as Vale merges them, and the last assignment wins, so a `YES` that a later `NO` in the same matcher overrides does not count)
16+
- declares a `NO`-verdict matcher before every `YES`-verdict matcher (no style is loaded, so a rule is off until a `YES`, and such a `NO` is either dead or overridden by the `YES` that follows; no config means it)
17+
18+
The schema SHALL report, without rejecting, a config that:
19+
20+
- assigns the same key twice inside one matcher (Vale 3.21.0 keeps the last assignment; 3.20.0 kept the first)
21+
- declares a `[*]` matcher
22+
- declares a matcher under `.taskless/**` (`check` excludes that tree from a whole-project walk, so the matcher is unnecessary there; it is not harmless, because it silences the rule on a path named explicitly, such as the rule's own fixture bucket under `.taskless/`)
23+
24+
A rejected config SHALL refuse the Vale engine for that `check` run: the engine reports a failure naming the rule and the offending line, that failure SHALL reach the exit code, and other engines SHALL still run. A rule with a rejected config SHALL NOT be silently omitted from the assembled config, because a rule that is present, verifies, and reports nothing is the silent-disable failure this engine's design exists to prevent. Advisories SHALL be surfaced as notices and SHALL NOT affect the exit code.
25+
26+
Assembly SHALL write each accepted config's source verbatim. The parsed structure is read for validation and for the list of matcher patterns; it is not re-serialized.
27+
28+
#### Scenario: A foreign assignment refuses the run
29+
30+
- **WHEN** `no-simply/.vale.ini` assigns `no-hedging.no-hedging = NO` and `check` runs
31+
- **THEN** the Vale engine SHALL report a failure naming `no-simply` and that line
32+
- **AND** the exit code SHALL be non-zero
33+
- **AND** ast-grep results for the same run SHALL still be reported
34+
35+
#### Scenario: A run-level key in a rule config is rejected
36+
37+
- **WHEN** a rule's config places `StylesPath = .` above its first matcher
38+
- **THEN** `verify` SHALL reject the rule, naming the key and the line
39+
- **AND** `check` SHALL refuse the Vale engine rather than strip the line
40+
41+
#### Scenario: A matcher without a breadcrumb is rejected
42+
43+
- **WHEN** a rule's config declares `[*.md]` with `<id>.<id> = YES` and no `tskl) rule` key
44+
- **THEN** `verify` SHALL reject the rule, naming the matcher
45+
46+
#### Scenario: A disable that precedes every enable is rejected
47+
48+
- **WHEN** a rule's config declares `[docs/legacy/**]` with `<id>.<id> = NO` and then `[docs/**]` with `<id>.<id> = YES`
49+
- **THEN** `verify` SHALL reject the rule, naming the `NO` matcher and the `YES` that re-enables it
50+
51+
#### Scenario: A BasedOnStyles assignment is rejected
52+
53+
- **WHEN** a rule's config sets `BasedOnStyles =` inside a matcher
54+
- **THEN** `verify` SHALL reject the rule under `vale-config-no-based-on-styles`, naming the line and saying that the line silences the other rules whose globs overlap
55+
- **AND** a `BasedOnStyles` naming a style SHALL be rejected under the same constraint
56+
57+
#### Scenario: A [formats] section is rejected as a matcher it cannot be
58+
59+
- **WHEN** a rule's config declares a `[formats]` section
60+
- **THEN** `verify` SHALL reject it for the breadcrumb it lacks and the foreign key it assigns, so no rule config can move a file between parser tiers
61+
62+
#### Scenario: A repeated key is reported, not rejected
63+
64+
- **WHEN** one matcher assigns `<id>.<id> = YES` and then `<id>.<id> = NO`, and an earlier matcher assigns `<id>.<id> = YES`
65+
- **THEN** `verify` SHALL accept the rule and report the repeat as a notice
66+
- **AND** `check` SHALL run the Vale engine and carry the same text in its notices
67+
68+
#### Scenario: A rule whose only enable is overridden is rejected
69+
70+
- **WHEN** the only matcher assigning `<id>.<id> = YES` later assigns `<id>.<id> = NO`, in the same section or in a second section with the same glob
71+
- **THEN** `verify` SHALL reject the rule as present but off, under `vale-config-enabled-somewhere`
72+
- **AND** the repeat SHALL still be reported as a notice
73+
74+
#### Scenario: A `.taskless/**` matcher is reported as unnecessary
75+
76+
- **WHEN** a rule's config declares a matcher under `.taskless/**`
77+
- **THEN** `verify` SHALL accept the rule and report both halves: that the matcher is unnecessary on a whole-project check, which excludes `.taskless/` before Vale runs, AND that it silences the rule on a path named explicitly, such as the rule's own fixture bucket
78+
- **AND** `check` SHALL carry the same text in its notices
79+
80+
#### Scenario: An accepted config is assembled byte-for-byte
81+
82+
- **WHEN** a config passes the schema
83+
- **THEN** the assembled run config SHALL contain that file's bytes unchanged under the rule's breadcrumb comment
84+
- **AND** the matcher patterns reported for the run SHALL equal the section names in the parsed structure
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
## 1. Spec
2+
3+
- [x] 1.1 Restate the `.taskless/**` advisory bullet in
4+
`cli-vale-rule-engine` to say it is unnecessary on a whole-project check
5+
AND silencing on a named path.
6+
- [x] 1.2 Restate the `A .taskless/** matcher is reported as unnecessary`
7+
scenario to require both halves, plus the `check`-notices half the other
8+
advisory scenarios carry.
9+
- [x] 1.3 Confirm the MODIFIED block restates the requirement in full: same
10+
title, all ten scenarios, and every bullet of both lists.
11+
- [x] 1.4 Dry-run `openspec archive` and diff the scenario count before and
12+
after to prove nothing is dropped.

‎openspec/specs/cli-vale-rule-engine/spec.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -233,7 +233,7 @@ The schema SHALL report, without rejecting, a config that:
233233

234234
- assigns the same key twice inside one matcher (Vale 3.21.0 keeps the last assignment; 3.20.0 kept the first)
235235
- declares a `[*]` matcher
236-
- declares a matcher under `.taskless/**` (`check` excludes that tree before Vale runs, so the matcher acts only under a bare `vale` invocation)
236+
- declares a matcher under `.taskless/**` (`check` excludes that tree from a whole-project walk, so the matcher is unnecessary there; it is not harmless, because it silences the rule on a path named explicitly, such as the rule's own fixture bucket under `.taskless/`)
237237

238238
A rejected config SHALL refuse the Vale engine for that `check` run: the engine reports a failure naming the rule and the offending line, that failure SHALL reach the exit code, and other engines SHALL still run. A rule with a rejected config SHALL NOT be silently omitted from the assembled config, because a rule that is present, verifies, and reports nothing is the silent-disable failure this engine's design exists to prevent. Advisories SHALL be surfaced as notices and SHALL NOT affect the exit code.
239239

@@ -288,7 +288,8 @@ Assembly SHALL write each accepted config's source verbatim. The parsed structur
288288
#### Scenario: A `.taskless/**` matcher is reported as unnecessary
289289

290290
- **WHEN** a rule's config declares a matcher under `.taskless/**`
291-
- **THEN** `verify` SHALL accept the rule and report that `check` already excludes that tree
291+
- **THEN** `verify` SHALL accept the rule and report both halves: that the matcher is unnecessary on a whole-project check, which excludes `.taskless/` before Vale runs, AND that it silences the rule on a path named explicitly, such as the rule's own fixture bucket
292+
- **AND** `check` SHALL carry the same text in its notices
292293

293294
#### Scenario: An accepted config is assembled byte-for-byte
294295

‎packages/cli/src/agent/create-vale-rule.md‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,22 @@ it.
491491
followed by Y" is therefore writable as a single `substitution`,
492492
but splitting it or narrowing with `scope` is still the cheaper
493493
rule when either will do.
494+
495+
Two measured limits sit on top of that, and both are silent:
496+
497+
- **A backreference does nothing in `swap`.** The same
498+
`(\w+) \1` that fires under `tokens` and under `raw` produces
499+
no finding as a `swap` key, with nothing on stderr and the
500+
rule loading cleanly, so the rule looks healthy and never
501+
fires at all. A repeated-word check has to be an `existence`
502+
rule; it cannot be a `substitution`.
503+
- **A trailing lookahead in `tokens` or `swap` has to peek at a
504+
non-word character.** The implicit `\b` is appended after the
505+
lookahead (the lookahead is zero-width, so the position is
506+
still where the match ended), which puts the boundary between
507+
the match and the text peeked at. `foo(?= bar)` fires;
508+
`foo(?=bar)` can never match, whatever the document says. Use
509+
`raw` when the lookahead has to land on a word character.
494510
- **Word boundaries are applied for you, around the whole pattern.**
495511
Measured: `Github` does not fire inside `GithubToken`, and the
496512
multi-word `click here` does not fire inside `Clicking here`.

0 commit comments

Comments
 (0)