fix(vale): reject a backreference in a swap key

[thecodedrift]

What

verify now rejects a Vale substitution rule whose swap key carries a backreference. It is the first hard error the Vale style layer raises for a pattern-quality problem the binary itself accepts.

The mechanism

A backreference in a swap key can never match, and Vale says nothing about it. The rule loads, runs on every document, writes nothing to stderr, and reports no finding. Measured against the vendored 3.22.0 binary:

swap key	as swap	same pattern under tokens / raw
(\w+) \1	silent	fires
(the) \1	silent	fires
the \1	silent	—
(?:\w+) \1	silent	—
the the (literal control)	fires	fires

The cause is not the regex engine. NewSubstitution (internal/check/substitution.go) compiles all of a rule's keys into one alternation, wrapping each key in a capture group of its own, because the index of the group that matched is how it recovers the replacement:

tokens += `(` + regexstr + `)|`

Those wrappers have to be numbered 1..n, so any group the author wrote is rewritten away first:

convertCaptureGroups(regexstr)   // `(?<!\\)\((?!\?)` -> `(?:`

Vale's guard only runs the rewrite when the key has a capturing (. That is why all four forms above behave alike: with a group, it is converted and \1 becomes the wrapper; without one, the rewrite is skipped and \1 is the wrapper. Either way it is a self-reference to the group still being matched. The true rule is therefore not "a backreference does nothing in swap" but no capture group survives a swap key.

The E201 evidence, and what it is not

There is no E201 here, and that is the point. Every existing hard error in this layer is justified by whole-run blast radius and its message says so: an unrecognised field or action name draws an E201, and Vale reads one config for the whole run, so a single bad key suppresses every other rule's findings; a sequence with no tokens panics with a Go stack trace and no findings at all. A swap backreference earns none of that. Vale's config parses, E201 is not raised, stderr is empty, the exit status is clean, and every other rule keeps reporting normally.

Why it is still a hard error

Owner's call, and the reasoning is "if it's broken in vale, we don't want it to be written as a rule."

The justification that the message has to carry itself: a rule that loads and never fires is indistinguishable from a clean project. A blast-radius failure is loud and gets fixed; this one is silent, and the author's own fail/ fixture is the only thing that could catch it — after they have already written the rule. So the message names the offending key, says the key can never match and that Vale reports nothing, and names existence as the check to write instead.

The measurement that justifies it

Migration cost is effectively zero. Across the seven published Vale style packages (Microsoft, Google, write-good, proselint, alex, Readability, Joblint) plus this repository's own rules: 161 rule files, 33 with a swap map, 1,502 swap keys, 0 carrying a live backreference. check is unaffected, and test already fails a rule whose fail/ fixture never fires.

That near-zero cost is also what makes detector precision load-bearing: there is no suppression mechanism anywhere in the CLI, so a false positive leaves an author with no recourse at all.

The detector

Variant A, character-class aware. Since convertCaptureGroups only ever replaces (, it can neither create nor destroy a \<digit> — so "apply the rewrite, then look for a survivor" collapses to one left-to-right scan for an unescaped \<1-9> outside a character class.

Character-class awareness is mandatory, not polish: inside […] a \1 is an octal escape, and [\1a]bc was measured matching abc.

key	detector	measured in Vale
(\w+) \1	reject	silent
(the) \1	reject	silent
the \1	reject	silent
(?:\w+) \1	reject	silent
[\1a]bc	accept	fires on abc
a\\1b	accept	fires on a\1b
q\(1\)z	accept	fires on q(1)z
(?:bull|ox)-like	accept	fires on ox-like
colour(s?) → color$1	accept	offers colors

Variant B — flag a backreference only when the key has a group to convert — was rejected: it misses the \1 and (?:\w+) \1, both genuinely inert, and flags nothing extra on any real corpus.

The rejection lives in the style layer, per rule, not in vale-config.ts's config layer: a config-layer rejection aborts the whole run and hides every other rule's findings.

The false mechanism claim, corrected

The recipe and vale-vendor-contract.test.ts both said Vale compiles a pattern with Go's own regexp first and falls back to regexp2 when that engine refuses it. There is no such path. Compile in internal/regex/regex.go:63 is:

func Compile(expr string) (*Regexp, error) {
	re, err := regexp2.Compile(expr, regexp2.RE2)
	...
}

regexp2 in RE2 compatibility mode, unconditionally. The user-facing conclusion (lookaround and backreferences are available) is unchanged; the explanation was wrong, and a wrong explanation is what made "but not in swap" look like an engine quirk rather than a substitution-only compilation detail.

Also in this PR

• create-vale-rule → topic v14. The constraint is generalised to "no capture group survives a swap key" with all four measured forms; $1 in the swap value is documented as working (Vale expands it against the original, unconverted key in subMsg); \1 in a character class is documented as fine; and the leading-lookbehind mirror is added — the recipe documented only the trailing-lookahead direction, but (?<=x)foo is equally silent under tokens and swap on xfoo and fires under raw.
• Vendor contract extended to consistency and conditional, so a widening of the blast radius is caught rather than assumed. Both were measured unaffected: consistency wraps each either key in a named group and regexp2 numbers named groups after unnamed ones, so the author's \1 still points at the author's group; conditional compiles first and second directly with no wrapping. (A first pass suggested both were broken too — that was a YAML double-quote escaping artifact in the probe, not Vale.)
• OpenSpec: one ADDED requirement on cli-rule-validation, "The schema may reject a pattern the binary accepts only when the pattern can never fire". No standing requirement is restated or renamed. Dry-run archive verified purely additive: 37 → 42 scenarios, 7 → 8 requirements, 0 dropped. Archived on this PR, which is the tip.

Gates

pnpm build, pnpm typecheck, pnpm lint (eslint + cli check: no issues), pnpm test — 1719 passed, 102 files, 0 failed.

---

Upstream issue draft — NOT FILED

This has not been filed and should not be filed as-is. The owner declined for now: "no. I don't trust you and I'd want to create a standalone reproduction repo."

Before anyone files this, it needs a standalone reproduction repository built from scratch — an independent Vale config, rules and fixtures, with a Vale binary obtained directly from upstream releases rather than our vendored @taskless/vale-* package. The report has to stand on its own, so that a maintainer can reproduce it without taking anything about our scaffolding on trust. Nothing below has been verified outside this repository.

Draft text follows, for copy-paste if and when that exists:

Title: substitution: a backreference in a swap key is silently ignored

Version: 3.22.0

What happens. A swap key containing a backreference never matches, and Vale reports nothing — no E201, nothing on stderr, clean exit. The identical pattern fires under existence with tokens and with raw.

# Repeat.yml
extends: substitution
message: "repeated: %s"
level: error
swap:
  '(\w+) \1': X

Against a document reading A the the repeated word. this produces no findings. Replacing the key with the literal the the produces one, so the rule, the config and the document are otherwise fine.

Why. NewSubstitution compiles all of a rule's swap keys into one alternation, wrapping each key in a capture group so the matched group's index identifies the replacement, and rewrites the author's groups to non-capturing via convertCaptureGroups to keep that numbering. A \1 in the key therefore refers to Vale's own wrapper rather than to anything in the pattern. Keys with no capturing group at all (the \1, (?:\w+) \1) are equally inert, since the rewrite is skipped and \1 is the wrapper directly.

Why it matters. The failure is silent. A rule that loads and never fires is indistinguishable from a rule with nothing to report, so an author gets no signal at the moment they are looking.

Suggested fix, in preference order:

1. Report it at load: an E201 on a swap key containing a backreference would make the rule's deadness visible where every other config mistake is already reported.
2. Failing that, document it on the substitution page, which currently says nothing about capture groups in keys.

$1 in the swap value is unaffected and works, because subMsg re-applies the original, unconverted key — worth keeping distinct in any documentation fix.

Fixes #392

[thecodedrift]

@claude /review

[claude]

Claude finished @thecodedrift's task in 5m 17s —— View job

---

Review complete

Review mode: incremental — read 0 prior review thread(s) before reviewing. .prior-review.json had no reviewThreads, reviews, or comments recorded for this PR, so this is the first review; there is nothing previously-raised to reconcile.

• Read .prior-review.json (empty — first review)
• Gather diff via gh pr diff 399
• Read changed files in full context (vale-rule.ts, create-vale-rule.md, both test files, the OpenSpec delta and archive)
• Cross-file check: confirmed validateValeRule is reached only from rules/inspect.ts, which only commands/verify.ts imports — supports the PR's claim that check is unaffected
• Post findings

Assessment

This is a tight, well-evidenced change. The detector (hasLiveBackreference / swapBackreferenceMessages in packages/cli/src/schemas/vale-rule.ts) is character-class aware and I traced it by hand against every case in the test suite ((\w+) \1, the \1, (?:\w+) \1, [\1a]bc, a\\1b, q\(1\)z) — the left-to-right scan with the backslash-pair-skip and inClass tracking gets all of them right, including escaped brackets (\[, \]) inside/outside a class. The scoping to extends === "substitution" correctly leaves consistency/spelling (permissive checks) alone, matching the measured fact that their backreferences aren't broken. The OpenSpec delta is a pure ADDED block (no MODIFIED), so there's no risk of the archive silently dropping an existing scenario. No functional bugs found.

Two minor, non-functional findings, both comment-only:

1. [New] Inline comment posted on packages/cli/src/schemas/vale-rule.ts:716 — the guard comment says swap "already rejects it on the other ten checks," but VALE_CHECK_TYPES has 12 entries; minus substitution and the 2 permissive checks (consistency, spelling) named in the next sentence, that's 9 strict checks, not 10. Doesn't affect runtime behavior.

2. [New] packages/cli/test/recipe-cross-references.test.ts:431-434 still carries the exact claim this PR sets out to correct ("Vale falls back to regexp2") in its explanatory comment. This file isn't touched by the PR and isn't part of this diff, so I didn't post an inline comment on it (GitHub won't accept one on a file with no diff hunk here), but since the PR's whole point is retiring that two-engine-fallback explanation everywhere it appears (create-vale-rule.md and vale-vendor-contract.test.ts are both fixed), this third occurrence looks like the one spot the sweep missed. The test's assertions themselves (toContain("regexp2"), not.toMatch(/do not exist in RE2/)) still pass either way — it's the prose explaining why the test exists that's now inaccurate.

Everything else checked out: the recipe's new "leading lookbehind" mirror, the $1-in-value vs \1-in-key asymmetry, the case-insensitive swap/Swap key handling (via canonicalKeys, since swap isn't in VALE_LITERAL_KEYS), the vendor-contract additions for consistency/conditional, and the changeset/patch-bump reasoning under the 0.y.z convention.

[claude]

[New] Minor: "the other ten checks" is off by one. VALE_CHECK_TYPES has 12 entries; excluding substitution leaves 11, of which 2 (consistency, spelling) are the permissive ones named in the very next clause, leaving 9 strict checks that reject swap via E201 — not ten. Doesn't affect behavior (comment only), but worth fixing given how measurement-precise the rest of this PR is.