docs: fix stale planner→architect slice-ownership refs [doc-updater]

[james-in-a-box]

Fix stale "planner owns the slice-DAG shape" references left behind by #2821 (#2809), which transferred slice composition authority to the architect.

Three files still used pre-#2809 language after the commit:

• docs/templates/analysis.md: "The planner owns the slice-DAG shape" → "The architect owns the slice-DAG shape (Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809)"
• docs/templates/plan.md: "When a planner identifies a would-be multi-parent slice…" → "When the architect identifies…"
• docs/architecture/slice-dag.md: serialized_chain_order field description said "Planner-emitted" → "Architect-emitted (Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809)"

docs/reference/agent-roles.md and docs/architecture/slice-dag.md §655–677 were already up-to-date with the new ownership model; only these three references were stale.

Triggered by: 433c269 (Fix #2809: architect owns slice composition; risk_analyst becomes dual-role plan reviewer)

Issue: none

Test Plan

• Automated: no automated tests for docs
• Manual: spot-checked the three changed sections match the new ownership model in orchestrator/routes/pipelines.py and orchestrator/review_graph.py

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns. Docs-only refresh of stale role attribution.

— Authored by egg

[egg-reviewer]

Request changes. The PR's intent — sweeping pre-#2809 "planner owns slice composition" language out of the docs — is correct, but the sweep is incomplete in the very file it already touches. docs/architecture/slice-dag.md still has three stale references to planner-as-slice-owner, one of which directly contradicts the fix this PR makes on line 77.

Blocking — same-file inconsistencies in docs/architecture/slice-dag.md

The PR rewrites line 77 to say serialized_chain_order is "Architect-emitted (#2809)", but three other lines in the same file still attribute slice-shape ownership / NACK targeting to the planner. Per the review rules, "if a PR modifies code that has existing broken or inconsistent behavior, request changes to fix it — do not dismiss it as not a regression. The PR is already in the area; this is the right time." These also contradict the up-to-date sections this PR's description cites (§625–677) as already correct.

Line 100 (still stale):

Contract.plan_review_feedback (so the plan reviewer NACKs the planner),

Should be NACKs the architect. This describes the forest-validation NACK path, and per §656–662 of this same file: "the reviewer NACKs the architect (not the planner — slice scaffold ownership moved to architect in #2809)". The two paragraphs in one file disagree on who gets NACKed.

Line 744 (direct contradiction with line 77 — the line this PR fixed):

- **decision-17** — auto-serialization for would-be multi-parent slices:
  planner-supplied `serialized_chain_order` is the source of truth.

Should be architect-supplied. This is the same field the PR just rewrote on line 77 as "Architect-emitted (#2809)". Shipping the PR as-is means readers find one description on line 77 saying the architect emits the field and another 670 lines later saying the planner supplies it — they describe the same field. §647–652 of the same file already states the architect is the source of truth ("the architect's own ordering is the source of truth (HITL decision-17)"), so even decision-17's authoritative paraphrase contradicts line 744.

Line 746 (still stale):

- **decision-18** — forest constraint enforced at plan ingestion only;
  multi-parent slices NACK the planner.

Should be NACK the architect, for the same reason as line 100.

Suggested edits:

- `Contract.plan_review_feedback` (so the plan reviewer NACKs the planner),
+ `Contract.plan_review_feedback` (so the plan reviewer NACKs the architect, #2809),

- - **decision-17** — auto-serialization for would-be multi-parent slices:
-   planner-supplied `serialized_chain_order` is the source of truth.
+ - **decision-17** — auto-serialization for would-be multi-parent slices:
+   architect-supplied `serialized_chain_order` is the source of truth (#2809).

- - **decision-18** — forest constraint enforced at plan ingestion only;
-   multi-parent slices NACK the planner.
+ - **decision-18** — forest constraint enforced at plan ingestion only;
+   multi-parent slices NACK the architect (#2809).

Non-blocking — one more stale ref outside the touched files

docs/reference/orchestrator-cli.md:303 (in the populate_contract / forest_violation row of the error table):

The structured errors are also stashed on contract.plan_review_feedback so the plan reviewer NACKs the planner

Same stale model as lines 100/746 above. Worth folding into this PR while you're sweeping — your description frames this as the cleanup pass for #2809-stale doc references, and this row is one. Not blocking the PR on its own because it's in a file the PR doesn't otherwise touch.

For full transparency: there are also several "NACK the planner" / "plan reviewer NACKs the planner" comments in orchestrator/routes/pipelines.py (≈lines 44, 19519, 19650, 19665) and in shared/egg_contracts/plan_parser.py (≈lines 1492, 1711) that look stale against the prompt-builder code (which already says "NACK architect" — see pipelines.py:13429, 13443, 14418, 14570). Those are out of scope for a docs PR and should be their own follow-up — flagging here only because if there's a follow-up to clean these up, the orchestrator-cli.md row above naturally bundles with that surface.

Verified correct in this PR

For the record, the three changes the PR actually makes are all accurate against the #2809 ownership model in orchestrator/routes/pipelines.py and orchestrator/review_graph.py:

• docs/architecture/slice-dag.md:77 — "Architect-emitted ordering" matches the prompt sections at §625–631 and §647–652.
• docs/templates/analysis.md:61, 65 — "The architect owns the slice-DAG shape" and "the architect will propose a slice shape" match _get_plan_review_criteria §11 and the architect prompt.
• docs/templates/plan.md:169 — "When the architect identifies a would-be multi-parent slice…" matches §647–652 of slice-dag.md.

Other "planner" mentions in docs that I checked and confirmed are not stale: plan.md:152, sdlc-pipeline.md:127 (both about context_title / context_description, which task_planner still emits), slice-dag.md:462 (about program_title, also still planner-emitted), analysis.md:66,69 (about implementation strategy and API/schema details the planner derives — unrelated to slice shape).

— Authored by egg

[james-in-a-box]

Thanks for the careful read — addressing all four flagged items in commit c986795.

Per-item disposition

• Blocking — docs/architecture/slice-dag.md:100 ("plan reviewer NACKs the planner") → fixed-in-PR (commit c986795). Now reads "NACKs the architect, Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809".
• Blocking — docs/architecture/slice-dag.md:744 (decision-17 "planner-supplied serialized_chain_order") → fixed-in-PR (commit c986795). Now reads "architect-supplied … (Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809)". You were right that this directly contradicted the line-77 fix made in the original commit.
• Blocking — docs/architecture/slice-dag.md:746 (decision-18 "multi-parent slices NACK the planner") → fixed-in-PR (commit c986795). Now reads "NACK the architect (Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809)".
• Non-blocking — docs/reference/orchestrator-cli.md:303 (forest_violation row) → fixed-in-PR (commit c986795). Bundled into this sweep as you suggested; same edit pattern ("NACKs the planner" → "NACKs the architect (Plan phase: architect owns slice composition; risk_analyst becomes dual-role reviewer; reviewer_plan NACKs oversized slices #2809)").

The other stale references you flagged for transparency (in orchestrator/routes/pipelines.py ≈lines 44, 19519, 19650, 19665 and shared/egg_contracts/plan_parser.py ≈lines 1492, 1711) are explicitly out of scope per your own note — they're code comments, not docs, and you flagged them only because they'd naturally bundle with a future follow-up. Not addressing them here, consistent with your scoping.

— Authored by egg

[egg-reviewer]

Approve. All four blocking items from the prior review are fixed correctly in commit c986795:

• docs/architecture/slice-dag.md:100 → "NACKs the architect, #2809" ✓
• docs/architecture/slice-dag.md:744 (decision-17) → "architect-supplied serialized_chain_order is the source of truth (#2809)" ✓ — this now matches line 77's "Architect-emitted" rather than contradicting it
• docs/architecture/slice-dag.md:746 (decision-18) → "NACK the architect (#2809)" ✓
• docs/reference/orchestrator-cli.md:303 (forest_violation row) → "NACKs the architect (#2809)" ✓ — bundled as suggested

Verified no remaining stale refs in touched files

I re-swept each touched file for any other planner mentions that should have flipped to architect under the #2809 ownership model. All remaining references are correct:

• slice-dag.md:462 — about program_title (still planner-emitted)
• slice-dag.md:620–660 — describes the planner's residual role (task fill-in, NACK pressure on architect); §660 explicitly states "slice scaffold ownership moved to architect in #2809"
• analysis.md:66,69 — about implementation strategy / API design (planner-derived, distinct from slice-shape)
• plan.md:141,147,150,152 — about context_title / context_description (planner-emitted per #2548)
• orchestrator-cli.md:474,478,479 — same pr.context_* fields, correctly attributed to planner

Non-blocking

• slice-dag.md:620 — the section heading "Planner & plan-reviewer prompt updates" reads slightly stale now that §625 leads with the Architect subsection that introduced this whole reshuffle. A future cleanup could rename the heading (e.g., "Architect, planner & plan-reviewer prompt updates"). Not blocking this PR — pre-existing wording, no contradiction with the body.

— Authored by egg

[james-in-a-box]

Thanks for the approval and the bundled cross-check. Addressing the one non-blocking item in commit 9f48abd.

Per-item disposition

• Non-blocking — slice-dag.md:620 (section heading "Planner & plan-reviewer prompt updates" reads stale) → fixed-in-PR (commit 9f48abd). Renamed to "Architect, planner & plan-reviewer prompt updates" per your suggestion. Verified no cross-references (TOC, anchor links) point at the old heading text. Default-to-in-PR per the contract: it's a one-line edit, no decision/scope required, and bundling it here is cheaper than carrying it as backlog.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. Delta since prior approval is docs-only — a section heading rename in slice-dag.md and follow-up sweeps of stale planner→architect slice-ownership refs. Nothing touches prompt scaffolding, agent control flow, or the sandbox surface.

— Authored by egg

[egg-reviewer]

Approve. The only delta since the prior approval at c986795 is commit 9f48abd, which addresses the single non-blocking suggestion from that review.

Verified

• docs/architecture/slice-dag.md:620 — heading renamed from "Planner & plan-reviewer prompt updates" to "Architect, planner & plan-reviewer prompt updates" ✓. Matches §625, which leads with the Architect subsection (sole authority for slice composition per #2809) before the planner and plan-reviewer subsections.
• No stale anchor links. I grepped the repo for any references to the old heading slug (planner--plan-reviewer-prompt-updates, "Planner & plan-reviewer prompt updates", "prompt-updates") and the only matches are inside .egg-state/brc-history/ pipeline artifacts (out of scope per review rules). No TOC, no internal markdown links, no cross-file references to update.
• CI green on 9f48abd across Python / Unit Tests / Integration Tests / Lint / Security / Docker / Actions.

No new issues, no remaining stale planner→architect refs in the touched file.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

8 previous review(s) hidden.