ref(runtime): share the capture enumeration, not only the classifier

`loadCaptureRules` and `verify`'s loop both read `captures/`, filtered to
`.yml`/`.yaml`, sorted, parsed each file and called `assessCaptureRule`. Only
the classification was shared, so a change to what counts as a capture — a
new extension, an exclusion, the sort order — had two call sites to keep in
step, and they could drift even though the classifier could not.

`assessCaptureDirectory` is now the one enumeration both walk. Unparseable
YAML reports through the same channel rather than as a separate branch in
each caller, so there is one list of reasons a capture is refused rather than
one list plus an exception. `verify`'s "no capture rules in captures/" check
counts the same assessments instead of doing its own `readdir`.

Corrects the reason I gave for deferring this. I said the shared helper wanted
to be the one the file-set writer uses when it validates `captures/`, and that
is wrong: the writer inspects delivered paths in memory and never enumerates a
directory, so there was no second use to wait for.

Found by the review on #216.

Author: theCodeDrift

packages/cli/src/rules/inspect.ts

@@ -1,12 +1,10 @@
 import { readFile } from "node:fs/promises";
-import { readdir } from "node:fs/promises";
-import { join } from "node:path";
 
 import { parse } from "yaml";
 
 import { ruleCapturesDirectory, ruleConfigPath, ruleFilePath } from "./engines";
 import { type EngineName } from "./layout";
-import { assessCaptureRule } from "./runtime/discover";
+import { assessCaptureDirectory } from "./runtime/discover";
 import { validateValeRule } from "../schemas/vale-rule";
 import { verifyRule, type VerifyResult } from "./verify";
 import { verifyValeRule } from "./vale/verify";
@@ -177,16 +175,8 @@ export async function verifyOneRule(
   }
   const captures = ruleCapturesDirectory(cwd, engine, ruleId);
   if (captures !== undefined) {
-    let captureFiles: string[] = [];
-    try {
-      const entries = await readdir(captures);
-      captureFiles = entries.filter(
-        (entry) => entry.endsWith(".yml") || entry.endsWith(".yaml")
-      );
-    } catch {
-      captureFiles = [];
-    }
-    if (captureFiles.length === 0) {
+    const assessed = await assessCaptureDirectory(captures);
+    if (assessed.length === 0) {
       errors.push(
         `${ruleId} has no capture rules in captures/, so check.ts would never be invoked.`
       );
@@ -198,18 +188,10 @@ export async function verifyOneRule(
     // the reason is named here, while the author is looking at one rule rather
     // than at an empty report.
     //
-    // `assessCaptureRule` is the SAME function discovery refuses with, so
-    // "verify says it is fine" and "the run silently skipped it" cannot come
-    // apart.
-    for (const fileName of captureFiles.toSorted()) {
-      let parsed: unknown;
-      try {
-        parsed = parse(await readFile(join(captures, fileName), "utf8"));
-      } catch {
-        errors.push(`${ruleId}/captures/${fileName} is not valid YAML.`);
-        continue;
-      }
-      const assessment = assessCaptureRule(parsed);
+    // The SAME enumeration and the SAME assessor discovery uses, so "verify
+    // says it is fine" and "the run silently skipped it" cannot come apart —
+    // neither about which files are candidates, nor about what each one is.
+    for (const { fileName, assessment } of assessed) {
       if (!assessment.ok) {
         errors.push(
           `${ruleId}/captures/${fileName} ${assessment.reason}. The capture is skipped, so the rule would run against less than it was written for.`

packages/cli/src/rules/runtime/discover.ts

@@ -143,33 +143,72 @@ export function assessCaptureRule(value: unknown): CaptureAssessment {
   return { ok: true, rule: candidate as CaptureRule & { id: string }, match };
 }
 
-/** Load and parse the capture rules of a single runtime-rule directory. */
-async function loadCaptureRules(
+/** One capture file in a rule's `captures/`, with the verdict on it. */
+export interface AssessedCapture {
+  /** Absolute path to the capture file. */
+  file: string;
+  /** Basename, as `verify` reports it. */
+  fileName: string;
+  assessment: CaptureAssessment;
+}
+
+/**
+ * Enumerate a rule's `captures/` and assess every file in it.
+ *
+ * ONE ENUMERATION, TWO CALLERS, for the same reason there is one assessor:
+ * discovery and `verify` must not disagree about which files are even
+ * candidates. Sharing only the classifier left this loop duplicated, so a
+ * change to what counts as a capture -- a new extension, an exclusion, the sort
+ * order -- had two call sites to keep in step, and they could drift even though
+ * the classifier could not.
+ *
+ * Unparseable YAML is reported through the same channel rather than as a
+ * separate branch in each caller, so there is one list of reasons a capture is
+ * refused rather than one list plus an exception.
+ */
+export async function assessCaptureDirectory(
   directory: string
-): Promise<LoadedCaptureRule[]> {
+): Promise<AssessedCapture[]> {
   let entries: string[];
   try {
     entries = await readdir(directory);
   } catch {
     return [];
   }
-  const ymlFiles = entries.filter(
-    (f) => f.endsWith(".yml") || f.endsWith(".yaml")
-  );
-  const loaded: LoadedCaptureRule[] = [];
-  for (const fileName of ymlFiles.toSorted()) {
+  const assessed: AssessedCapture[] = [];
+  const candidates = entries
+    .filter((name) => name.endsWith(".yml") || name.endsWith(".yaml"))
+    .toSorted();
+  for (const fileName of candidates) {
     const file = join(directory, fileName);
     let parsed: unknown;
     try {
       parsed = parse(await readFile(file, "utf8"));
     } catch {
-      continue; // not valid YAML — skip
+      assessed.push({
+        file,
+        fileName,
+        assessment: { ok: false, reason: "is not valid YAML" },
+      });
+      continue;
     }
+    assessed.push({ file, fileName, assessment: assessCaptureRule(parsed) });
+  }
+  return assessed;
+}
+
+/** Load the capture rules of one runtime-rule directory, refusing the rest. */
+async function loadCaptureRules(
+  directory: string
+): Promise<LoadedCaptureRule[]> {
+  const loaded: LoadedCaptureRule[] = [];
+  for (const { file, fileName, assessment } of await assessCaptureDirectory(
+    directory
+  )) {
     // Refused, never coerced or guessed at. `verify` names the file and the
-    // reason (see `inspect.ts`, which calls the same assessor), so a capture
+    // reason (see `inspect.ts`, which walks the same assessments), so a capture
     // disappearing from the run is explained rather than left to look like a
     // rule that found nothing.
-    const assessment = assessCaptureRule(parsed);
     if (!assessment.ok) continue;
     const { rule, match } = assessment;
     loaded.push({