code-review: make DOMAIN_CRITIC_CAP operator-tunable

CHANGELOG.md

@@ -4,6 +4,16 @@ All notable changes to the claude-plugins project will be documented in this fil
 
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). Entries are listed newest-first; each plugin section is treated as released when merged to `main`.
 
+### code-review v3.14.0
+
+#### Added
+- `.closedloop-ai/settings/code-review.json` accepts `domain_critic_cap`, the number of domain critics that may spawn across the required and best-effort buckets combined (default 3; `source: "core"` reviewers stay exempt). `0` is a kill switch; invalid entries fall back to the default. `arbitrate-budget --domain-critic-cap` overrides it per run, and takes precedence over the settings file.
+
+#### Fixed
+- On a PR review, `domain_critic_cap` is read from the settings file as committed on `origin/<base_ref>` rather than from the checked-out head. The reviewed tree is contributor-authored, so a head-only copy could set the budget deciding which reviewers inspect that same diff — `domain_critic_cap: 0` suppressed every rule and critic reviewer, and because cap drops emit no coverage gap, nothing downstream reported the absent coverage. A cap the operator committed on the base branch still applies; one present only on the head is ignored, and off a PR the working tree is read as before. `bha_unified_threshold_loc` and `out_of_hunk_confidence_floor` are still read from the working tree on every run.
+- `domain_critic_cap` is clamped to the budget headroom left after BHA partitions and the non-critic required core are reserved. Above that headroom the surplus critics were selected and then dropped as `budget_exceeded` coverage gaps carrying `required: True`, which canonical verdict Rule 1 turns into CHANGES_REQUESTED — so raising the cap flipped the verdict on a run that would otherwise have been APPROVED. The surplus now defers under `defer_reason: "domain_critic_cap"`, and `budget.domain_critic_cap` reports the cap that actually governed selection.
+- `README.md` and `commands/start.md` no longer state that required critics dropped by the domain-critic cap emit coverage-gap findings. They have not since cap drops began reporting only through `deferred_for_budget`, so an operator lowering the cap was pointed at a safety signal that never appears. Both now also record the cap's effective ceiling and the base-ref trust anchor.
+
 ### code-review v3.13.0
 
 #### Fixed

plugins/code-review/.claude-plugin/plugin.json

@@ -1,7 +1,7 @@
 {
   "name": "code-review",
   "description": "Code review plugin",
-  "version": "3.13.0",
+  "version": "3.14.0",
   "author": {
     "name": "ClosedLoop",
     "email": "support@closedloop.ai"

plugins/code-review/.codex-plugin/plugin.json

@@ -1,6 +1,6 @@
 {
   "name": "code-review",
-  "version": "3.13.0",
+  "version": "3.14.0",
   "description": "Code review plugin for automated review workflows and inline GitHub PR comments.",
   "author": {
     "name": "ClosedLoop",

plugins/code-review/README.md

@@ -194,7 +194,7 @@ Three tiers select which reviewer fleet runs:
 | `design_critic` (always-on at deep) | ✗ | ✗ | ✓ |
 | `impact_analyzer` (FEA-1401, on exported-symbol change/deletion or a deleted file) | ✗ | ✗ | ✓ (on signal or file deletion) |
 
-**Standard-mode budget arithmetic.** With PLN-807 Phase 4, `arbitrate-budget` reserves BHA partitions FIRST (from `_max_bha_partitions_by_loc`) and then allocates the remaining budget to critics and best-effort. The total domain-critic count across both required and best-effort buckets is capped uniformly at `DOMAIN_CRITIC_CAP = 3` (by priority asc, reviewer asc) for both standard and deep tiers. Required-bucket critics dropped by the cap emit coverage-gap findings; cap-deferred entries carry `defer_reason: "domain_critic_cap"` in `deferred_for_budget`. PRs with sparse critic-gates rosters see identical fleet to pre-PLN-807.
+**Standard-mode budget arithmetic.** With PLN-807 Phase 4, `arbitrate-budget` reserves BHA partitions FIRST (from `_max_bha_partitions_by_loc`) and then allocates the remaining budget to critics and best-effort. The total domain-critic count across both required and best-effort buckets is capped uniformly at `DOMAIN_CRITIC_CAP` (default 3, operator-tunable via `.closedloop-ai/settings/code-review.json:domain_critic_cap`; by priority asc, reviewer asc) for both standard and deep tiers. Cap-deferred entries — from either bucket — carry `defer_reason: "domain_critic_cap"` in `deferred_for_budget` and do **not** emit coverage-gap findings: a required gap carries `required: True`, which `_compute_canonical_verdict` Rule 1 turns into CHANGES_REQUESTED, so emitting one would block every repo whose `critic-gates.json` resolves more required critics than the cap. Lowering the cap therefore raises no safety signal — `deferred_for_budget` is the only record of what it dropped. PRs with sparse critic-gates rosters see identical fleet to pre-PLN-807.
 
 **Tier-mismatch nudge.** Shallow runs emit a single LOW system-scoped finding (`system_marker: "tier_mismatch_nudge"`) when the diff would benefit from a higher tier. Heuristics: diff > 3000 LOC; schema/migration paths (`/migrations/`, `/schemas/`, `/models/`); public API surface (`plugin.json`, `index.ts`, `__init__.py`, etc.).
 
@@ -310,11 +310,19 @@ Sensitive-path policy. See `start.md` for the full glob syntax and the three sup
 
 ### `code-review.json` (PLN-774)
 
-Operator-tunable reviewer behavior. Currently exposes the BHA conditional-partitioning threshold; future knobs will land here too.
+Operator-tunable reviewer behavior.
 
 | Key | Default | Behavior |
 |---|---|---|
 | `bha_unified_threshold_loc` | `5000` | PRs with total changed LOC at or below this value get a single "unified" BHA partition so cross-region invariants (declaration ↔ enforcement, definition ↔ reference) stay visible to one reviewer's context. PRs above the threshold fall back to the standard bin-pack (`REBALANCE_LOC_BUDGET=1200` LOC per partition). **Setting the value to `0` disables unified mode entirely (always-partition; restores pre-PLN-774 behavior — the regression escape hatch).** Invalid entries (wrong type, negative) silently fall back to the default. |
+| `out_of_hunk_confidence_floor` | `0.80` | P2+ findings whose line falls outside the file's changed range survive validation when `confidence >` this floor. `1.0` is a kill switch (strict in-hunk only); `0.0` admits every out-of-hunk P2+. |
+| `domain_critic_cap` | `3` | How many domain critics may spawn across the required and best-effort buckets combined. `source: "core"` reviewers (Design Critic, Impact Analyzer) are exempt. **Setting the value to `0` spawns no domain critics at all (kill switch).** Invalid entries (wrong type, negative) silently fall back to the default. Also overridable per-run with `arbitrate-budget --domain-critic-cap`. |
+
+**When to raise `domain_critic_cap`.** The cap drops by `(priority asc, reviewer asc)`. A repo whose `critic-gates.json` still uses the legacy `moduleCritics[]` schema gets every entry migrated as `required: False` with **no** `priority`, so all of them sit at the default `2` and the tiebreak degenerates to **alphabetical by reviewer name** — systematically favoring early names over relevance, and cutting the very critic the coverage critic proposed *for* the diff. Raising the cap is the blunt remedy; the precise one is migrating the relevant rules to the canonical `coverage[]` schema, which supports explicit `priority` and `required` (see `_migrate_module_critics` for what the legacy form forces).
+
+**The useful range tops out at the total `--cap` headroom.** The cap is a soft budget inside `--cap` (default 20), which reserves BHA partitions and the non-critic required core first, so the most critics that can actually spawn is roughly `--cap − bha_partitions − required core`. A value above that is clamped to the headroom rather than honored: the surplus defers under `defer_reason: "domain_critic_cap"`, and `budget.domain_critic_cap` in `coverage.json.final` reads back as the clamped value, not the one requested. Note that `--cap` itself is not exposed in this settings file, so raising the headroom is a per-run flag, not a setting.
+
+**On a PR review this setting is read from `origin/<base_ref>`, not from the checked-out head.** The tree under review is contributor-authored, so honoring a head copy would let the author of a diff set the budget that decides which reviewers inspect it — `domain_critic_cap: 0` suppresses every rule/critic reviewer, and because cap drops emit no coverage gap (see above) nothing downstream reports the missing coverage. A value the operator has committed on the base branch still applies; one that exists only on the head is ignored. Off a PR the working tree is the operator's own checkout and is read directly. The same anchoring is **not** yet applied to `bha_unified_threshold_loc` or `out_of_hunk_confidence_floor`, which are still read from the working tree on every run.
 
 The chosen mode + count surface in `partitions.json` (`partition_mode`, `partition_count`, `total_changed_loc`, `unified_threshold_loc`), propagate into `verify_manifest.json`, and render in both presenters (local-mode Verifier Stats footer and GitHub Step 6e). Under partitioned mode, `stats.verification.by_reviewer` splits BHA findings per partition (`bha_p0`, `bha_p1`, …) so an over-rejecting partition surfaces in the FP-rate column.
 

plugins/code-review/commands/start.md

@@ -32,7 +32,7 @@ Run a multi-agent code review with partitioned deep review, deterministic hygien
 The `--depth` flag selects which reviewer fleet runs. Default `standard`. Bare `/start` invocations preserve historical behavior.
 
 - **shallow** — hygiene + BHA (partitioned at >5000 LOC) + BHB + unified_auditor + verifier. Skips signal extraction, coverage planning/critic, and all `critic-gates.json` entries. Static spawn spec; no routing/critic decisions. Hygiene emits a `tier_mismatch_nudge` MEDIUM finding (category `Coverage`) when the PR's diff size, schema/migration paths, or public API surface suggest standard would catch more.
-- **standard** — current behavior. Full fleet with signal-driven routing, coverage critic, repo-specific critic activation via `critic-gates.json`. Budget arithmetic reserves BHA partitions FIRST (Phase 4) and caps total domain critics across both required and best-effort buckets at the tier-uniform `DOMAIN_CRITIC_CAP = 3` (standard and deep alike). Required critics dropped by the cap emit coverage-gap findings.
+- **standard** — current behavior. Full fleet with signal-driven routing, coverage critic, repo-specific critic activation via `critic-gates.json`. Budget arithmetic reserves BHA partitions FIRST (Phase 4) and caps total domain critics across both required and best-effort buckets at the tier-uniform `DOMAIN_CRITIC_CAP` (default 3, standard and deep alike; operator-tunable via `.closedloop-ai/settings/code-review.json:domain_critic_cap`, or per-run with `arbitrate-budget --domain-critic-cap`). Critics dropped by the cap land in `deferred_for_budget` with `defer_reason: "domain_critic_cap"` and emit no coverage-gap finding, so lowering the cap is silent by design. On a PR review the cap is read from `origin/<base_ref>`, not the checked-out head, so the diff under review cannot set the budget that decides which reviewers inspect it.
 - **deep** — standard plus two deep-only conditional core reviewers. The always-on **Design Critic** runs on every deep review (no trigger): a software-design craftsmanship reviewer covering module depth/information hiding, SOLID, dependency direction and layer boundaries, and project structure (drawing on *A Philosophy of Software Design*, SOLID, and *Clean Architecture*); it is `source: "core"` so it is exempt from `DOMAIN_CRITIC_CAP`, runs on Sonnet, is code-intelligence-aware (uses whichever indexing MCP server the operator's session provides for module layout and dependency direction, else grep), and emits `category: "Code Quality"` findings scoped to design flaws this change introduces or worsens. The signal-gated **Impact Analyzer** (FEA-1401), a cross-file blast-radius reviewer, runs when signal extraction detects `exported_symbol_change` or `symbol_deletion`, or when the diff deletes a file outright (`file_deletion`; at standard and shallow depth Bug Hunter B and the fast-path reviewer own deletions). The analyzer identifies changed exported symbols, finds external usages outside the diff (via whichever indexing MCP server the operator's session provides, else grep), and emits findings with `external_impact[]` listing every callsite that breaks under the new signature. Cost-capped at 30 symbols × 50 callsites with a 5-minute wall budget; deferred symbols surface in the Coverage Plan footer. Impact findings carry `category: "ImpactAnalysis"` and are verifier-audited per-entry (cited callsites read, snippet content-matched, grep replayed). ≥2 verified BLOCKING/HIGH Impact findings escalate the verdict to `NEEDS_ATTENTION` (Rule 6). Deep's extra breadth comes from these two reviewers rather than a wider domain-critic cap.
 
 Tier transitions are detected via `review_state.json`: a cached `shallow` review does not satisfy a subsequent `standard` invocation — the deeper run actually executes the previously skipped reviewers.
@@ -352,7 +352,7 @@ These conditions mirror the cache-update gate (Gate C) and the pre-Phase-4b "Rev
 
 ## Operator Settings
 
-Two optional operator-authored config files live under `.closedloop-ai/settings/`. Both are absent by default; the pipeline uses built-in defaults until they exist.
+Three optional operator-authored config files live under `.closedloop-ai/settings/`. All are absent by default; the pipeline uses built-in defaults until they exist.
 
 ### `.closedloop-ai/settings/verdict-thresholds.json` (FEA-1401)