(magpie) branch main updated: fix(pr-management-code-review): name adversarial reviewers as prose in the review body (#1685)

[po...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

potiuk pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/magpie.git


The following commit(s) were added to refs/heads/main by this push:
     new d4b7dc34 fix(pr-management-code-review): name adversarial reviewers as prose in the review body (#1685)
d4b7dc34 is described below

commit d4b7dc34eab334a1fbe0a3478185eb6718df4912
Author: Jarek Potiuk <ja...@potiuk.com>
AuthorDate: Sat Oct 17 21:22:29 2026 +0200

    fix(pr-management-code-review): name adversarial reviewers as prose in the review body (#1685)
    
    A finding flagged only by the adversarial-review tool carries its
    reviewers as a list, and the body printed it raw: "Flagged by ['codex']
    (adversarial review)". The attribution line now joins the names
    ("codex", "codex and copilot", "codex, copilot and gemini"), still accepts
    a single name, and falls back to "the adversarial reviewer" when none is
    given.
    
    Generated-by: Claude Opus 5
---
 .../src/pr_management/code_review/body.py          | 13 ++++++++--
 tools/pr-management/tests/code_review/test_body.py | 28 ++++++++++++++++++++++
 2 files changed, 39 insertions(+), 2 deletions(-)

diff --git a/tools/pr-management/src/pr_management/code_review/body.py b/tools/pr-management/src/pr_management/code_review/body.py
index e39494e1..3eeee094 100644
--- a/tools/pr-management/src/pr_management/code_review/body.py
+++ b/tools/pr-management/src/pr_management/code_review/body.py
@@ -165,6 +165,16 @@ def _where(finding: dict[str, Any]) -> str:
     return f"`{finding['file']}:{line}`" if line else f"`{finding.get('file') or 'general'}`"
 
 
+def _reviewer_names(reviewers: object) -> str:
+    """The adversarial reviewers as prose: a list from the tool path, or one name."""
+    if isinstance(reviewers, (list, tuple)):
+        names = [str(r) for r in reviewers if str(r).strip()]
+        if len(names) > 1:
+            return ", ".join(names[:-1]) + " and " + names[-1]
+        return names[0] if names else "the adversarial reviewer"
+    return str(reviewers) if reviewers else "the adversarial reviewer"
+
+
 def _block(finding: dict[str, Any], prefix: str) -> str:
     lines = [
         f"### {prefix}{finding.get('rule_id') or finding.get('category') or 'Finding'} ({_where(finding)})",
@@ -182,8 +192,7 @@ def _block(finding: dict[str, Any], prefix: str) -> str:
         lines += [
             "*Flagged by both the primary and adversarial reviewers.*"
             if finding["source"] == "both"
-            else f"*Flagged by {finding.get('reviewers') or 'the adversarial reviewer'} (adversarial review); "
-            "cross-checked.*",
+            else f"*Flagged by {_reviewer_names(finding.get('reviewers'))} (adversarial review); cross-checked.*",
             "",
         ]
     return "\n".join(lines).rstrip()
diff --git a/tools/pr-management/tests/code_review/test_body.py b/tools/pr-management/tests/code_review/test_body.py
index effd62dc..c7b13691 100644
--- a/tools/pr-management/tests/code_review/test_body.py
+++ b/tools/pr-management/tests/code_review/test_body.py
@@ -182,6 +182,34 @@ def test_compose_orders_sections_and_folds_dropped_inline_comments() -> None:
     assert body.verify_footer(text, "request-changes")["footer_present"]
 
 
+@pytest.mark.parametrize(
+    ("reviewers", "expected"),
+    [
+        (["codex"], "*Flagged by codex (adversarial review); cross-checked.*"),
+        (["codex", "copilot"], "*Flagged by codex and copilot (adversarial review); cross-checked.*"),
+        (
+            ["codex", "copilot", "gemini"],
+            "*Flagged by codex, copilot and gemini (adversarial review); cross-checked.*",
+        ),
+        ("codex", "*Flagged by codex (adversarial review); cross-checked.*"),
+        (None, "*Flagged by the adversarial reviewer (adversarial review); cross-checked.*"),
+        ([], "*Flagged by the adversarial reviewer (adversarial review); cross-checked.*"),
+    ],
+)
+def test_adversarial_attribution_names_the_reviewers_as_prose(reviewers: object, expected: str) -> None:
+    finding = {"file": "hook.py", "line": 12, "severity": "major", "rule_id": "r", "source": "adversarial"}
+    if reviewers is not None:
+        finding["reviewers"] = reviewers
+    text = body.compose(
+        summary="One finding.",
+        findings=[finding],
+        inline_kept=[],
+        footer_text=body.footer("request-changes", "Foo", URL),
+    )
+    assert expected in text
+    assert "['" not in text
+
+
 def test_inline_comments_and_payload() -> None:
     files = diff.parse(DIFF)
     findings = [