docs: add lens reviewers and fan-out docs [doc-updater]

[james-in-a-box]

docs: add lens reviewers and fan-out docs [doc-updater]

Update documentation to reflect changes from a1e69de (PR #2061):

• docs/reference/agent-roles.md: Added reviewer_security and reviewer_concurrency to the REVIEW category table and Role Overview table. Added dedicated sections for both advisory lens reviewers. Updated reviewer_code section to document the new subagent fan-out behavior (threshold gate, partition-by-task, 6-subagent cap, cross-partition consistency pass, and the parallel knob).
• README.md: Updated implement phase pipeline diagram and step-3 description to include the two new advisory lens reviewers.
• docs/guides/sdlc-pipeline.md: Added reviewer_security and reviewer_concurrency to the Concurrent Execution Mode agent list. Added reviewer_code.parallel to the phase_configs example and documented the knob in a reference table.

Triggered by: #2061

Issue: none

Test Plan

• Automated: doc-only changes, no code tests needed
• Manual: review updated sections for accuracy against the new code in shared/egg_contracts/agent_roles.py, orchestrator/review_graph.py, shared/egg_contracts/models.py, and orchestrator/routes/pipelines.py

Authored-by: egg

[egg-reviewer]

No agent-mode design concerns. Documentation-only change describing the new advisory lens reviewers (reviewer_security, reviewer_concurrency) and the reviewer_code.parallel knob — no prompt content, output formats, pre-fetching, or LLM-invocation patterns to assess.

— Authored by egg

[egg-reviewer]

Review

Doc-only PR. I verified every technical claim against the source.

Verified ✓

• reviewer_security / reviewer_concurrency defined in shared/egg_contracts/agent_roles.py:623-669, both REVIEW category, allowed_write [reviews/, agent-outputs/], _REVIEWER_BLOCKED_WRITE matches the documented "blocked" list.
• ADVISORY criticality for both lens reviewers in orchestrator/review_graph.py:247-253 (4 advisory edges into coder/tester).
• _PHASE_REVIEWERS["implement"] registers both new roles (agent_roles.py:1085-1086).
• Subagent fan-out prompt (orchestrator/routes/pipelines.py:4452-4540): threshold gate, partition-by-task, 6-subagent cap, mandatory cross-partition pass that runs in all paths (incl. below-threshold and both fallbacks) — all match the doc paragraph.
• ReviewerCodeConfig.parallel field with default True in shared/egg_contracts/models.py:301-339. get_reviewer_code_parallel() resolver handles all three None-layer paths.
• Output filename pattern {identifier}-implement-reviewer_{security,concurrency}-review.json is consistent with existing _verdict_path_for_type() convention.
• README implement-box and step 3 wording, and the Concurrent Execution Mode line in sdlc-pipeline.md:1208-1209, accurately reflect roster composition.

Non-blocking observations

1. Threshold wording is slightly imprecise. agent-roles.md says files_changed > 10 OR loc > 500. The actual gate in pipelines.py:4475 is files_changed > 10 OR (loc_added + loc_removed) > 500. "loc" is ambiguous — readers could read it as "lines added" only. Suggest: loc_added + loc_removed > 500 or "total LOC changed > 500".

2. 5-minute per-subagent timeout omitted. The fan-out prompt (pipelines.py:4515) imposes a 300-second wall-clock cap per subagent that NACKs the partition on timeout. The doc paragraph mentions threshold/cap/consistency-pass but skips this. Useful operational detail; consider adding "5-minute (300s) per-subagent timeout".

3. "Parallel?" column wording is inconsistent. Existing rows use Yes (with <peer>, <peer>) to enumerate concurrent peers. The two new rows use Yes (advisory) which conflates the parallelism column with criticality. The lens reviewers ARE concurrent with reviewer_code/reviewer_contract/each other — the column should ideally mirror that pattern, e.g. Yes (with reviewer_code, reviewer_contract, reviewer_concurrency). Criticality already has its own column elsewhere; "advisory" can live in the per-role section text.

4. "Depends On = coder, tester" carries forward a doc/code drift. The dependencies field in agent_roles.py for all four implement-phase reviewers (incl. the two existing ones) is [TASK_PLANNER, RISK_ANALYST], not [CODER, TESTER]. The new rows perpetuate the existing convention; matching the existing pattern is fine, but the underlying drift between the doc table's "Depends On" column and the code's dependencies attribute predates this PR. Not asking you to fix it here — flagging so it's visible.

Verdict

Approve. The new content is accurate and matches the code. The four notes above are stylistic / clarity improvements, not blockers.

— Authored by egg

[james-in-a-box]

Thanks for the careful review. Pushed fbb297c addressing notes 1–3:

1. Threshold wording — loc > 500 → loc_added + loc_removed > 500, matching pipelines.py:4475.
2. 5-minute timeout — added to the fan-out paragraph: subagents have a 300-second wall-clock cap that NACKs the partition on overrun.
3. Parallel? column — replaced Yes (advisory) with Yes (with reviewer_code, reviewer_contract, reviewer_<peer>) for both lens-reviewer rows, mirroring the existing pattern. Criticality stays in the per-role section.

Note 4 (Depends On = coder, tester doc/code drift) — leaving as-is per your guidance; it predates this PR and the doc-table convention is consistent across all four implement-phase reviewer rows.

— Authored by egg

[egg-reviewer]

No agent-mode design concerns. Re-review delta is a 3-line stylistic/clarity update to docs/reference/agent-roles.md (LOC threshold formula, 5-minute subagent timeout, Parallel? column convention) addressing prior review notes 1–3. No prompt content, output formats, pre-fetching, or LLM-invocation patterns changed.

— Authored by egg

[egg-reviewer]

Re-review

Verified the delta (fbb297c) against source. All three actionable notes from my prior review are correctly addressed:

1. Threshold wording — loc > 500 → loc_added + loc_removed > 500. Matches the gate at orchestrator/routes/pipelines.py:4475.
2. 5-minute timeout — added (capped at 6, with a 5-minute / 300-second per-subagent wall-clock timeout that NACKs the partition on overrun). Matches the cap at pipelines.py:4515 and the NACK-on-overrun rule at pipelines.py:4519-4521 (subagents that exceed 300s yield a partition NACK with reason "subagent timeout").
3. Parallel? column — both lens-reviewer rows now use Yes (with reviewer_code, reviewer_contract, reviewer_<peer>), mirroring the existing convention. Criticality stays in the per-role section, as intended.

Note 4 (Depends On = coder, tester doc/code drift) was correctly left out of scope; it predates this PR and the convention is consistent across all four implement-phase reviewer rows.

No new issues introduced. Doc-only delta, no functional impact.

Verdict

Approve.

— Authored by egg

[james-in-a-box]

egg is addressing review feedback...

[james-in-a-box]

egg feedback addressed. View run logs

6 previous review(s) hidden.