From 67e1c0906115cf1a1c62be2db0efb4e8aa519b2a Mon Sep 17 00:00:00 2001 From: James Wiesebron Date: Mon, 27 Apr 2026 13:31:09 -0700 Subject: [PATCH 1/2] docs+tests: post-#2152 doc-sweep + security-criteria section-4 pin PR #2152 deferred two non-blocking items (see https://github.com/jwbron/egg/pull/2152#issuecomment-4330049669): - docs/guides/agent-teams.md still described the pre-#2139 implement-phase topology (3 reviewers, 5 edges). Update the 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). - orchestrator/tests/test_lens_reviewer_prompts.py pinned TASK-2-1's cross-file/handler markers but not section 4 of the security criteria (the PR #1964 jira-wrapper Dockerfile/symlink-mismatch pattern). Add a one-line dockerfile-symlink assertion so a future edit can't silently drop the lens. --- docs/guides/agent-teams.md | 7 +++++-- orchestrator/tests/test_lens_reviewer_prompts.py | 9 ++++++--- 2 files changed, 11 insertions(+), 5 deletions(-) diff --git a/docs/guides/agent-teams.md b/docs/guides/agent-teams.md index 214d46dbab..9481ba43cd 100644 --- a/docs/guides/agent-teams.md +++ b/docs/guides/agent-teams.md @@ -56,7 +56,7 @@ BRC is a structured consensus protocol inspired by Interactive Consistency, Ack- Not all agents review all other agents. The review graph is **asymmetric by role type**: - **Producers** (coder, tester, documenter, autofixer, conflict_resolver): Create artifacts and propose them for review. Includes execution-category and utility-category agents. -- **Reviewers** (reviewer_code, reviewer_contract): Evaluate producers' proposals and issue ACK/NACK judgments. All review-category agents. +- **Reviewers** (reviewer_code, reviewer_code_holistic, reviewer_contract, reviewer_security, reviewer_concurrency): Evaluate producers' proposals and issue ACK/NACK judgments. All review-category agents. This eliminates circular ACK problems. A coder doesn't ACK a reviewer's review of its own code — it *responds to NACKs* by revising and re-proposing. @@ -65,12 +65,15 @@ This eliminates circular ACK problems. A coder doesn't ACK a reviewer's review o | Reviewer | Reviews proposals from | |----------|----------------------| | reviewer_code | coder, tester | +| reviewer_code_holistic | coder, tester | | reviewer_contract | coder | +| reviewer_security | coder, tester | +| reviewer_concurrency | coder, tester | | tester | coder (implicitly — writes tests against the code, runs lint/type-checks, ACKs if tests and checks pass) | The tester has a **dual role**: it is both a producer (proposes test artifacts) and a reviewer (evaluates coder's work by running tests and lint/type-checks against it). -This gives 5 directed review edges (4 critical + 1 advisory to documenter) for the default implement phase instead of ~20 for full N=5 pairwise review. The edge count varies by phase configuration. +This gives 11 directed review edges (10 critical + 1 advisory to documenter) for the default implement phase instead of ~30 for full N=6 pairwise review. The edge count varies by phase configuration. #### BRC Phases diff --git a/orchestrator/tests/test_lens_reviewer_prompts.py b/orchestrator/tests/test_lens_reviewer_prompts.py index 9a9231bebc..bd2b1af836 100644 --- a/orchestrator/tests/test_lens_reviewer_prompts.py +++ b/orchestrator/tests/test_lens_reviewer_prompts.py @@ -56,13 +56,16 @@ def test_returns_non_empty(self) -> None: def test_loads_from_shared_file(self) -> None: """Happy path: the loader returns the on-disk file's contents. - Asserts the regression-guard markers from TASK-2-1's required body: - ``cross-file allowlist mismatch`` and - ``handler-vs-validator path mismatch``. + Asserts the regression-guard markers from TASK-2-1's required body + (``cross-file allowlist mismatch`` and + ``handler-vs-validator path mismatch``) plus the section-4 + ``Dockerfile-symlink`` slug from the PR #1964 jira-wrapper pattern, + so a future edit can't silently drop the lens. """ content = _get_security_review_criteria() assert "cross-file allowlist mismatch" in content.lower() assert "handler-vs-validator path mismatch" in content.lower() + assert "dockerfile-symlink" in content.lower() def test_inline_fallback_when_shared_file_missing(self) -> None: with patch("routes.pipelines._read_shared_criteria", return_value=None): From d153144a4bf5b8ecad98e1a9054925e928c44277 Mon Sep 17 00:00:00 2001 From: "egg-reviewer[bot]" <261018737+egg-reviewer[bot]@users.noreply.github.com> Date: Mon, 27 Apr 2026 20:44:22 +0000 Subject: [PATCH 2/2] Address review: fix N=8 pairwise math + dockerfile-symlink fallback parity Two non-blocking suggestions from #2161 review: - agent-teams.md: 'N=6 pairwise / ~30' was mathematically off. The default implement phase has 8 distinct agents (3 producers + 6 reviewers, with tester counted once for its dual role), so the pairwise upper bound is 8x7=56, not ~30. Restated to 'N=8 / ~56' with an inline note clarifying how N is counted. - test_lens_reviewer_prompts.py: the 'dockerfile-symlink' assertion only guarded test_loads_from_shared_file. The inline fallback in _get_security_review_criteria also names the pattern, so a parallel edit could silently drop section 4 from the fallback path. Added the same slug assertion to test_inline_fallback_when_shared_file_missing so both code paths are pinned. --- docs/guides/agent-teams.md | 2 +- orchestrator/tests/test_lens_reviewer_prompts.py | 4 ++++ 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/guides/agent-teams.md b/docs/guides/agent-teams.md index 9481ba43cd..0dc31a67a3 100644 --- a/docs/guides/agent-teams.md +++ b/docs/guides/agent-teams.md @@ -73,7 +73,7 @@ This eliminates circular ACK problems. A coder doesn't ACK a reviewer's review o The tester has a **dual role**: it is both a producer (proposes test artifacts) and a reviewer (evaluates coder's work by running tests and lint/type-checks against it). -This gives 11 directed review edges (10 critical + 1 advisory to documenter) for the default implement phase instead of ~30 for full N=6 pairwise review. The edge count varies by phase configuration. +This gives 11 directed review edges (10 critical + 1 advisory to documenter) for the default implement phase instead of ~56 for full N=8 pairwise review (3 producers + 6 reviewers, with tester counted once for its dual role). The edge count varies by phase configuration. #### BRC Phases diff --git a/orchestrator/tests/test_lens_reviewer_prompts.py b/orchestrator/tests/test_lens_reviewer_prompts.py index bd2b1af836..997ed09718 100644 --- a/orchestrator/tests/test_lens_reviewer_prompts.py +++ b/orchestrator/tests/test_lens_reviewer_prompts.py @@ -71,6 +71,10 @@ def test_inline_fallback_when_shared_file_missing(self) -> None: with patch("routes.pipelines._read_shared_criteria", return_value=None): content = _get_security_review_criteria() assert content.strip() != "", "Security inline fallback must be non-empty" + # Section-4 parity with on-disk file: the inline fallback also names + # the PR #1964 jira-wrapper Dockerfile/symlink-mismatch pattern, so a + # future edit cannot silently drop the lens from the fallback path. + assert "dockerfile-symlink" in content.lower() class TestConcurrencyCriteriaLoader: