Skip to content

Commit 406d89c

Browse files
committed
fix(cli): scope the oversized-file scan to what Vale would actually lint
The preemptive size scan walked every file in the tree, with no relationship to what any Vale rule's matcher covers. Running it against this repository reported pnpm-lock.yaml and packages/cli/CHANGELOG.md as "not checked" even though no rule's .vale.ini section names either file — Vale was never going to open them, so that was a false positive, not a caught coverage hole. assembleValeConfig now returns the section glob patterns it wrote into the assembled config alongside its path, read from the same in-memory strings it is about to write rather than by re-parsing the file afterward. That is threaded through assembleEngineConfigs -> DispatchOptions.valeSections -> ValeRunOptions.sectionGlobs -> findOversizedFiles, which globs by those patterns instead of a bare **/* walk, exactly as findConverterDependentFiles globs by its extension list. A named path is no longer stat-checked unconditionally either, once sections are known: Vale spends 9ms and finds nothing on an oversized file no section names, the same as a file it never opened, so checking it regardless of scope would reintroduce the same false positive via an explicit path instead of a whole-project walk. A caller with no assembled config to ask (runVale's sectionGlobs left undefined) keeps the previous exhaustive walk unchanged - verifyValeRule's isolating config, and any test handing runVale a hand-written .vale.ini directly.
1 parent 150b118 commit 406d89c

11 files changed

Lines changed: 372 additions & 92 deletions

File tree

‎.changeset/skip-oversized-vale-files.md‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -12,16 +12,24 @@ fixed, on a path #300 did not cover).
1212

1313
`runVale` now excludes a target file over 128KB (`VALE_MAX_FILE_BYTES` in
1414
`src/rules/vale/run.ts`) before invoking Vale at all, the same preemptive
15-
treatment already given to a format Vale cannot parse. Excluded files are
16-
named in a `notices` entry, not reported as a failing finding: unlike an
17-
unparseable file (where Vale itself proves the file was a real target by
18-
erroring on it), this exclusion runs from a bare filesystem walk with no way
19-
to confirm any rule's matcher would have reached the file — reporting it as
20-
a blocking error produced false failures on files no rule ever touches (a
21-
lockfile, a generated changelog).
15+
treatment already given to a format Vale cannot parse — but only when some
16+
Vale rule's own `.vale.ini` section could actually reach that file.
17+
`assembleValeConfig` now returns the section patterns it wrote alongside the
18+
config path, and the size scan globs by those patterns (`findOversizedFiles`
19+
in `src/rules/vale/formats.ts`) instead of walking every file in the project.
20+
A first version of this fix scanned the whole tree unconditionally and named
21+
`pnpm-lock.yaml` and `packages/cli/CHANGELOG.md` as "not checked" on this very
22+
repository, even though no rule's matcher touches either file — Vale was
23+
never going to open them, so that was a false positive, not a caught coverage
24+
hole. Excluded files are named in a `notices` entry rather than a finding:
25+
unlike an unparseable file (where Vale itself proves the file was a real
26+
target by erroring on it), this exclusion is a preemptive guess from a
27+
filesystem walk, and a soft advisory fits an unconfirmed guess better than a
28+
hard error.
2229

2330
A consumer may now see a `check` that previously counted a large file's
24-
prose findings instead report a `notices` entry naming that file as skipped.
25-
128KB is comfortably past hand-written prose (roughly 20,000 words); this
26-
should only affect generated output, pasted data, or exported notes checked
27-
directly against a matching rule.
31+
prose findings instead report a `notices` entry naming that file as skipped
32+
— but only for a file some rule's own scope actually reaches. 128KB is
33+
comfortably past hand-written prose (roughly 20,000 words); this should only
34+
affect generated output, pasted data, or exported notes checked directly
35+
against a matching rule.

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -576,15 +576,22 @@ it.
576576
large enough document can consume the whole run's time budget on its
577577
own and cost every other file its findings: the same failure mode as
578578
the unreadable-file case above, from a different cause. `check`
579-
preempts it: a target file over 128KB is skipped, and named in a
580-
`notices` entry rather than a finding, since nothing here can confirm
581-
whether any matcher would actually have reached it (a large lockfile
582-
or a generated file outside every rule's scope is common, and
583-
flagging one as a failure would be a false positive). A rule's own
579+
preempts it: a target file over 128KB is skipped **only if some
580+
matcher's own section would actually reach it**. The scan asks the
581+
assembled config's own section patterns, the same ones you write in
582+
this file's `.vale.ini`, rather than walking every file in the
583+
project. A large lockfile or a generated file no rule's glob names is
584+
left alone entirely, not merely reported softly: naming a file no
585+
matcher was ever going to check would be a false positive, not a
586+
caught coverage hole. A file that IS excluded is named in a `notices`
587+
entry rather than a finding: unlike the unreadable-file case above,
588+
where Vale's own error proves the file was a real target, this is a
589+
preemptive guess from a filesystem walk, and a soft advisory fits an
590+
unconfirmed guess better than a hard error does. A rule's own
584591
fixtures are never this large in practice, so this should not surface
585-
while authoring one. It matters when scoping a matcher at a whole
586-
project, where a generated changelog or an exported note can cross
587-
it.
592+
while authoring one. It matters when a matcher's glob is broad, such as
593+
`[*.md]` or `[**/README.md]` at the project root, where a generated
594+
changelog or an exported note can cross it.
588595
589596
That example changed with Vale v3.18.0, which is the point: the
590597
dangerous extension is whichever one the list above says needs a

‎packages/cli/src/commands/check.ts‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,8 @@ export const checkCommand = defineCommand({
252252
cwd,
253253
paths: existingPaths,
254254
astGrepConfigPath: assembled.sg,
255-
valeConfigPath: assembled.vale,
255+
valeConfigPath: assembled.vale?.path,
256+
valeSections: assembled.vale?.sections,
256257
runtimeRules: plan.execute,
257258
runtimeTimeoutMs: parseTimeoutMs(args.timeout),
258259
});

‎packages/cli/src/rules/assemble.ts‎

Lines changed: 46 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -97,19 +97,59 @@ function valeRuleBlock(ruleId: string, body: string): string {
9797
return [`# tskl) rule = ${ruleId}`, body, ""].join("\n");
9898
}
9999

100+
/**
101+
* A section header (`[pattern]`) from an assembled rule's own body.
102+
*
103+
* Read from the exact string this module is about to write — never from the
104+
* file after writing it. Re-reading the written `.vale.ini` to recover its
105+
* own sections would be the mistake `STYLEGUIDE-CODE.md`'s "Verify Build
106+
* Output In The Build, Not By Parsing It" warns against: this function
107+
* already IS the generator, holding the structured pieces before they are
108+
* joined into text, so there is nothing to re-derive.
109+
*/
110+
function sectionPatternsOf(body: string): string[] {
111+
const patterns: string[] = [];
112+
for (const line of body.split("\n")) {
113+
const match = /^\[(.+)\]$/.exec(line.trim());
114+
if (match?.[1] !== undefined) patterns.push(match[1]);
115+
}
116+
return patterns;
117+
}
118+
119+
/**
120+
* What `assembleValeConfig` produced: where to point `--config`, and the
121+
* section patterns it wrote there.
122+
*
123+
* `sections` exists so a caller that needs to know what Vale would actually
124+
* lint — `findOversizedFiles` in `vale/formats.ts`, scoping its preemptive
125+
* size guard to files some rule's matcher could reach — can ask this module
126+
* directly instead of re-parsing the config it just wrote.
127+
*/
128+
export interface AssembledValeConfig {
129+
/** Config path relative to the project root, for `--config`. */
130+
path: string;
131+
/**
132+
* Every section glob pattern written into the config, deduplicated and
133+
* sorted for a stable read order. Root-relative, exactly as Vale reads
134+
* them — the same strings a `[…]` line in a rule's own `.vale.ini` names.
135+
*/
136+
sections: string[];
137+
}
138+
100139
/**
101140
* Assemble `.taskless/.vale.ini` from every Vale rule's own config.
102141
*
103-
* Returns the config path relative to the project root, or `undefined` when no
142+
* Returns the config path and its section patterns, or `undefined` when no
104143
* Vale rule declares any config — there is nothing to run, and writing an empty
105144
* config would invite Vale to lint the project against no rules and report a
106145
* clean pass.
107146
*/
108147
export async function assembleValeConfig(
109148
cwd: string
110-
): Promise<string | undefined> {
149+
): Promise<AssembledValeConfig | undefined> {
111150
const ruleIds = await listRuleIds(cwd, "vale");
112151
const blocks: string[] = [];
152+
const sections = new Set<string>();
113153

114154
for (const ruleId of ruleIds) {
115155
const configPath = ruleConfigPath(cwd, "vale", ruleId);
@@ -125,6 +165,7 @@ export async function assembleValeConfig(
125165
}
126166
const body = ruleConfigBody(source);
127167
if (body === "") continue;
168+
for (const pattern of sectionPatternsOf(body)) sections.add(pattern);
128169
blocks.push(valeRuleBlock(ruleId, body));
129170
}
130171

@@ -134,7 +175,7 @@ export async function assembleValeConfig(
134175
const target = join(cwd, ASSEMBLED_VALE_CONFIG);
135176
await mkdir(dirname(target), { recursive: true });
136177
await writeFile(target, contents, "utf8");
137-
return ASSEMBLED_VALE_CONFIG;
178+
return { path: ASSEMBLED_VALE_CONFIG, sections: [...sections].toSorted() };
138179
}
139180

140181
/**
@@ -193,8 +234,8 @@ export async function assembleSgConfig(
193234

194235
/** Both assembled configs, for a run that needs whichever engines are present. */
195236
export interface AssembledConfigs {
196-
/** `--config` for Vale, or `undefined` when no Vale rule is configured. */
197-
vale: string | undefined;
237+
/** Vale's config and section patterns, or `undefined` when no Vale rule is configured. */
238+
vale: AssembledValeConfig | undefined;
198239
/** `-c` for ast-grep, or `undefined` when there are no ast-grep rules. */
199240
sg: string | undefined;
200241
}

‎packages/cli/src/rules/dispatch.ts‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,16 @@ export interface DispatchOptions {
9191
* written. The config is the only honest signal that there is Vale work.
9292
*/
9393
valeConfigPath: string | undefined;
94+
/**
95+
* The section glob patterns `assembleValeConfig` wrote into that config, or
96+
* `undefined` when it produced nothing (mirrors `valeConfigPath`).
97+
*
98+
* Threaded through to `runVale` so its preemptive oversized-file guard can
99+
* scope its scan to files some rule's matcher could actually reach, rather
100+
* than statting the whole project — see `findOversizedFiles` in
101+
* `vale/formats.ts`.
102+
*/
103+
valeSections?: string[] | undefined;
94104
/** Runtime rules that survived planning. Empty means the harness is skipped. */
95105
runtimeRules: RuntimeRule[];
96106
runtimeTimeoutMs?: number;
@@ -177,6 +187,7 @@ async function runValeEngine(options: DispatchOptions): Promise<EngineOutcome> {
177187
paths: options.paths,
178188
configPath: options.valeConfigPath,
179189
timeoutMs: options.valeTimeoutMs,
190+
sectionGlobs: options.valeSections,
180191
});
181192

182193
if (outcome.status === "ok") {

‎packages/cli/src/rules/vale/formats.ts‎

Lines changed: 109 additions & 42 deletions
Original file line numberDiff line numberDiff line change
@@ -166,6 +166,11 @@ const UNWALKED_DIRECTORIES = new Set([
166166
TASKLESS_DIRECTORY,
167167
]);
168168

169+
/** `glob`'s `exclude` predicate for {@link UNWALKED_DIRECTORIES}. */
170+
function isUnwalkedEntry(entry: string | Buffer): boolean {
171+
return UNWALKED_DIRECTORIES.has(basename(String(entry)));
172+
}
173+
169174
/**
170175
* Converter-dependent files inside the run's target set.
171176
*
@@ -260,18 +265,48 @@ export interface OversizedFile {
260265
* discover the cost the hard way — is the same trade `converterExclusionGlobs`
261266
* makes for a format Vale cannot parse at all.
262267
*
263-
* Unlike the converter walk, this cannot be scoped by extension: an oversized
264-
* file can have any extension, or none, so every file under the target roots
265-
* has to be listed and stat'd. That is a real cost on the happy path, where
266-
* nothing is oversized and the walk still runs — measured and reported in the
267-
* PR that introduced this function.
268-
*
269-
* A named path is stat'd directly, exactly as `targetFileParseError` does
270-
* elsewhere in this package: an explicit request is not resolved through the
271-
* walk that answers a whole-project run. Whether named or discovered by the
272-
* walk, an oversized file is excluded unconditionally, on every run — the same
273-
* asymmetry `findConverterDependentFiles` documents, and for the same reason:
274-
* handing Vale this file does not check it badly, it risks the entire batch's
268+
* `sectionGlobs`, when given, is `AssembledValeConfig.sections` from
269+
* `assembleValeConfig` — the exact section patterns Vale's own rules are
270+
* scoped to. The scan globs those patterns instead of every file in the
271+
* tree, exactly as {@link findConverterDependentFiles} globs by its extension
272+
* list, and for the same reason precision matters here: a bare `**\/*` walk
273+
* finds every file under the target roots regardless of whether any rule
274+
* would ever touch it, and reporting one of those as "not checked" is a false
275+
* positive, not a caught coverage hole. Measured against this repository:
276+
* `pnpm-lock.yaml` and `packages/cli/CHANGELOG.md` are both over the limit,
277+
* and neither is named by any `[section]` in any rule's `.vale.ini` — no
278+
* rule was ever going to open either one, so the un-scoped walk reported
279+
* lost coverage that never existed.
280+
*
281+
* The patterns are read from `assembleValeConfig`'s own return value, never
282+
* by re-parsing the `.vale.ini` it wrote — see the doc on
283+
* `sectionPatternsOf` in `assemble.ts` for why that distinction matters.
284+
*
285+
* `sectionGlobs === undefined` falls back to the previous exhaustive `**\/*`
286+
* walk under each target root. That path exists for a caller with no
287+
* assembled config to ask — `verifyValeRule`'s isolating config, or a test
288+
* that hands `runVale` a hand-written `.vale.ini` directly — and is
289+
* unaffected by everything below: same cost, same behavior as before this
290+
* parameter existed.
291+
*
292+
* A named path is stat'd directly when there is no `sectionGlobs` to consult
293+
* (the fallback below), exactly as `targetFileParseError` does elsewhere in
294+
* this package: an explicit request is not resolved through the walk that
295+
* answers a whole-project run. **That changes once `sectionGlobs` is given.**
296+
* An explicitly named file is not exempt from scoping either — measured
297+
* against the real binary, Vale spends 9ms and reports nothing on a 128KB+
298+
* file whose extension no section names, the same as a file it never opened
299+
* at all, because no rule is ever assigned to run against it. Checking it
300+
* unconditionally would reintroduce the exact false positive this parameter
301+
* exists to remove, just reachable via `check some-file.yaml` instead of a
302+
* whole-project run. So when sections are known, a named file is a candidate
303+
* only if it is also a match for one of them — the same membership test the
304+
* walk below already computes.
305+
*
306+
* Whether named or discovered by the walk, an oversized file is excluded
307+
* unconditionally once it qualifies, on every run — the same asymmetry
308+
* `findConverterDependentFiles` documents, and for the same reason: handing
309+
* Vale this file does not check it badly, it risks the entire batch's
275310
* timeout.
276311
*
277312
* Errors are swallowed the same way as {@link findConverterDependentFiles} and
@@ -283,51 +318,83 @@ export interface OversizedFile {
283318
export async function findOversizedFiles(
284319
cwd: string,
285320
paths: string[],
286-
maxBytes: number
321+
maxBytes: number,
322+
sectionGlobs?: string[]
287323
): Promise<OversizedFile[]> {
288-
const named: OversizedFile[] = [];
289-
const roots: string[] = [];
290-
if (paths.length === 0) {
291-
roots.push(".");
292-
} else {
293-
for (const path of paths) {
324+
const roots = paths.length === 0 ? ["."] : paths;
325+
const found = new Map<string, OversizedFile>();
326+
327+
const checkCandidate = async (relative: string): Promise<void> => {
328+
if (found.has(relative)) return;
329+
try {
330+
const stats = await stat(resolvePath(cwd, relative));
331+
if (stats.isFile() && stats.size > maxBytes) {
332+
found.set(relative, { file: relative, size: stats.size });
333+
}
334+
} catch {
335+
// Gone between listing and stat, or unreadable. Not a reason to drop
336+
// the exclusion already computed.
337+
}
338+
};
339+
340+
if (sectionGlobs === undefined) {
341+
// No assembled config to ask what Vale would actually lint — fall back to
342+
// the previous behavior: every named path is a candidate regardless of
343+
// scope, and every root is walked exhaustively.
344+
for (const path of paths) await checkCandidate(path);
345+
for (const root of roots) {
346+
const prefix = root === "." || root === "" ? "" : `${root}/`;
294347
try {
295-
const stats = await stat(resolvePath(cwd, path));
296-
if (stats.isFile() && stats.size > maxBytes) {
297-
named.push({ file: path, size: stats.size });
348+
for await (const match of glob(`${prefix}**/*`, {
349+
cwd,
350+
exclude: isUnwalkedEntry,
351+
})) {
352+
await checkCandidate(String(match));
298353
}
299354
} catch {
300-
// Unreadable or missing: not this function's problem. The run itself
301-
// will report it if it matters.
355+
// A target that is not a directory, an unreadable subtree, a
356+
// platform where `glob` rejects the pattern: all of them mean "no
357+
// notice, never no fix". The exclusion has already been applied by
358+
// the time this runs.
302359
}
303-
roots.push(path);
304360
}
361+
return [...found.values()].toSorted((a, b) => a.file.localeCompare(b.file));
305362
}
306363

307-
const found = new Map(named.map((entry) => [entry.file, entry]));
308-
for (const root of roots) {
309-
const prefix = root === "." || root === "" ? "" : `${root}/`;
364+
// Section patterns are root-relative, exactly as Vale reads them — never
365+
// prefixed per target root, the way the extension-based fallback above is.
366+
// A whole-project run (`roots === ["."]`) needs no further narrowing: every
367+
// match is already in scope. An explicit target (`check src/` or
368+
// `check src/doc.md`) narrows the matches down to that subtree afterward
369+
// instead, because a section like `CLAUDE.md` or `**/README.md` has no
370+
// meaningful "under src/" form to prefix onto — Vale itself evaluates every
371+
// section against the whole project and only its own target list decides
372+
// what it actually visits, so intersecting after the glob mirrors that
373+
// rather than guessing at one. This is also what makes a named file's
374+
// in-scope test free: it needs no separate membership check, because a
375+
// pattern like `**/README.md` already matches a top-level `README.md`
376+
// found this way, whether or not the caller named it explicitly.
377+
const wholeProject = paths.length === 0;
378+
for (const pattern of sectionGlobs) {
310379
try {
311-
for await (const match of glob(`${prefix}**/*`, {
380+
for await (const match of glob(pattern, {
312381
cwd,
313-
exclude: (entry) => UNWALKED_DIRECTORIES.has(basename(String(entry))),
382+
exclude: isUnwalkedEntry,
314383
})) {
315384
const relative = String(match);
316-
if (found.has(relative)) continue;
317-
try {
318-
const stats = await stat(resolvePath(cwd, relative));
319-
if (stats.isFile() && stats.size > maxBytes) {
320-
found.set(relative, { file: relative, size: stats.size });
321-
}
322-
} catch {
323-
// Same reasoning as above: gone between listing and stat, or
324-
// unreadable. Not a reason to drop the exclusion already computed.
385+
if (
386+
!wholeProject &&
387+
!roots.some(
388+
(root) => relative === root || relative.startsWith(`${root}/`)
389+
)
390+
) {
391+
continue;
325392
}
393+
await checkCandidate(relative);
326394
}
327395
} catch {
328-
// A target that is not a directory, an unreadable subtree, a platform
329-
// where `glob` rejects the pattern: all of them mean "no notice, never
330-
// no fix". The exclusion has already been applied by the time this runs.
396+
// Same reasoning as the fallback walk: a malformed pattern or an
397+
// unreadable subtree means "no notice, never no fix".
331398
}
332399
}
333400

0 commit comments

Comments
 (0)