# The review file set - what was read, and what was not, on the record - [Why this exists](#why-this-exists) - [The set is fixed before the reviewer sees the diff](#the-set-is-fixed-before-the-reviewer-sees-the-diff) - [Every exclusion names the pattern that produced it](#every-exclusion-names-the-pattern-that-produced-it) - [The reviewer answers for each file](#the-reviewer-answers-for-each-file) - [Invocation](#invocation) - [What this is not](#what-this-is-not) > The denominator for Phase 4's other axis. Loaded on demand by > `/multi-agent:review` and by pipeline Phase 4. ## Why this exists Phase 4 had a size cap and no exclusion list. When the diff exceeds the phase token allowance the cap truncates the LARGEST files first, so a regenerated lockfile or a snapshot dump is not merely wasted budget: it is the thing that survives while real code is cut. The second half is worse and quieter. A reviewer that opened one file of ten and a reviewer that read all ten and found nothing return the identical `{"findings": [], "approved": true}`. Nothing in the pipeline could tell them apart, so "no findings" has been carrying two meanings at once. Both halves are one fix: decide what is worth reading BEFORE the cap decides what fits, and make the reviewer answer for each file it was given. ## The set is fixed before the reviewer sees the diff The same rule `selectedRules[]` follows, for the same reason. After a model has seen the diff, "I did not open that one" and "there was nothing there" become the same sentence, and whichever one is cheaper to say is the one that gets said. So the set is computed from `git diff --name-only`, written to `.pipeline/review-files.json`, and never recomputed inside the round. ```bash git -C "$WORKTREE" diff --name-only "$BASE_BRANCH"...HEAD \ | node $HOME/.claude/scripts/review-file-filter.mjs \ > "$WORKTREE/.pipeline/review-files.json" ``` Report shape: ```json { "reviewed": ["src/App.swift"], "excluded": [ { "path": "package-lock.json", "reason": "lockfile - resolved by the package manager, not written by hand", "pattern": "**/package-lock.json" } ], "total": 2, "patternsSource": ".../schemas/review-file-exclusions.json", "patternCount": 45 } ``` `reviewed` is what goes into the diff cap and into the reviewer prompt, as a `${REVIEW_FILES}` block beside `${CRITERIA}` in the shared cache prefix. It has to be in the prompt: a reviewer asked to account for a set it was never shown can only guess, and `fileCoverage` would then fail on every single dispatch - a gate that always fires is a gate that gets switched off. `excluded` goes into the run report, never into silence. ## Every exclusion names the pattern that produced it A file that disappears between the diff and the review is indistinguishable from a file nobody found anything in - which is the exact confusion this whole feature exists to remove, so reintroducing it in the filter would be self-defeating. Each excluded row carries both the human reason and the glob that matched, so a reviewer, a PR reader or a future maintainer can dispute the call rather than discover it. The pattern list is data, in `schemas/review-file-exclusions.json`, and it is generic: generated trees, lockfiles, recorded snapshots, vendored source, build output, binary assets. No stack, project or company name appears in it. A pattern with no reason invalidates the whole list rather than being defaulted - the default would be exactly the sentence the caller is supposed to print. **It fails open, on purpose.** An unreadable or malformed pattern file yields every file reviewed, the reason on stderr, and exit 2. Failing closed would review nothing and report a clean run. ## The reviewer answers for each file `reviewer-output.schema.json` (v1.3.0) carries `fileCoverage[]`: one row per path in `reviewed`, `{path, verdict: reviewed|skipped, reason}`. There is deliberately no `partial` - a file read in part is read, and what was not understood belongs in a finding. `validate-reviewer.mjs --coverage ` enforces it, catching the same three failures the conformance checklist catches on the rule axis: | Failure | Why it matters | |---|---| | a file in the set with no row | silently unread, and the empty `findings[]` reads as clean | | a row for a path outside the set | an answer about something the reviewer was not given, the same shape as a hallucinated rule ID | | `skipped` with no reason | a drop with no cause is indistinguishable from a read | `skipped` is legitimate and expected: a file past the diff cap, a file whose content the host truncated. What it may not be is unexplained. "Not relevant" is a review decision and belongs in a verdict of `reviewed`, not a skip. An empty `reviewed` set (a diff that is entirely lockfiles) demands no checklist at all. Requiring an empty array there would fail honest output, and the filter's `excluded[]` is what carries that information onward. ## Invocation ```bash node $HOME/.claude/scripts/validate-reviewer.mjs "$REVIEWER_FILE" \ --criteria "$WORKTREE/.pipeline/criteria-manifest.json" \ --coverage "$WORKTREE/.pipeline/review-files.json" ``` Without `--coverage` the field stays optional, so every existing caller keeps working unchanged. With it, the checklist is enforced and exit 1 takes the same single self-correction rework the rest of the validator gate takes. ## What this is not It is not a relevance filter. Nothing here decides that a file is uninteresting; it decides that a file is not human-authored source, which is a mechanical question with a mechanical answer. The moment a pattern starts encoding "we probably do not care about this directory", the list has become a way to hide work, and the near-miss assertions in `smoke-review-file-filter.sh` (`CodeGenerator.swift`, `generated-report.md`, `distribution/`, `buildSrc/`) are what fail when it does.