feat(ce-code-review): configurable model and effort for high-stakes reviewers

.compound-engineering/config.local.example.yaml

@@ -74,6 +74,29 @@
 # plan_model: fable              # model alias for the plan-authoring step (no default)
 # brainstorm_model: fable        # model alias for approach generation (no default)
 
+# Pin the model (and, best-effort, the reasoning effort) for ce-code-review's
+# three high-stakes reviewers only -- correctness-reviewer, security-reviewer,
+# and adversarial-reviewer. Every other persona (mid-tier, ce-doc-review,
+# ce-simplify-code) is untouched. This is an exact pin, not a floor -- once
+# set, it replaces session-model inheritance for the trio even when the
+# session model is more capable, so a high or low choice here is a trade-off
+# the user owns deliberately. Unlike plan_model/brainstorm_model above, this
+# key is read two layers deep and local-first -- the gitignored
+# config.local.yaml wins if set there, otherwise the TRACKED config.yaml
+# supplies a repo-wide default that reaches fresh worktrees too. An explicit
+# in-run model request (e.g. naming a model in your prompt) overrides either
+# layer for that run only. review_effort only takes effect alongside a
+# resolved review_model route, and only on a harness whose dispatch primitive
+# exposes a per-dispatch effort control -- elsewhere it is silently skipped.
+# Invalid, commented, or missing values fall through independently per key --
+# for review_model, straight to today's behavior (trio inherits the session
+# model); for review_effort, it is simply omitted and never affects model
+# resolution, so a valid review_model pin is never discarded because
+# review_effort was absent or bad.
+
+# review_model: opus             # model alias for the trio (no default; exact pin, can lower capability too)
+# review_effort: high            # low | medium | high | xhigh (no default; best-effort, trio-only)
+
 # --- PR concept teaching (ce-commit-push-pr) ---
 # When a PR introduces a concept new to the codebase, the generated PR
 # description gains a "New concepts" section teaching it. The section toggle is

docs/residual-review-findings/nick-review-model-config.md

@@ -0,0 +1,12 @@
+# Residual Review Findings — nick/review-model-config
+
+Source: ce-code-review run 20260731-214329-4ae4d74b (mode:agent, plan docs/plans/2026-07-31-005-feat-review-model-config-plan.md). All findings validated by an independent validation pass; none met the autonomous apply bar (anchor-75, single reviewer), so they are recorded here for resolution during PR review.
+
+## Residual Review Findings
+
+- ~~P1 Stage 3d announce sequencing gap~~ — resolved during PR review: Stage 3d now performs a minimal inline two-key read (Codex review thread raised the same defect; fixed on the PR).
+- **P2** `tests/review-skill-contract.test.ts:409` — the new contract test never pins the dispatch-time Session-model bullet that actually applies `review_model`; reverting that bullet alone would not fail any test. Suggested fix: assert the bullet's override wording directly. (testing, confidence 75)
+- **P3** `tests/review-skill-contract.test.ts:417` — two-layer precedence asserted by presence (`toContain` x2), not order; reversing local-first precedence in prose would not be caught. Suggested fix: one ordered regex. (testing, confidence 75)
+- **P3 (advisory)** `tests/review-skill-contract.test.ts:453` — the trio-order regex is satisfied by the pre-existing line-26 enumeration, so it does not pin the new Config override paragraph. Suggested fix: scope the regex to the new paragraph. (correctness + testing, confidence 75)
+
+Tracker filing: skipped — fork has issues disabled; upstream issues inappropriate for unmerged branch residuals (no_sink).

docs/skills/ce-code-review.md

@@ -90,6 +90,14 @@ This shares the provider/route kernel with `ce-doc-review` (parity-tested in CI)
 
 Large diffs stay on the same single-peer route without being serialized into one enormous prompt. The orchestrator sends a compact semantic review map — intent, material risk divisions, generated-tree treatment, and cross-division interactions — while the worker keeps the exact diff outside the prompt as a private, selectively readable artifact. Deterministic code never invents risk groups or cuts semantic shards; the adversarial agent works within the orchestrator's divisions and narrows its reads again when needed.
 
+### 1c. Configuring the high-stakes reviewer model
+
+By default the three high-stakes reviewers — `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer` — inherit whatever model is running the current session, same as every other persona. Set `review_model` (a model alias, e.g. `opus`) to pin those three, and only those three, to a specific model regardless of the session model. This is an exact pin, not a floor: it can lower capability as well as raise it, so the choice is a trade-off the user owns deliberately.
+
+Unlike `plan_model`/`brainstorm_model`, `review_model` is read two layers deep, local-first: the checkout-local `config.local.yaml` wins if it sets the key, otherwise the **tracked** `config.yaml` supplies a repo-wide default — useful here specifically because fresh worktrees don't inherit the gitignored local file. An explicit in-run model request (naming a model in your prompt, in either direction) overrides either layer for that run only.
+
+`review_effort` (`low | medium | high | xhigh`) refines the pin's reasoning effort, but only when a `review_model` route actually resolved (from config or an in-run request) and only on a harness whose dispatch primitive exposes a per-dispatch effort control — elsewhere it's silently skipped. Fall-through is independent per key: an invalid, commented, or missing `review_model` falls through silently to today's behavior (trio inherits the session model), while an invalid, commented, or missing `review_effort` is simply omitted and never discards a valid `review_model` pin. See the [configuration reference](./configuration.md) for the shared config-layer contract.
+
 ### 2. Severity (P0-P3) and autofix class are orthogonal
 
 Severity answers **urgency** (P0=critical breakage, P3=user discretion). The autofix class is **signal** about follow-up shape (not apply permission):

docs/skills/configuration.md

@@ -8,7 +8,7 @@ Run `/ce-setup` to create or repair the file and its `.gitignore` coverage. The
 
 By default every CE-written artifact folder lives under `docs/` — `docs/plans/`, `docs/solutions/`, and the rest. `docs_root` relocates that root to any repo-relative folder, for projects where `docs/` is already tracked content owned by something else (an Obsidian vault, a docs site). Unset, behavior is byte-identical to today.
 
-`docs_root` reads from two layers, first non-empty wins: the checkout-local `config.local.yaml` above, then a **tracked** `.compound-engineering/config.yaml`. Prefer the tracked file — it is committed, so the setting reaches every clone and worktree of the project; the local file is per-checkout and would have to be re-set in each new worktree. (The tracked `config.yaml` is a general layer; today `docs_root` is its only consumer.)
+`docs_root` reads from two layers, first non-empty wins: the checkout-local `config.local.yaml` above, then a **tracked** `.compound-engineering/config.yaml`. Prefer the tracked file — it is committed, so the setting reaches every clone and worktree of the project; the local file is per-checkout and would have to be re-set in each new worktree. (The tracked `config.yaml` is a general layer; its consumers today are `docs_root` and the `review_model`/`review_effort` keys.)
 
 Two things make `docs_root` unlike the other settings:
 
@@ -40,6 +40,7 @@ All settings are optional. Commented examples are documentation, not active valu
 | [`ce-plan`](./ce-plan.md), [`ce-brainstorm`](./ce-brainstorm.md) | `plan_model`, `brainstorm_model` | Model elevation: send the reasoning-heavy step to a named model (e.g. `fable`, `opus`) instead of the session model. Value is a model alias; a prompt request or an orchestrator's `plan_model:<alias>` carrier (e.g. from `lfg`, honored even in pipeline mode) overrides it. Takes effect on every harness — natively where the host serves the model, else via the Claude CLI, else inline. No default (elevation off). |
 | [`ce-work`](./ce-work.md), [`lfg`](./lfg.md) | `work_engine_mode`, `work_engine_preferences` | Ordered implementation-author preferences. Mode is `off`, `prefer`, or `require`; each entry has a `harness` and optional `model`. See [Implementation routing](#implementation-routing). |
 | [`ce-code-review`](./ce-code-review.md), [`ce-doc-review`](./ce-doc-review.md) | `cross_model_peer` | Preferred cross-model review target: `codex`, `claude`, `grok`, `cursor`, or `composer`. The review skills still apply host-independence and route-availability gates. |
+| [`ce-code-review`](./ce-code-review.md) | `review_model`, `review_effort` | Exact-pin model (and best-effort reasoning effort) for the high-stakes trio only — correctness-reviewer, security-reviewer, adversarial-reviewer; every other persona is unaffected. Read two layers, local-first (`config.local.yaml` then the tracked `config.yaml`, unlike `plan_model`/`brainstorm_model`'s local-only read); an in-run model request overrides both for that run. The pin replaces session-model inheritance even when it lowers capability — the user owns that trade-off. `review_effort` applies only alongside a resolved model route and only where the harness exposes per-dispatch effort; otherwise it is silently skipped. No default (elevation off). |
 | [`ce-commit-push-pr`](./ce-commit-push-pr.md) | `pr_teaching_section`, `pr_teaching_archive`, `auto_babysit` | Toggle PR concept teaching, opt into explainer archival, or opt out of the default babysit handoff. Defaults: `true`, `false`, and `true`. |
 | [`ce-product-pulse`](./ce-product-pulse.md) | `pulse_product_name`, `pulse_lookback_default`, `pulse_primary_event`, `pulse_value_event`, `pulse_completion_events` | Product identity, reporting window, and the events that represent engagement, value, and completion. The setup interview writes these values. |
 | [`ce-product-pulse`](./ce-product-pulse.md) | `pulse_quality_scoring`, `pulse_quality_dimension`, `pulse_analytics_source`, `pulse_tracing_source`, `pulse_payments_source`, `pulse_db_enabled` | Optional quality scoring and read-only data-source routing. |

skills/ce-code-review/SKILL.md

@@ -481,7 +481,9 @@ When a job ID is returned and task tracking is active, add a distinct task that
 
 Do not proceed until the final local roster is materialized. This is a routing boundary, not a preference: a started peer and the in-process adversarial reviewer must never both receive the same review brief.
 
-Announce that final team before spawning, as a user-facing summary: name the always-on reviewers plainly, and for each conditional reviewer give the one-line reason it was added (the real concern, not the keyword that matched). Do **not** put local reviewer model-tier labels (`[session model]`/`[mid-tier]`) or scope-mode codenames in this announce — those are internal. Still decide each local reviewer's tier here and keep it in working state for Stage 4. The cross-model line is separate and follows the receipt-aware model/reasoning and route wording in its reference. This is progress reporting, not a blocking confirmation.
+Before this announce, resolve the config pin directly with a minimal read — do not load `references/dispatch-reviewers.md` yet (Stage 4 below still gates that load): read `review_model` and `review_effort` from `<repo-root>/.compound-engineering/config.local.yaml` first, then `config.yaml` (first non-empty value matching the model-alias shape `^[A-Za-z0-9._-]{1,64}$` wins, `#`-commented lines ignored) — the same two-layer read the `docs_root` block above uses. Use these resolved values both for this stage's tier decision and for the announce line below. `references/dispatch-reviewers.md` (loaded at Stage 4) remains the authoritative statement of precedence and degradation for these keys and consumes the same resolved values; this Stage 3d read only lets the announce state the pin without loading that reference early.
+
+Announce that final team before spawning, as a user-facing summary: name the always-on reviewers plainly, and for each conditional reviewer give the one-line reason it was added (the real concern, not the keyword that matched). Do **not** put local reviewer model-tier labels (`[session model]`/`[mid-tier]`) or scope-mode codenames in this announce — those are internal. When a `review_model` config pin is active for the high-stakes trio, include one line naming the configured model (and effort, when applied) as *requested* for those reviewers. Still decide each local reviewer's tier here and keep it in working state for Stage 4. The cross-model line is separate and follows the receipt-aware model/reasoning and route wording in its reference. This is progress reporting, not a blocking confirmation.
 
 ### Stage 4: Dispatch and collect reviewers
 

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

@@ -23,10 +23,12 @@ When Stage 3c selected the lite roster, the fast pass still runs.
 
 #### Model tiering
 
-Three reviewers inherit the session model with no override: `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer`. These perform the highest-stakes analysis — logic bugs, security vulnerabilities, adversarial failure scenarios — and should run at whatever capability level the user has configured. If the user is on Opus, these get Opus.
+Three reviewers inherit the session model with no override by default: `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer`. These perform the highest-stakes analysis — logic bugs, security vulnerabilities, adversarial failure scenarios — and should run at whatever capability level the user has configured. If the user is on Opus, these get Opus.
 
 All other persona subagents and CE local prompt assets use the platform's mid-tier model to reduce cost and latency. See the Spawning subsection below for the exact dispatch-time override.
 
+**Config override.** Read `review_model` and `review_effort` the same two-layer local-first way `docs_root` is read: `<repo-root>/.compound-engineering/config.local.yaml` first, then `config.yaml` (`<repo-root>` = `git rev-parse --show-toplevel`); the first non-empty value per key wins; `#`-commented lines are ignored. A resolved `review_model` must additionally match the model-alias shape `^[A-Za-z0-9._-]{1,64}$` (the same grammar the plan_model elevation carrier uses) before it is treated as a pin; a value that fails this shape check is treated as unset. This is a deterministic pre-bind check that catches malformed values (empty, whitespace, path-like, oversized) — it does not and cannot validate that a well-formed alias actually names a real, capable model; that is caught only by the rejected-selector fallback below. Fall-through is independent per key — an invalid or missing `review_model` falls through silently to session-model inheritance, exactly like a typo'd alias that is never passed through as a pin; an invalid or missing `review_effort` never touches model resolution at all, it only means effort is omitted — a valid `review_model` pin is never discarded because `review_effort` was absent or bad. A resolved `review_model` replaces session-model inheritance for `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer` only — an exact pin, applied even when the session model is more capable than the configured one. An explicit in-run request naming a model for the reviewers outranks these keys in either direction (elevate or lower) for that run. `review_effort` takes effect only when an explicit model route resolved for the trio (via `review_model` or an in-run request) and only on a harness whose dispatch primitive exposes a per-dispatch effort override; otherwise it is omitted silently — degrade the outcome, never the boundary. Once a model pin is configured it is never *silently* dropped: on a harness whose dispatch primitive exposes no model selector at all, the pin is dropped with disclosure (see the Spawning subsection's unsupported-host rule) rather than silently, and the effort refinement on top of a resolved pin can still be omitted silently.
+
 The orchestrator (this skill) also inherits the session model; it handles intent discovery, reviewer selection, finding merge/dedup, and synthesis.
 
 #### Run ID
@@ -39,9 +41,9 @@ Use the run ID and absolute run dir already created at the Stage 3d routing boun
 
 Omit the `mode` parameter when dispatching sub-agents so the user's configured permission settings apply. Do not pass `mode: "auto"`.
 
-**Model override at dispatch time — this is a correctness guarantee, not cosmetics.** Omitting the override on a top-tier parent session (e.g. Opus) silently runs that reviewer at the expensive tier — the regression this prevents. The tier is a deterministic function of the persona, so as you select reviewers in Stage 3, **record each reviewer's tier in an internal working list** — that list is your external memory (the role the old printed `[session model]`/`[mid-tier]` labels served) and it must exist and be honored even though it is no longer rendered in the user-facing announce:
+**Model override at dispatch time — this is a correctness guarantee, not cosmetics.** Omitting the override on a top-tier parent session (e.g. Opus) silently runs that reviewer at the expensive tier — the regression this prevents. The tier is a deterministic function of the persona and the resolved `review_model` config, so as you select reviewers in Stage 3, **record each reviewer's tier in an internal working list** — that list is your external memory (the role the old printed `[session model]`/`[mid-tier]` labels served) and it must exist and be honored even though it is no longer rendered in the user-facing announce:
 
-- **Session model** (no override; inherits the session model) — `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer` only.
+- **Session model** (inherits the session model, unless the Model tiering `review_model` config resolved above — then pass that model instead, plus `review_effort` on a harness whose dispatch primitive exposes a per-dispatch effort override, omitting effort silently where it does not) — `correctness-reviewer`, `security-reviewer`, and `adversarial-reviewer` only. **Unsupported-host degradation:** when the active dispatch primitive exposes no model or custom-agent selector at all (e.g. Codex task wording, which cannot carry a model pin), passing the resolved `review_model` is impossible — mirror the Mid-tier rule below: omit the override, let the reviewer inherit the session model, and disclose the unapplied pin by name in Coverage/the announce. This is the pin dropped *with disclosure*, not the silent drop the Model tiering paragraph rules out. **Rejected-selector fallback:** the shape check above only catches malformed values, not a well-formed but wrong alias (e.g. `opuz`) that a selector-capable host rejects at spawn time. If dispatch fails with an unrecognized-alias/unknown-model error (not a capacity error — see the backpressure rule above), re-dispatch that reviewer once with the override omitted so it inherits the session model, and disclose the unapplied pin by name in Coverage. A rejected pin must never cost the reviewer its dispatch.
 - **Mid-tier** — every other persona and CE agent: pass the platform's balanced mid-tier model. In Claude Code, that is the Sonnet class. In Codex, apply this tier only when the active dispatch primitive exposes an explicit model or custom-agent selector; task wording alone does not select a different model. Otherwise omit the override and inherit the parent model — a working review on the parent model beats a broken dispatch on an unrecognized name.
 
 Apply this on **every** Agent / `spawn_agent` / subagent call. A missed override is a silent cost-and-quality regression, so treat the internal tier list as load-bearing — moving it out of the user-facing output removed the *display*, not the discipline.