Reviewer roster reshape: drop reviewer_code fan-out, promote lens reviewers to CRITICAL (#2139)

docs/architecture/orchestrator.md

@@ -285,12 +285,13 @@ The orchestrator coordinates specialized agent roles across pipeline phases. Eac
 | **Coder** | Write code, create commits, push branches |
 | **Tester** | Find gaps in implementation, write and run tests, run linters/type checkers, apply auto-fixes |
 | **Documenter** | Update docs and READMEs |
-| **Reviewer (Code)** | Security, correctness, code quality, testing, documentation. On large diffs (~10 changed files OR ~500 LOC) self-gates and partitions the change set across Claude Agent SDK `Task` subagents along the implement-phase task list ([#1965](https://github.com/jwbron/egg/issues/1965)); see [Concurrent Execution: Implement-phase `reviewer_code` Subagent Fan-Out](../guides/concurrent-execution.md#implement-phase-reviewer_code-subagent-fan-out). |
+| **Reviewer (Code)** | Security, correctness, code quality, testing, documentation. Reviews every changed file systematically and emits a single CRITICAL ACK / NACK on the full diff. |
+| **Reviewer (Code Holistic)** | Single-pass cross-module coherence review ([#2126](https://github.com/jwbron/egg/issues/2126)) — runs alongside Reviewer (Code) and gates consensus independently on architectural-coherence findings. |
 | **Reviewer (Contract)** | Verify acceptance criteria met, task completion status |
-| **Reviewer (Security)** _(ADVISORY)_ | Security-lens review focused on cross-file allowlist mismatches, handler-vs-validator path mismatches, uncommitted-artifact / Dockerfile-symlink mismatches, secret leakage, and cross-file OWASP top-10 patterns. Criteria: [`shared/prompts/security-review-criteria.md`](../../shared/prompts/security-review-criteria.md). NACKs are recorded but do not deadlock consensus until [#1997](https://github.com/jwbron/egg/issues/1997)'s severity-tagged NACK signalling lands. |
-| **Reviewer (Concurrency)** _(ADVISORY)_ | Concurrency-lens review focused on race conditions, deadlocks, shared-state mutation, retry storms, resource-cleanup ordering, and BRC-protocol invariants. Criteria: [`shared/prompts/concurrency-review-criteria.md`](../../shared/prompts/concurrency-review-criteria.md). Same ADVISORY semantics as Reviewer (Security). |
+| **Reviewer (Security)** | Security-lens review focused on cross-file allowlist mismatches, handler-vs-validator path mismatches, uncommitted-artifact / Dockerfile-symlink mismatches, secret leakage, and cross-file OWASP top-10 patterns. Criteria: [`shared/prompts/security-review-criteria.md`](../../shared/prompts/security-review-criteria.md). CRITICAL — a NACK blocks consensus ([#2139](https://github.com/jwbron/egg/issues/2139)). |
+| **Reviewer (Concurrency)** | Concurrency-lens review focused on race conditions, deadlocks, shared-state mutation, retry storms, resource-cleanup ordering, and BRC-protocol invariants. Criteria: [`shared/prompts/concurrency-review-criteria.md`](../../shared/prompts/concurrency-review-criteria.md). CRITICAL — same as Reviewer (Security) ([#2139](https://github.com/jwbron/egg/issues/2139)). |
 
-**Execution model**: All implement phase agents run concurrently via the BRC consensus protocol. Agents communicate via the orchestrator message bus and reach phase completion through peer consensus. The two ADVISORY lens reviewers run alongside the critical reviewers on the same change set; promotion from ADVISORY to CRITICAL is gated on [#1997](https://github.com/jwbron/egg/issues/1997).
+**Execution model**: All implement phase agents run concurrently via the BRC consensus protocol. Agents communicate via the orchestrator message bus and reach phase completion through peer consensus.
 
 ### Prompt Context Scoping
 

docs/guides/concurrent-execution.md

@@ -42,7 +42,7 @@ When concurrent execution starts, the `ConcurrentPhaseExecutor` (in `orchestrato
 |-------|--------------|
 | `refine` | `refiner`, `reviewer_refine`, `reviewer_agent_design` (egg repo only) |
 | `plan` | `architect`, `task_planner`, `risk_analyst`, `reviewer_plan` |
-| `implement` | `coder`, `tester`, `documenter`, `reviewer_code`, `reviewer_code_holistic`, `reviewer_contract`, `reviewer_security` (ADVISORY), `reviewer_concurrency` (ADVISORY) |
+| `implement` | `coder`, `tester`, `documenter`, `reviewer_code`, `reviewer_code_holistic`, `reviewer_contract`, `reviewer_security`, `reviewer_concurrency` |
 
 **Shared branch**: All agents operate on the pipeline's shared branch (e.g., `egg/issue-123`). Agents coordinate commits via the message bus to sequence their work and avoid conflicts.
 
@@ -353,9 +353,9 @@ A reviewer has three outcomes on a proposal:
 - **NACK** — proposal is wrong; producer must iterate before merge.
 - **Conditional ACK** — proposal is correct but requires a specific human-only action *at merge time* (e.g. a `git mv`, a cross-repo config flip). Pass `--pre-merge-condition "..."` on `egg-orch consensus ack`; the condition is persisted on the approval-matrix edge, scoped to the current proposal version, surfaced in `egg-orch consensus status`, and rendered in a **Pre-merge Obligations** section on the auto-created PR body so the merger cannot skim past it. Not a soft NACK — if the agents can address the issue themselves, NACK instead. See the [Conditional ACK reference](../reference/conditional-ack.md).
 
-### Implement-phase `reviewer_code` Subagent Fan-Out
+### Implement-phase Reviewer Roster
 
-On the implement phase, `reviewer_code` self-gates on diff size and partitions large diffs across Claude Agent SDK `Task` subagents so every changed file is read carefully ([#1965](https://github.com/jwbron/egg/issues/1965)). The reviewer first runs `git diff --numstat` against the resolved base ref; when the diff exceeds **~10 changed files OR ~500 lines of change**, it self-fetches `phases.implement.tasks[]` via `mcp__sdlc__show_contract` and spawns one subagent per task partition (capped at 6 subagents per review with a 5-minute / 300-second per-subagent wall-clock timeout). Each subagent re-runs `git diff` filtered by its assigned path globs, reviews only its slice, and is forbidden from spawning subagents of its own. The parent reviewer then performs a **cross-partition consistency pass** (handler ↔ allowlist, route ↔ schema, fixture ↔ Dockerfile/symlink, import-graph cycles) before emitting a single ACK / NACK on behalf of the whole review — closing the cross-file blind spot that let [PR #1964](https://github.com/jwbron/egg/pull/1964)'s `^project$` allowlist bypass slip through. Below the threshold, on empty implement-phase task lists (custom-phase invocations, contractless `babysit_pr`), or when MCP is unreachable from the subagent context, the reviewer falls back to single-pass review with a STATUS heartbeat noting the gate decision (`fan-out: enabled / skipped`). Parallelism is configurable per pipeline via `phase_configs.implement.reviewer_code.parallel` (default `true`); the fan-out block lives in `_build_review_prompt()` in `orchestrator/routes/pipelines.py` and is described in detail in [`shared/prompts/REVIEWER-SYNC.md`](../../shared/prompts/REVIEWER-SYNC.md). `reviewer_code_holistic` ([#2126](https://github.com/jwbron/egg/issues/2126)) runs alongside `reviewer_code` as a distinct CRITICAL reviewer focused on cross-module coherence — it skims the full diff once and runs four holistic passes (end-to-end use case, doc↔code symmetry, synthetic-key/sentinel audit, silent-fallback hunt) rather than reviewing every file line-by-line. Its NACK gates consensus independently and is not averaged with fan-out slice ACKs. The two ADVISORY lens reviewers `reviewer_security` and `reviewer_concurrency` (criteria in [`security-review-criteria.md`](../../shared/prompts/security-review-criteria.md) and [`concurrency-review-criteria.md`](../../shared/prompts/concurrency-review-criteria.md)) also run alongside `reviewer_code` on the same change set; their NACKs are recorded but do not deadlock consensus until severity-tagged NACK signalling lands in [#1997](https://github.com/jwbron/egg/issues/1997).
+On the implement phase, `reviewer_code` reviews every changed file systematically and emits a single CRITICAL ACK / NACK on the full diff. `reviewer_code_holistic` ([#2126](https://github.com/jwbron/egg/issues/2126)) runs alongside as a distinct CRITICAL reviewer focused on cross-module coherence — it skims the full diff once and runs four holistic passes (end-to-end use case, doc↔code symmetry, synthetic-key/sentinel audit, silent-fallback hunt) rather than verifying every line. Its NACK gates consensus independently of `reviewer_code`'s. Two CRITICAL lens reviewers `reviewer_security` and `reviewer_concurrency` (criteria in [`security-review-criteria.md`](../../shared/prompts/security-review-criteria.md) and [`concurrency-review-criteria.md`](../../shared/prompts/concurrency-review-criteria.md)) also run on the same change set; a NACK from either blocks consensus until the producer re-proposes ([#2139](https://github.com/jwbron/egg/issues/2139) — promoted from ADVISORY, closing [#1997](https://github.com/jwbron/egg/issues/1997)).
 
 ### Pre-Proposal ACK Protection
 

docs/guides/sdlc-pipeline.md

@@ -941,23 +941,14 @@ Contracts can override phase defaults via the `phase_configs` field:
         }
       ],
       "max_review_cycles": 5,
-      "human_review_mechanism": "PR_REVIEW",
-      "reviewer_code": {
-        "parallel": true
-      }
+      "human_review_mechanism": "PR_REVIEW"
     }
   }
 }
 ```
 
 When `phase_configs.{phase}.checks` is specified, it completely replaces the default checks for that phase.
 
-The `reviewer_code` object exposes a single knob:
-
-| Field | Type | Default | Description |
-|-------|------|---------|-------------|
-| `parallel` | bool | `true` | Fan out `reviewer_code` subagents in parallel. Set `false` to force sequential review for cost or quota reasons. |
-
 ### Writing Custom Checks
 
 Custom checks can be configured per-repository in `~/.config/egg/repositories.yaml` (see above) or by adding check definitions to `shared/egg_contracts/phase_defaults.py`.

docs/reference/agent-roles.md

@@ -243,16 +243,14 @@ each surface so reviewers know to keep them in sync.
 - Allowed writes: `.egg-state/reviews/`, `.egg-state/agent-outputs/`
 - Blocked: All source, docs, tests, contracts, drafts
 
-**Subagent fan-out**: On large diffs (`files_changed > 10` OR `loc_added + loc_removed > 500`), `reviewer_code` fans out into Claude Agent SDK subagents — one per implement-phase task partition (capped at 6, with a 5-minute / 300-second per-subagent wall-clock timeout that NACKs the partition on overrun). Each subagent reviews its slice; the parent aggregates findings and emits the single ACK/NACK. A mandatory cross-partition consistency pass runs regardless of whether fan-out fires. Fan-out can be forced sequential via `phase_configs.implement.reviewer_code.parallel = false` (default: `true`).
-
 **Outputs**:
 - `.egg-state/reviews/{identifier}-implement-code-review.json` — Verdict file
 
 ### `reviewer_code_holistic`
 
-**Purpose**: Single-pass holistic code review focused on cross-module coherence. Runs alongside `reviewer_code`'s slice-by-slice fan-out — its job is the architectural-coherence question no fan-out slice owns.
+**Purpose**: Single-pass holistic code review focused on cross-module coherence. Runs alongside `reviewer_code` — its job is the architectural-coherence question line-by-line review does not own.
 
-**Criticality**: CRITICAL — NACKs block consensus on their own and are not averaged against `reviewer_code`'s fan-out ACKs.
+**Criticality**: CRITICAL — NACKs block consensus on their own, independent of `reviewer_code`'s verdict.
 
 **Focus areas** (four mandatory passes):
 1. Walk the primary advertised use case end-to-end across the full diff.
@@ -280,9 +278,9 @@ each surface so reviewers know to keep them in sync.
 
 ### `reviewer_security`
 
-**Purpose**: ADVISORY security-lens reviewer. Focuses exclusively on cross-file security invariants that a general code reviewer may miss: cross-file allowlist mismatches, handler-vs-validator path mismatches, information-disclosure and authorization-bypass patterns, uncommitted-artifact/Dockerfile-symlink mismatches, secret leakage, and OWASP top-10 patterns spanning multiple changed files.
+**Purpose**: Security-lens reviewer. Focuses exclusively on cross-file security invariants that a general code reviewer may miss: cross-file allowlist mismatches, handler-vs-validator path mismatches, information-disclosure and authorization-bypass patterns, uncommitted-artifact/Dockerfile-symlink mismatches, secret leakage, and OWASP top-10 patterns spanning multiple changed files.
 
-**Criticality**: ADVISORY — NACKs block consensus informally but do not deadlock BRC until severity-tagged NACK signalling lands. Promotion to CRITICAL is intentionally deferred.
+**Criticality**: CRITICAL — a NACK blocks consensus until the producer re-proposes ([#2139](https://github.com/jwbron/egg/issues/2139); promoted from ADVISORY, closing [#1997](https://github.com/jwbron/egg/issues/1997)).
 
 **File access**:
 - Allowed writes: `.egg-state/reviews/`, `.egg-state/agent-outputs/`
@@ -293,9 +291,9 @@ each surface so reviewers know to keep them in sync.
 
 ### `reviewer_concurrency`
 
-**Purpose**: ADVISORY concurrency-lens reviewer. Focuses exclusively on concurrency invariants: race conditions, deadlocks, shared-state mutation without synchronization, async-context leakage, retry-storm patterns, resource-cleanup ordering bugs, and BRC-protocol invariants (send→wait ordering, cursor threading, heartbeat-stall windows).
+**Purpose**: Concurrency-lens reviewer. Focuses exclusively on concurrency invariants: race conditions, deadlocks, shared-state mutation without synchronization, async-context leakage, retry-storm patterns, resource-cleanup ordering bugs, and BRC-protocol invariants (send→wait ordering, cursor threading, heartbeat-stall windows).
 
-**Criticality**: ADVISORY — same deferral rationale as `reviewer_security` above.
+**Criticality**: CRITICAL — same as `reviewer_security` above ([#2139](https://github.com/jwbron/egg/issues/2139)).
 
 **File access**:
 - Allowed writes: `.egg-state/reviews/`, `.egg-state/agent-outputs/`

docs/reference/checkpoint-browser.md

@@ -63,7 +63,7 @@ The `--agent-type` flag accepts both coarse agent types (e.g., `reviewer`) and c
 
 | Composite Role | Description |
 |----------------|-------------|
-| `reviewer_code` | Code quality reviewer (fan-out, slice-by-slice) |
+| `reviewer_code` | Code quality reviewer (line-by-line) |
 | `reviewer_code_holistic` | Holistic code reviewer (cross-module coherence) |
 | `reviewer_contract` | Contract compliance reviewer |
 | `reviewer_agent_design` | Agent design reviewer |