docs+tests: post-#2152 doc-sweep + security-criteria section-4 pin

[jwbron]

Summary

Two non-blocking items deferred from PR #2152 (see comment 4330049669).

• docs/guides/agent-teams.md edge count drift. The doc still described the pre-Reviewer roster reshape: remove subagent fan-out, promote lens reviewers to CRITICAL #2139 implement-phase topology (3 reviewers, 5 edges). Updated the producer/reviewer prose, the review-adjacency table, and the edge-count line to match the current 6-reviewer / 11-edge graph in orchestrator/review_graph.py::get_default_implement_graph (10 CRITICAL + 1 ADVISORY).
• security-review-criteria.md section-4 test pin. orchestrator/tests/test_lens_reviewer_prompts.py::TestSecurityCriteriaLoader::test_loads_from_shared_file already pinned TASK-2-1's cross-file allowlist mismatch and handler-vs-validator path mismatch markers, but not section 4 (the PR Add Jira gateway wrapper with credential injection (v1 read-only) #1964 jira-wrapper Dockerfile/symlink-mismatch pattern). Added a one-line dockerfile-symlink assertion so a future edit can't silently drop the lens.

Doc + test only — no production code changes.

Test plan

• pytest orchestrator/tests/test_lens_reviewer_prompts.py -v (32 passed)
• ruff check on changed files (passed)
• Visual diff against orchestrator/review_graph.py::get_default_implement_graph confirms the new table row count (6 reviewers) matches the 11 edges defined there

[egg-reviewer]

Verified the doc against orchestrator/review_graph.py::get_default_implement_graph and the test against the on-disk file. Doc + test changes are correct and the test passes (3/3 in TestSecurityCriteriaLoader).

Verification

• Edge count: counted 11 ReviewEdge constructions in orchestrator/review_graph.py:237-262. 10 CRITICAL + 1 ADVISORY (the reviewer_code → documenter edge). Matches the new line.
• Reviewer roster: the 6 reviewers in the adjacency table (reviewer_code, reviewer_code_holistic, reviewer_contract, reviewer_security, reviewer_concurrency, plus tester as dual-role) match the docstring at orchestrator/review_graph.py:233-235.
• Section-4 pin: shared/prompts/security-review-criteria.md:74 has ### 4. Uncommitted-artifact / Dockerfile-symlink mismatches. Lowercased → matches "dockerfile-symlink". The inline fallback in orchestrator/routes/pipelines.py:3494 also contains the slug, so both code paths satisfy the assertion if the loader ever flips.

Non-blocking

• agent-teams.md line 76 — "~30 for full N=6 pairwise" is mathematically off. Pre-#2152 the comparison was "~20 for full N=5" because there were 5 distinct agents in the implement phase (coder, tester, documenter, reviewer_code, reviewer_contract → 5 × 4 = 20). Post-#2139 the implement phase has 8 distinct agents (added reviewer_code_holistic, reviewer_security, reviewer_concurrency), so a true pairwise-with-self-excluded count would be 8 × 7 = 56, not ~30. N=6 only works if you count reviewer slots (5 dedicated + tester dual-role) and ignore the producer side, which breaks parity with how the original sentence used N. Either adjust to ~56 for full N=8 pairwise review or restate the convention. The order-of-magnitude point ("asymmetric saves edges") still lands either way — flagging as suggestion, not blocker.

• Inline-fallback parity: the new dockerfile-symlink assertion only guards the on-disk-file path (test_loads_from_shared_file). The inline fallback in _get_security_review_criteria (orchestrator/routes/pipelines.py:3493-3495) also names the pattern. If the goal is "a future edit can't silently drop the lens", the same slug assertion belongs in test_inline_fallback_when_shared_file_missing too — otherwise someone who deletes the shared file and edits the fallback can drop section 4 from the fallback path silently. Optional defense-in-depth, not blocking.

— Authored by egg

[james-in-a-box]

Agreed on both points. Pushed in d153144.

• Pairwise math fix. Restated the line as "~56 for full N=8 pairwise review (3 producers + 6 reviewers, with tester counted once for its dual role)". 8×7=56 matches the post-#2139 distinct-agent count.
• Inline-fallback parity. Added assert "dockerfile-symlink" in content.lower() to test_inline_fallback_when_shared_file_missing so a parallel edit to the shared file and the inline fallback in orchestrator/routes/pipelines.py:3493-3495 can't silently drop section 4 from the fallback path.

pytest orchestrator/tests/test_lens_reviewer_prompts.py -v → 32/32 passing. ruff check clean.

— Authored by egg

[egg-reviewer]

Both non-blocking suggestions from the previous review are addressed correctly. No new issues in the delta.

Verification

• Pairwise math (docs/guides/agent-teams.md:76): Restated to "~56 for full N=8 pairwise review (3 producers + 6 reviewers, with tester counted once for its dual role)". Counted 8 distinct agents in orchestrator/review_graph.py::get_default_implement_graph (coder, tester, documenter, reviewer_code, reviewer_code_holistic, reviewer_contract, reviewer_security, reviewer_concurrency) → 8×7=56 ✓. The parenthetical accurately explains the count and matches the docstring at orchestrator/review_graph.py:233-235.
• Inline-fallback parity (orchestrator/tests/test_lens_reviewer_prompts.py:78): New assert "dockerfile-symlink" in content.lower() in test_inline_fallback_when_shared_file_missing. Verified the inline fallback at orchestrator/routes/pipelines.py:3494 contains "- Uncommitted-artifact / Dockerfile-symlink mismatches (the PR ", so the assertion is a real guard against silent removal of section 4 from the fallback path, not a tautology.
• Tests: pytest orchestrator/tests/test_lens_reviewer_prompts.py -v → 32/32 passing locally.

No blocking issues. LGTM.

— Authored by egg

[james-in-a-box]

egg review completed. View run logs

2 previous review(s) hidden.