Fix #2809: architect owns slice composition; risk_analyst becomes dual-role plan reviewer

docs/architecture/slice-dag.md

@@ -622,36 +622,59 @@ team uses.
 The dynamic prompt builders for `task_planner` and `reviewer_plan` were
 extended to teach the agents the new schema and constraints:
 
-- **Planner (`task_planner`)** — three new sections appended to the plan
-  phase prompt:
-  1. *Slice-sizing guidance* (advisory only, never enforced; HITL
-     decision-6 opt-2): the planner is encouraged to keep slices ≤1,000
-     LOC and informed that the plan reviewer issues escalating advisories
-     at >1,000 / >2,000 LOC.
+- **Architect (`architect`)** — sole authority for slice composition
+  (#2809, inverting HITL decision-6 opt-2). The architect prompt
+  declares this authority explicitly and the architect emits a binding
+  `architect-slices.yaml` scaffold alongside its analysis JSON
+  (`{identifier}-architect-slices.yaml` under `.egg-state/agent-outputs/`).
+  The scaffold encodes slice `id` / `name` / `goal` / `parent_slice_id`;
+  `tasks:` is intentionally omitted (that is `task_planner`'s job).
+- **Planner (`task_planner`)** — sections appended to the plan phase
+  prompt:
+  1. *Slice composition is NOT the planner's call* (#2809): the
+     architect's `architect-slices.yaml` scaffold is binding. The
+     planner copies it verbatim into the `# yaml-tasks` appendix
+     (same `id` / `name` / `goal` / `parent_slice_id`, same order) and
+     fills in `tasks:` under each slice. The planner does not
+     silently re-shape slices — if a slice needs to be subdivided,
+     the planner raises NACK pressure on the architect (via the
+     plan prose, which `risk_analyst` / `reviewer_plan` pick up)
+     rather than re-shaping locally.
   2. *Forest constraint* (HARD): every slice must have ≤1 DAG parent;
      the populator hard-rejects multi-parent slices with
-     `ForestValidationError`.
-  3. *Auto-serialization rule with worked example*: when the planner
-     identifies a slice that would otherwise have >1 parents, it
-     serialises the upstream cluster into a chain and records the chosen
-     order on the downstream slice's `serialized_chain_order` field. The
-     fallback heuristic (`files_affected` Jaccard >0.3, then descending
-     fan-out) is documented; the planner's own ordering is the source of
-     truth (HITL decision-17).
+     `ForestValidationError`. The architect's scaffold encodes this
+     via `parent_slice_id`; the planner preserves it.
+  3. *Auto-serialization* for would-be multi-parent slices: the
+     architect is responsible for serialising the upstream cluster
+     and populating `serialized_chain_order` on the downstream
+     slice. The fallback heuristic (`files_affected` Jaccard >0.3,
+     then descending fan-out) is documented; the architect's own
+     ordering is the source of truth (HITL decision-17). The
+     planner preserves the field verbatim from the scaffold.
   4. The yaml-block key swap: `slices:` is canonical, `phases:` is
      backward-compat.
 - **Plan reviewer (`reviewer_plan`)** — two new prompt sections:
   1. *Forest-violation NACK*: when the populator left a "Plan ingestion
      REJECTED" block on `plan_review_feedback` (or a `forest_violation`
-     log discriminator is present), the reviewer NACKs the planner with
-     the structured errors verbatim and instructs re-emission with
-     `serialized_chain_order` populated.
-  2. *Slice-sizing advisory* (advisory only, NEVER NACK): tone scales
-     with magnitude — 1,000–2,000 LOC: "consider splitting"; >2,000 LOC:
-     "well above the soft target — strongly consider splitting". HITL
-     decision-6 opt-2 keeps override authority with the
-     refiner/operator; promoting the advisory to a hard NACK requires a
-     fresh HITL revision.
+     log discriminator is present), the reviewer NACKs the **architect**
+     (not the planner — slice scaffold ownership moved to architect in
+     #2809) with the structured errors verbatim and instructs re-emission
+     of the scaffold with `serialized_chain_order` populated.
+  2. *Slice-sizing NACK* (hard, judgment-based; #2809 inverts HITL
+     decision-6 opt-2). The reviewer is empowered AND required to
+     hard-NACK the architect on `slice_size` when a slice is oversized
+     for one BRC cycle. There is no fixed LOC budget — the rubric is
+     judgment-based: NACK when a slice bundles >~3 distinct
+     file-categories, combines deletion-heavy work with new-API
+     introduction, would require >3–4 commit-propose-revise cycles to
+     converge, or contains independent task groups with no internal
+     dependency. The reviewer names the seam where subdivision is
+     appropriate so the architect's re-propose is actionable. See
+     `_get_plan_review_criteria()` §11 for the full rubric and worked
+     NACK examples. Refiner/operator override remains available when a
+     large slice is deliberate (e.g. atomic schema migration) — the
+     architect cites the override in the analysis and the reviewer
+     ACKs once the rationale is on the record.
 
 ## Configuration knobs
 

docs/reference/agent-roles.md

@@ -25,8 +25,8 @@ Use `get_roles_by_category(AgentCategory.REVIEW)` to dynamically query roles by
 | `reviewer_agent_design` | Review | Refine (egg repo only) | Yes (with `reviewer_refine`) | refiner |
 | `architect` | Analysis | Plan | No | — |
 | `task_planner` | Analysis | Plan | Yes (with `risk_analyst`) | architect |
-| `risk_analyst` | Analysis | Plan | Yes (with `task_planner`) | architect |
-| `reviewer_plan` | Review | Plan | No | task_planner, risk_analyst |
+| `risk_analyst` | Analysis (dual-role: also reviews `architect` + `task_planner`) | Plan | Yes (with `task_planner`) | architect |
+| `reviewer_plan` | Review | Plan | No | architect, task_planner, risk_analyst |
 | `applier` | Execution | Apply (epic-mode only) | No | — |
 | `coder` | Execution | Implement | No | — |
 | `tester` | Execution | Implement | Yes (with `documenter`) | coder |
@@ -85,37 +85,46 @@ All agents within a phase run concurrently via BRC consensus. Concurrency is ena
 
 ### `architect`
 
-**Purpose**: Analyze the task, research the codebase, and recommend a high-level implementation approach.
+**Purpose**: Analyze the task, research the codebase, and recommend a high-level implementation approach. **Sole authority for slice composition** in the plan phase (#2809) — owns slice count, slice boundaries, slice DAG shape, and sub-slicing. Emits a binding `architect-slices.yaml` scaffold that `task_planner` copies verbatim into the plan document's `# yaml-tasks` appendix.
 
 **File access**:
 - Allowed writes: `.egg-state/drafts/`, `.egg-state/agent-outputs/`
 - Blocked: `src/`, `lib/`, `shared/`, `gateway/`, `sandbox/`, `action/`, `docs/`, `tests/`, `.egg-state/contracts/`, `.egg-state/reviews/`, `.github/`
 
 **Outputs**:
 - `.egg-state/agent-outputs/{identifier}-architect-output.json` — Architectural analysis
+- `.egg-state/agent-outputs/{identifier}-architect-slices.yaml` — Binding slice scaffold (`id` / `name` / `goal` / `parent_slice_id`; no `tasks:` — that is `task_planner`'s job)
 
 **Prompt context**: Full issue body, refine analysis.
 
+**Reviewed by**: `reviewer_plan` (structural lens, CRITICAL) and `risk_analyst` (risk lens, CRITICAL).
+
 ### `task_planner`
 
-**Purpose**: Break the work into discrete phases and tasks with acceptance criteria. Produces the plan document with a YAML appendix.
+**Purpose**: Enumerate the discrete tasks (with acceptance criteria) **inside the architect's slice scaffold**. Produces the plan document with a YAML appendix; the slice shape comes from `architect-slices.yaml` verbatim (#2809). When a slice needs subdivision, the planner raises NACK pressure on the architect via the plan prose rather than silently re-shaping slices.
 
 **File access**: Same as `architect`.
 
 **Outputs**:
-- `.egg-state/drafts/{identifier}-plan.md` — The plan document (includes YAML appendix)
+- `.egg-state/drafts/{identifier}-plan.md` — The plan document (includes YAML appendix; slice scaffold copied verbatim from architect, `tasks:` enumerated underneath)
 - `.egg-state/agent-outputs/{identifier}-task_planner-output.json` — Handoff data
 
-**Prompt context**: Full issue body, architect output.
+**Prompt context**: Full issue body, architect output, architect slice scaffold.
+
+**Reviewed by**: `reviewer_plan` (structural lens, CRITICAL) and `risk_analyst` (risk lens, CRITICAL).
 
 ### `risk_analyst`
 
-**Purpose**: Identify technical risks and propose mitigation strategies.
+**Purpose**: Identify technical risks and propose mitigation strategies. **Dual-role in the plan phase** (#2809) — also reviews `architect` and `task_planner` through the risk lens, mirroring the implement-phase `tester` dual-role pattern (#2749).
+
+**Producer role**: emit a risk register (top risks, mitigations, blocking concerns).
+
+**Reviewer role**: CRITICAL reviewer of `architect` and `task_planner`. NACK when a risk is severe enough that shipping the plan as-proposed would invite a known-class failure (security regression, data loss, compliance break, runtime-primitive or trust-boundary mismatch). ACK when risks are real but mitigated, or low enough that the plan can ship and the risks belong in the register as forward-looking notes. Verdict / feedback is mirrored from the producer artifact into the `egg-orch consensus ack` / `nack` reason body so the upstream producer can act on it. The *Dual-Role Execution Order* banner in the BRC preamble is the authoritative ordering — `CONSENSUS_PROPOSE`-augmented producer waits wake the agent on architect's / task_planner's proposals.
 
 **File access**: Same as `architect`.
 
 **Outputs**:
-- `.egg-state/agent-outputs/{identifier}-risk_analyst-output.json` — Risk analysis
+- `.egg-state/agent-outputs/{identifier}-risk_analyst-output.json` — Risk analysis (producer artifact carries the verdict + `blocking_concerns` + `feedback` payload mirrored into reviewer ACK/NACK reasons)
 
 **Prompt context**: Full issue body, architect output.
 

orchestrator/review_graph.py

@@ -236,22 +236,40 @@ def get_default_refine_graph() -> ReviewGraph:
 def get_default_plan_graph() -> ReviewGraph:
     """Get the default review graph for the plan phase.
 
-    Review adjacency per the phase-role mappings:
-    - reviewer_plan reviews architect (critical)
-    - reviewer_plan reviews task_planner (critical)
-    - reviewer_plan reviews risk_analyst (advisory)
+    Review adjacency per the phase-role mappings (issue #2809):
+    - reviewer_plan reviews architect (critical) — structural lens
+    - reviewer_plan reviews task_planner (critical) — structural lens
+    - reviewer_plan reviews risk_analyst (advisory) — risk register
+      still reviewable as a producer artifact
+    - risk_analyst reviews architect (critical) — risk lens, dual-role
+    - risk_analyst reviews task_planner (critical) — risk lens, dual-role
+
+    Plan-phase consensus therefore requires **both** ``reviewer_plan``
+    and ``risk_analyst`` to ACK every CRITICAL producer (architect,
+    task_planner). The two lenses catch what one wouldn't: ``reviewer_plan``
+    audits structure (slice DAG shape, slice_size, role assignments,
+    test strategy, rollback, PR block); ``risk_analyst`` audits the
+    risk surface (what could go wrong with this design; blocking
+    concerns). ``risk_analyst`` is dual-role — it produces the risk
+    register and also reviews its upstream peers, mirroring the
+    implement-phase ``tester`` pattern (#2749).
 
     Producers: architect, task_planner, risk_analyst
-    Reviewers: reviewer_plan
+    Reviewers: reviewer_plan, risk_analyst (dual-role)
     """
     return ReviewGraph(
         [
-            # reviewer_plan reviews architect (critical)
+            # reviewer_plan reviews architect (critical) — structural lens
             ReviewEdge("reviewer_plan", "architect", ReviewCriticality.CRITICAL),
-            # reviewer_plan reviews task_planner (critical)
+            # reviewer_plan reviews task_planner (critical) — structural lens
             ReviewEdge("reviewer_plan", "task_planner", ReviewCriticality.CRITICAL),
-            # reviewer_plan reviews risk_analyst (advisory)
+            # reviewer_plan reviews risk_analyst (advisory) — risk
+            # register still reviewable as a producer artifact
             ReviewEdge("reviewer_plan", "risk_analyst", ReviewCriticality.ADVISORY),
+            # risk_analyst reviews architect (critical — risk lens, #2809)
+            ReviewEdge("risk_analyst", "architect", ReviewCriticality.CRITICAL),
+            # risk_analyst reviews task_planner (critical — risk lens, #2809)
+            ReviewEdge("risk_analyst", "task_planner", ReviewCriticality.CRITICAL),
         ]
     )