fix(ce-code-review): take review criteria from a repo-owned standards file (#1572)

Author: tmchow

AGENTS.md

@@ -155,7 +155,7 @@ When a skill needs to discover a project convention at runtime — the issue tra
 **Name a concrete file only where the skill must do something a context reference can't express:**
 
 - **Writing a convention back** (e.g., persisting `project_tracker: linear`) needs a target — name it minimally and as an example ("the project's root agent-instructions file, e.g., `AGENTS.md`; if it `@`-includes another, write to the substantive one").
-- **Reading content that is genuinely not auto-loaded** — a subdirectory-scoped instruction file governing the area being changed, an optional project doc like `STRATEGY.md` / `CONCEPTS.md` / `README.md`, or any file a *fresh subagent* (which does not inherit the parent's loaded instructions) must open to do its job. Auditing tools that must enumerate every standards file (e.g., `ce-code-review`'s project-standards reviewer globbing all `CLAUDE.md`/`AGENTS.md`) are a legitimate exception — they review the files, they don't re-read them for context.
+- **Reading content that is genuinely not auto-loaded** — a subdirectory-scoped instruction file governing the area being changed, an optional project doc like `STRATEGY.md` / `CONCEPTS.md` / `README.md`, or any file a *fresh subagent* (which does not inherit the parent's loaded instructions) must open to do its job. Auditing tools that must enumerate every criteria file are a legitimate exception — they review the files, they don't re-read them for context. `ce-code-review`'s project-standards reviewer globs `CODING_STANDARDS.md`, the designated criteria source, and reads `CLAUDE.md`/`AGENTS.md` only as criteria for changed files that no `CODING_STANDARDS.md` governs.
 
 **Describe the capability, not the tool.** Pair this with naming the *category* of thing rather than a closed set: "the project's issue tracker (e.g., GitHub Issues, Linear, Jira)" and "whatever interface that tracker exposes (connector/MCP, documented API, or a documented CLI)" — never assume a specific CLI exists, and never treat a missing binary / env var / MCP server as proof the capability is unavailable.
 

docs/plans/ce-code-review-reviewer-consolidation-and-standards-file.md

@@ -14,7 +14,7 @@ The scope step in the SKILL.md handles discovery and passes you the resolved dif
 
 ## Remote scope (`pr-remote` and `branch-remote`)
 
-When the review context includes `<pr-scope-mode>pr-remote</pr-scope-mode>` or `<pr-scope-mode>branch-remote</pr-scope-mode>`, the working tree is **not** the reviewed head. Do **not** use Read/Grep on workspace paths for files in the changed-file list — they may not match the branch or PR under review.
+When the review context includes `<pr-scope-mode>pr-remote</pr-scope-mode>` or `<pr-scope-mode>branch-remote</pr-scope-mode>`, the working tree is **not** the reviewed head. Do **not** use Read/Grep on workspace paths for any file that belongs to the reviewed tree — the changed-file list, and any other file the review points you at, such as a criteria file. They may not match the branch or PR under review, and an unchanged file is not exempt: the checkout can hold a version the reviewed head never had.
 
 Instead:
 

skills/ce-code-review/references/diff-scope.md

@@ -68,7 +68,7 @@ For each selected reviewer, and only for those, read the corresponding local pro
 4. PR metadata: title, body, and URL when reviewing a PR (empty string otherwise). Passed in a `<pr-context>` block so reviewers can verify code against stated intent
 5. Review context: intent summary, file list, diff, scope mode (`local-aligned` | `pr-remote` | `branch-remote`), and remote head ref (`PR_HEAD_REF` or `<branch-head-ref>`) when set
 6. Run ID and reviewer name for the artifact file path
-7. **For selected `project-standards` only:** the non-empty standards file path list from Stage 3b, wrapped in a `<standards-paths>` block appended to the review context
+7. **For selected `project-standards` only:** the non-empty Stage 3b criteria mapping — each criteria file with the changed files it governs — wrapped in a `<standards-paths>` block appended to the review context
 8. **For `data-migration` only:** the resolved review base ref from Stage 1 (`BASE:` marker), wrapped in `<review-base>` inside the review context so schema drift checks never assume `main`
 
 Persona sub-agents are **read-only** with respect to the project: they review and return structured JSON. They do not edit project files or propose refactors. The one permitted write is saving their full analysis to the resolved run-artifact path specified in the output contract.

skills/ce-code-review/references/dispatch-reviewers.md

@@ -11,7 +11,7 @@ Correctness is spawned on every multi-agent review. Project-standards is spawned
 | Persona | Prompt asset | Focus |
 |---------|-------|-------|
 | `correctness` | `correctness-reviewer` | Logic errors, edge cases, state bugs, error propagation, intent compliance |
-| `project-standards` | `project-standards-reviewer` | CLAUDE.md and AGENTS.md compliance -- frontmatter, references, naming, cross-platform portability, tool selection |
+| `project-standards` | `project-standards-reviewer` | Compliance with the project's designated criteria files, cited rule by rule |
 
 ## Generic conditional
 

skills/ce-code-review/references/persona-catalog.md

@@ -1,21 +1,23 @@
 # Project Standards Reviewer
 
-You audit code changes against the project's own standards files -- CLAUDE.md, AGENTS.md, and any directory-scoped equivalents. Your job is to catch violations of rules the project has explicitly written down, not to invent new rules or apply generic best practices. Every finding you report must cite a specific rule from a specific standards file.
+You audit code changes against the criteria files the project has designated, at the paths you are given. Your job is to catch violations of rules the project has explicitly written down, not to invent new rules or apply generic best practices. Every finding you report must cite a specific rule from a specific standards file.
 
 ## Standards discovery
 
-The orchestrator passes a `<standards-paths>` block listing the file paths of all relevant CLAUDE.md and AGENTS.md files. These include root-level files plus any found in ancestor directories of changed files (a standards file in a parent directory governs everything below it). Read those files to obtain the review criteria.
+**Judge each changed file only against the criteria paired with it.** No changed file is ever graded against two kinds of criteria file, so a rule from a criteria file that does not govern a path is not a finding against that path.
 
-If no `<standards-paths>` block is present (standalone usage), discover the paths yourself:
+The orchestrator passes a `<standards-paths>` block pairing each criteria file with the changed files it governs. Read those files and apply that pairing as given.
 
-1. Use the native file-search/glob tool to find all `CLAUDE.md` and `AGENTS.md` files in the repository.
-2. For each changed file, check its ancestor directories up to the repo root for standards files. A file like root `AGENTS.md` applies to the whole checkout, while `skills/AGENTS.md` applies to all changes under `skills/`.
-3. Read each relevant standards file found.
+If no `<standards-paths>` block is present (standalone usage), build the same pairing yourself. Find every `CODING_STANDARDS.md`, `CLAUDE.md`, and `AGENTS.md` in the repository and keep those whose directory is an ancestor of a changed file — a root-level file governs the whole checkout, `skills/AGENTS.md` only what is under `skills/`. `CODING_STANDARDS.md` is the designated criteria source, so an instruction file supplies criteria only for changed files that no `CODING_STANDARDS.md` governs.
 
-In either case, identify which sections apply to the file types in the diff. A skill compliance checklist does not apply to a TypeScript converter change. A commit convention section does not apply to a markdown content change. Match rules to the files they govern.
+**The content is the contract, not the format.** A criteria file may be written by a person or by another tool, so expect any shape: prose, bullets, tables, nested headings, with or without frontmatter. Extract the rules whatever the shape. Never require a schema, an identifier, or a section layout, and never report a formatting choice as a finding.
+
+Within each criteria file you read, identify which sections apply to the file types in the diff. A skill compliance checklist does not apply to a TypeScript converter change. A commit convention section does not apply to a markdown content change. Match sections to the file types they address.
 
 ## What you're hunting for
 
+The shapes below are examples of how a written rule gets violated, drawn from an agent-skills repository. They are not the criteria. The criteria are whatever the discovered files state, so a repository whose rules cover none of these shapes is reviewed against its own rules and not against this list.
+
 - **YAML frontmatter violations** -- missing required fields (`name`, `description`), description values that don't follow the stated format ("what it does and when to use it"), names that don't match directory names. The standards files define what frontmatter must contain; check each changed skill or agent file against those requirements.
 
 - **Reference file inclusion mistakes** -- markdown links to skill-local reference files used where the standards require backtick paths or `@` inline inclusion. Backtick paths used for files the standards say should be `@`-inlined (small structural files under ~150 lines). `@` includes used for files the standards say should be backtick paths (large files, executable scripts). The standards file specifies which mode to use and why; cite the relevant rule.
@@ -50,7 +52,7 @@ Use the anchored confidence rubric in the subagent template. Persona-specific gu
 - **Violations that automated checks already catch.** If `bun test` validates YAML strict parsing, or a linter enforces formatting, skip it. Focus on semantic compliance that tools miss.
 - **Pre-existing violations in unchanged code.** If an existing SKILL.md already uses markdown links for references but the diff didn't touch those lines, mark it `pre_existing`. Only flag it as primary if the diff introduces or modifies the violation.
 - **Generic best practices not in any standards file.** You review against the project's written rules, not industry conventions. If the standards files don't mention it, you don't flag it.
-- **Opinions on the quality of the standards themselves.** The standards files are your criteria, not your review target. Do not suggest improvements to CLAUDE.md or AGENTS.md content.
+- **Opinions on the quality of the criteria themselves.** The criteria files are what you review against, not what you review. Do not suggest improvements to their content.
 
 ## Evidence requirements
 

skills/ce-code-review/references/personas/project-standards-reviewer.md

@@ -64,16 +64,19 @@ For `deployment-verification-agent`, use the same migration-artifact gate when t
 
 ### Stage 3b: Discover project standards paths
 
-Before spawning sub-agents, find the file paths (not contents) of all relevant standards files for the `project-standards` persona. Use the native file-search/glob tool to locate:
+**Goal:** the mapping that pairs each criteria file governing this change with the changed files it governs, for the `project-standards` persona. Paths, not contents.
 
-1. Use the native file-search tool (e.g., Glob in Claude Code) to find all `**/CLAUDE.md` and `**/AGENTS.md` in the repo.
-2. Filter to those whose directory is an ancestor of at least one changed file. A standards file governs all files below it (e.g., `AGENTS.md` at the repo root applies to the whole checkout, while `skills/AGENTS.md` would apply to everything under `skills/`).
+Enumerate the candidates from **the tree under review**, never from whichever tree happens to be checked out: the workspace only in `local-aligned` scope, and the reviewed head ref in `pr-remote` and `branch-remote` (Stage 1 resolved which). A criteria file that exists only in the reviewed tree must appear, and one deleted there must not, or the persona enforces criteria the change never had.
 
-Distinguish an empty successful search from a failed or unavailable search:
+Candidates are `CODING_STANDARDS.md`, `CLAUDE.md`, and `AGENTS.md` at any depth. Keep those whose directory is an ancestor of a changed file — a root-level file governs the whole checkout, `skills/AGENTS.md` only what is under `skills/`.
 
-- One or more applicable paths: select `project-standards` and pass the path list inside a `<standards-paths>` block in its Stage 4 context. The persona reads the files itself, targeting only relevant sections.
+`CODING_STANDARDS.md` is the designated criteria source, so an instruction file supplies criteria only for changed files that no `CODING_STANDARDS.md` governs, and no file is graded against both kinds. Every governing `CODING_STANDARDS.md` still applies together. When the instruction-file fallback supplied the criteria for any changed file, name it as the fallback in Coverage.
+
+**Done** when no changed file could be graded against two kinds of criteria. A changed file that no criteria file governs is a complete result, not a gap. **On uncertainty, fail closed** — an error is never an empty result:
+
+- One or more applicable paths: select `project-standards` and pass the mapping inside a `<standards-paths>` block in its Stage 4 context. The persona applies the precedence you resolved rather than re-deriving it, and reads the files itself, targeting only relevant sections.
 - Empty successful search: do not dispatch `project-standards`; record `project standards: not run (no applicable standards files)` in Coverage.
-- Search failure or uncertain scope: fail closed by dispatching `project-standards` with the uncertainty stated; never treat an error as an empty result.
+- Search failure or uncertain scope: dispatch `project-standards` with the uncertainty stated.
 
 ### Stage 3c: Small-diff fast path (reduce the roster for trivial, low-risk diffs)
 

skills/ce-code-review/references/select-and-route.md

@@ -133,7 +133,7 @@ False-positive categories to actively suppress. Do NOT emit a finding when any o
 - **Suggestions that restate what the code already does in different words.** "Consider extracting this into a helper" when the code is already a small helper, "consider adding a guard" when a guard one line up already enforces it.
 - **Generic "consider adding" advice without a concrete failure mode.** If you cannot name what breaks, the finding is not actionable. Either find the failure mode or suppress.
 - **Issues with a relevant lint-ignore comment.** Code that carries an explicit lint disable comment for the rule you are about to flag (`eslint-disable-next-line no-unused-vars`, `# rubocop:disable Style/StringLiterals`, `# noqa: E501`, etc.) — suppress unless the suppression itself violates a project-standards rule that explicitly forbids disabling that lint for this code shape. The author already chose to suppress; re-flagging it via a different reviewer creates noise and ignores their decision.
-- **General code-quality concerns not codified in CLAUDE.md / AGENTS.md.** "This file is getting long," "this method has too many parameters," "this is hard to read" — without a project-standards rule to anchor the concern, these are subjective and waste reviewer time. If the project explicitly bans long files or sets a parameter-count limit in its standards, that is a project-standards finding; otherwise suppress.
+- **General code-quality concerns with no rule behind them.** "This file is getting long," "this method has too many parameters," "this is hard to read" — without a rule from one of the criteria files this review designated to anchor the concern, these are subjective and waste reviewer time. When a criteria file does state the limit, that is a project-standards finding; otherwise suppress.
 - **Speculative future-work concerns with no current signal.** "This might break under load," "what if the requirements change," "this could be hard to test later" — not findings unless the diff introduces concrete evidence the concern is reachable now.
 
 **Advisory observations — route to advisory autofix_class, do not force a decision.** If the honest answer to "what actually breaks if we do not fix this?" is "nothing breaks, but…", the finding is advisory. Set `autofix_class: advisory` and `confidence: 50` so synthesis routes the finding to a soft bucket rather than surfacing it as a primary action item. Do not suppress — the observation may have value; it just does not warrant user judgment. Typical advisory shapes: design asymmetry the PR improves but does not fully resolve, opportunity to consolidate two similar helpers when neither is broken, residual risk worth noting in the report.