docs: Add reviewer_code_holistic to agent docs [doc-updater]

[james-in-a-box]

Update documentation to reflect the reviewer_code_holistic role added in #2126 (#2130).

reviewer_code_holistic is a new CRITICAL implement-phase reviewer that runs alongside reviewer_code's fan-out. Its job is cross-module coherence (end-to-end use-case walk, doc↔code symmetry, synthetic-key audit, silent-fallback hunt) rather than line-by-line correctness. Its NACK gates consensus independently — it is not averaged with the fan-out reviewer's slice ACKs.

• docs/reference/agent-roles.md: Added reviewer_code_holistic to the REVIEW category table, Role Overview table, and a new Implement Phase section with criticality, focus areas, and file access
• docs/guides/concurrent-execution.md: Added to implement-phase spawn table; updated fan-out section to clarify its distinct CRITICAL role
• docs/reference/checkpoint-browser.md: Added to composite BRC role filtering table
• README.md: Updated implement-phase ASCII diagram to show the new role

Triggered by: #2130

Test Plan

• Automated: doc-only changes; no code paths affected
• Manual: verify role appears correctly in all four updated doc sections

Authored-by: egg

[egg-reviewer]

Output path is wrong — propagates pre-existing doc bug

Blocking: docs/reference/agent-roles.md:266 documents the new role's output as:

.egg-state/reviews/{identifier}-implement-reviewer_code_holistic-review.json

But the actual path is .egg-state/reviews/{identifier}-implement-code-holistic-review.json. Two pieces of evidence:

1. orchestrator/routes/pipelines.py:9227-9229 strips the reviewer_ prefix and converts _ → -:

   elif role_value.startswith("reviewer_"):
       reviewer_type = role_value.replace("reviewer_", "", 1).replace("_", "-")

   Then pipelines.py:3717 builds the filename as {prefix}-{phase}-{reviewer_type}-review.json.
2. The on-disk artifacts confirm this: .egg-state/reviews/ contains 507-implement-code-review.json, 545-implement-code-review.json, etc. — never *-implement-reviewer_code-*.

This means anyone using the doc to grep, glob, or open the verdict file will be looking at the wrong path. For a holistic-coherence reviewer whose own job description includes "doc↔code symmetry," shipping a wrong path on the very page that introduces it is the worst possible failure mode.

Pre-existing scope

The bug is not unique to the new entry. The same wrong format exists at lines 70, 81, 128, 247, 277, 290, 303 (every reviewer in the file). Per review-rules, pre-existing issues in code a PR touches are blocking — the PR is already in agent-roles.md, so this is the natural place to fix all eight occurrences in one shot. Concretely:

Line	Current (wrong)	Should be
70	…refine-reviewer_refine-review.json	…refine-refine-review.json
81	…refine-reviewer_agent_design-review.json	…refine-agent-design-review.json
128	…plan-reviewer_plan-review.json	…plan-plan-review.json
247	…implement-reviewer_code-review.json	…implement-code-review.json
266	…implement-reviewer_code_holistic-review.json	…implement-code-holistic-review.json
277	…implement-reviewer_contract-review.json	…implement-contract-review.json
290	…implement-reviewer_security-review.json	…implement-security-review.json
303	…implement-reviewer_concurrency-review.json	…implement-concurrency-review.json

Cross-check against .egg-state/reviews/ — the on-disk filenames (*-plan-plan-review.json, *-refine-refine-review.json, *-refine-agent-design-review.json, *-implement-code-review.json) match the right-hand column.

Everything else is accurate

I verified the rest of the PR's claims against the codebase:

• Role exists, REVIEW category — shared/egg_contracts/roles.py:74 (REVIEWER_CODE_HOLISTIC = "reviewer_code_holistic"), full role definition at roles.py:527-548 with category=AgentCategory.REVIEW.
• CRITICAL criticality — orchestrator/review_graph.py:245-247 registers both edges (→ coder, → tester) as ReviewCriticality.CRITICAL. Test at orchestrator/tests/test_reviewer_code_holistic.py:44-50 explicitly asserts this.
• Implement-phase roster — shared/egg_contracts/roles.py:1114-1121 includes AgentRole.REVIEWER_CODE_HOLISTIC in _PHASE_REVIEWERS["implement"].
• File access — roles.py:538-548 matches the doc exactly: writes to .egg-state/reviews/, .egg-state/agent-outputs/; _REVIEWER_BLOCKED_WRITE at lines 489-497 covers src/, lib/, docs/, tests/, test/, .egg-state/contracts/, .egg-state/drafts/.
• Checkpoint browser composite role — added correctly in docs/reference/checkpoint-browser.md.
• README ASCII box — width arithmetic is consistent (47 inner chars on every line of the implement box).
• concurrent-execution.md paragraph — accurately describes "NACK gates consensus independently and is not averaged with fan-out slice ACKs," matches review_graph.py and the test in test_reviewer_code_holistic.py:57-66.

Summary

Fix the eight output-path lines in docs/reference/agent-roles.md and re-push. The rest of the PR is accurate and well-scoped.

— Authored by egg

[egg-reviewer]

Re-review: blocking issue from prior review is fully addressed

The new commit f353ab9 fixes every one of the eight wrong output-path lines I called out in the previous review. I re-verified each one against the code and on-disk artifacts:

Line	Before	After (now correct)
70	refine-reviewer_refine	refine-refine ✓
81	refine-reviewer_agent_design	refine-agent-design ✓
128	plan-reviewer_plan	plan-plan ✓
247	implement-reviewer_code	implement-code ✓
266	implement-reviewer_code_holistic	implement-code-holistic ✓
277	implement-reviewer_contract	implement-contract ✓
290	implement-reviewer_security	implement-security ✓
303	implement-reviewer_concurrency	implement-concurrency ✓

These match:

• The transform at orchestrator/routes/pipelines.py:9229 (role_value.replace("reviewer_", "", 1).replace("_", "-")) feeding into the path template at pipelines.py:3717 ({prefix}-{phase}-{reviewer_type}-review.json).
• The actual on-disk filenames in .egg-state/reviews/ (e.g. 507-implement-code-review.json, 1014-plan-plan-review.json, 1014-refine-agent-design-review.json, 1014-refine-refine-review.json).

No regressions, no leftover stragglers

I also grepped for any remaining reviewer_*-review.json strings across the repo:

• grep -rn "reviewer_.*review.json" --include="*.md" returns only docs/architecture/orchestrator.md:191, which uses {reviewer_type} as a generic template placeholder — that's correct usage and out of scope for this PR.
• The PR's other touched docs (concurrent-execution.md, checkpoint-browser.md, README.md) contain no path strings, so they did not need changes.

Verdict

The pre-existing scope issue I raised has been fully and correctly resolved. All other facts in the PR (role registration, criticality, file access, ASCII diagram width, fan-out coexistence semantics) were already verified against the codebase in the prior review and remain accurate.

Approving.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

3 previous review(s) hidden.