fix: dedup empty reviewer summary by PR state, not stale re_review flag (#1391)

A push that lands while a reviewer run is in flight is delivered as a
queued message into the still-running first-review run, whose
configurable still has re_review=False. The empty-review guard in
publish_review only skipped the 'No issues found' summary when
is_re_review was True, so the queued reconcile published a second,
duplicate top-level 'No issues found' review.

Key the empty-review skip off actual PR state instead: add
open_swe_review_exists(), which detects the marker render_review_body
embeds in every Open SWE review body, and skip the summary when a prior
Open SWE review already exists (regardless of the re_review flag). Fails
open on API error so a genuine first review is never suppressed.

agent/reviewer_publish.py

@@ -221,6 +221,15 @@ def render_inline_comment_payload(finding: Finding) -> dict[str, Any] | None:
     return payload
 
 
+def review_summary_marker(pr_number: int) -> str:
+    """The hidden marker embedded in every Open SWE review summary body.
+
+    Used both to stamp the summary (``render_review_body``) and to detect
+    (``open_swe_review_exists``) whether Open SWE has already reviewed a PR.
+    """
+    return f"<!-- open-swe-reviewer pr={pr_number} -->"
+
+
 def render_review_body(*, pr_number: int, surfaced_count: int, trace_url: str | None = None) -> str:
     """Compose the top-level review body."""
     if surfaced_count == 0:
@@ -235,10 +244,57 @@ def render_review_body(*, pr_number: int, surfaced_count: int, trace_url: str |
     parts = [headline]
     if trace_url:
         parts.append(f"[View Open SWE trace]({trace_url})")
-    parts.append(f"<!-- open-swe-reviewer pr={pr_number} -->")
+    parts.append(review_summary_marker(pr_number))
     return "\n\n".join(parts)
 
 
+async def open_swe_review_exists(
+    *,
+    owner: str,
+    repo: str,
+    pr_number: int,
+    token: str,
+) -> bool:
+    """Return True if Open SWE has already posted a review summary on this PR.
+
+    Detected via the ``review_summary_marker`` that ``render_review_body``
+    embeds in every Open SWE review body. The reviewer uses this to avoid
+    posting a duplicate "No issues found" summary when the ``re_review`` config
+    flag is stale — a push that lands mid-run is delivered as a queued message
+    into the still-running first-review run, whose configurable still says
+    ``re_review=False``, so the empty-review guard can't trust that flag alone.
+
+    On any API failure this returns False (fail open): the only consequence is
+    a possible duplicate summary, never a suppressed first review.
+    """
+    marker = review_summary_marker(pr_number)
+    url = f"{_GITHUB_API_BASE}/repos/{owner}/{repo}/pulls/{pr_number}/reviews"
+    headers = _github_headers(token)
+    params: dict[str, Any] = {"per_page": 100, "page": 1}
+    async with httpx.AsyncClient() as client:
+        while True:
+            try:
+                response = await client.get(url, headers=headers, params=params, timeout=30)
+                response.raise_for_status()
+            except httpx.HTTPError:
+                logger.exception(
+                    "Failed to list PR reviews for %s/%s#%s",
+                    owner,
+                    repo,
+                    pr_number,
+                )
+                return False
+            data = response.json()
+            if not isinstance(data, list) or not data:
+                return False
+            for review in data:
+                if isinstance(review, dict) and marker in (review.get("body") or ""):
+                    return True
+            if len(data) < 100:  # noqa: PLR2004
+                return False
+            params["page"] += 1
+
+
 async def post_pull_request_review(
     *,
     owner: str,

agent/tools/publish_review.py

@@ -27,6 +27,7 @@ from ..reviewer_publish import (
     fetch_pr_review_threads,
     fetch_review_comments,
     fetch_review_thread_id_for_comment,
+    open_swe_review_exists,
     parse_review_comment_marker,
     post_pull_request_review,
     render_inline_comment_payload,
@@ -235,12 +236,20 @@ async def _publish_review_async(
         inline_comments.append(payload)
         eligible_with_payload.append((dict(finding), payload))
 
-    # On re-review with nothing new to surface, skip the "no issues found"
+    # With nothing new to surface, skip the "no issues found" summary if Open
-    # comment — the user already saw the previous findings, and posting
+    # SWE has already reviewed this PR — the user already saw the previous
-    # another summary on every push is noise. Still resolve threads for
+    # result, and posting another summary on every push is noise. We can't rely
-    # findings that just moved to resolved, and advance last_reviewed_sha so
+    # on the static re_review flag alone: a push that lands mid-run is delivered
-    # subsequent pushes don't redo the same diff.
+    # as a queued message into the still-running first-review run, whose
-    if is_re_review and not inline_comments:
+    # configurable still says re_review=False, so that path would post a
+    # duplicate "No issues found". Key off the actual PR state (an existing Open
+    # SWE review summary) instead. Still resolve threads for findings that just
+    # moved to resolved, and advance last_reviewed_sha so subsequent pushes
+    # don't redo the same diff.
+    if not inline_comments and (
+        is_re_review
+        or await open_swe_review_exists(owner=owner, repo=repo, pr_number=pr_number, token=token)
+    ):
         resolved_thread_count = await _resolve_threads_for_resolved_findings(
             owner=owner,
             repo=repo,

tests/test_reviewer_publish.py

@@ -11,6 +11,7 @@ import pytest
 from agent.reviewer_findings import Finding, new_finding
 from agent.reviewer_publish import (
     fetch_pr_review_threads,
+    open_swe_review_exists,
     parse_review_comment_marker,
     post_pull_request_review,
     render_inline_comment_body,
@@ -19,6 +20,7 @@ from agent.reviewer_publish import (
     render_review_body,
     reply_to_review_comment,
     resolve_review_thread,
+    review_summary_marker,
 )
 
 
@@ -42,6 +44,7 @@ def _isolate_publish_review_pr_state() -> Iterator[None]:
     with (
         patch("agent.tools.publish_review.fetch_pr_review_threads", AsyncMock(return_value=[])),
         patch("agent.tools.publish_review.replace_findings", AsyncMock()),
+        patch("agent.tools.publish_review.open_swe_review_exists", AsyncMock(return_value=False)),
     ):
         yield
 
@@ -512,6 +515,135 @@ async def test_publish_review_skips_post_on_re_review_with_no_new_findings() ->
     assert result["skipped_empty_re_review"] is True
 
 
+@pytest.mark.asyncio
+async def test_publish_review_skips_duplicate_empty_summary_when_open_swe_already_reviewed() -> (
+    None
+):
+    """A push landing mid-run is queued into the still-running first-review run,
+    whose configurable still says re_review=False. With nothing to surface, the
+    empty-review guard must key off the existing Open SWE review summary on the
+    PR (not the stale flag) so it does not post a duplicate "No issues found"."""
+    from agent.tools.publish_review import _publish_review_async
+
+    post_review = AsyncMock()
+    set_metadata = AsyncMock()
+    resolve_threads = AsyncMock(return_value=0)
+    review_exists = AsyncMock(return_value=True)
+    slack_reply = AsyncMock()
+
+    with (
+        patch("agent.tools.publish_review.get_thread_id_from_runtime", return_value="tid"),
+        patch("agent.tools.publish_review.list_findings_async", AsyncMock(return_value=[])),
+        patch("agent.tools.publish_review.open_swe_review_exists", review_exists),
+        patch("agent.tools.publish_review.post_pull_request_review", post_review),
+        patch("agent.tools.publish_review._resolve_threads_for_resolved_findings", resolve_threads),
+        patch("agent.tools.publish_review.set_reviewer_thread_metadata", set_metadata),
+        patch("agent.tools.publish_review._maybe_post_slack_completion_reply", slack_reply),
+    ):
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="newsha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=False,
+        )
+
+    post_review.assert_not_called()
+    review_exists.assert_awaited_once()
+    resolve_threads.assert_awaited_once()
+    slack_reply.assert_not_called()
+    assert result["success"] is True
+    assert result["review_id"] is None
+    assert result["surfaced_count"] == 0
+    assert result["skipped_empty_re_review"] is True
+    set_metadata.assert_awaited_once_with("tid", last_reviewed_sha="newsha")
+
+
+@pytest.mark.asyncio
+async def test_publish_review_skips_review_existence_check_on_re_review() -> None:
+    """When re_review is already True we know a prior review exists, so the
+    empty-review guard must short-circuit without an extra reviews API call."""
+    from agent.tools.publish_review import _publish_review_async
+
+    review_exists = AsyncMock(return_value=True)
+
+    with (
+        patch("agent.tools.publish_review.get_thread_id_from_runtime", return_value="tid"),
+        patch("agent.tools.publish_review.list_findings_async", AsyncMock(return_value=[])),
+        patch("agent.tools.publish_review.open_swe_review_exists", review_exists),
+        patch("agent.tools.publish_review.post_pull_request_review", AsyncMock()) as post_review,
+        patch(
+            "agent.tools.publish_review._resolve_threads_for_resolved_findings",
+            new_callable=AsyncMock,
+            return_value=0,
+        ),
+        patch("agent.tools.publish_review.set_reviewer_thread_metadata", new_callable=AsyncMock),
+    ):
+        result = await _publish_review_async(
+            owner="o",
+            repo="r",
+            pr_number=7,
+            head_sha="newsha",
+            token="t",
+            severity_threshold="medium",
+            cap=15,
+            is_re_review=True,
+        )
+
+    review_exists.assert_not_called()
+    post_review.assert_not_called()
+    assert result["skipped_empty_re_review"] is True
+
+
+@pytest.mark.asyncio
+async def test_open_swe_review_exists_detects_summary_marker() -> None:
+    response = MagicMock()
+    response.json.return_value = [
+        {"id": 1, "body": "some human review"},
+        {"id": 2, "body": f"## ✅ Open SWE Review\n\n{review_summary_marker(7)}"},
+    ]
+    response.raise_for_status.return_value = None
+
+    client_cm = AsyncMock()
+    client_cm.__aenter__.return_value = client_cm
+    client_cm.get = AsyncMock(return_value=response)
+
+    with patch("agent.reviewer_publish.httpx.AsyncClient", return_value=client_cm):
+        exists = await open_swe_review_exists(owner="o", repo="r", pr_number=7, token="t")
+    assert exists is True
+
+
+@pytest.mark.asyncio
+async def test_open_swe_review_exists_false_without_marker() -> None:
+    response = MagicMock()
+    response.json.return_value = [{"id": 1, "body": "looks good to me"}]
+    response.raise_for_status.return_value = None
+
+    client_cm = AsyncMock()
+    client_cm.__aenter__.return_value = client_cm
+    client_cm.get = AsyncMock(return_value=response)
+
+    with patch("agent.reviewer_publish.httpx.AsyncClient", return_value=client_cm):
+        exists = await open_swe_review_exists(owner="o", repo="r", pr_number=7, token="t")
+    assert exists is False
+
+
+@pytest.mark.asyncio
+async def test_open_swe_review_exists_fails_open_on_http_error() -> None:
+    import httpx
+
+    client_cm = AsyncMock()
+    client_cm.__aenter__.return_value = client_cm
+    client_cm.get = AsyncMock(side_effect=httpx.HTTPError("boom"))
+
+    with patch("agent.reviewer_publish.httpx.AsyncClient", return_value=client_cm):
+        exists = await open_swe_review_exists(owner="o", repo="r", pr_number=7, token="t")
+    assert exists is False
+
+
 @pytest.mark.asyncio
 async def test_re_review_backfills_existing_marker_and_skips_duplicate_post() -> None:
     from agent.tools.publish_review import _publish_review_async