adam/open-swe
1
0
Fork 0
mirror of https://github.com/Sea-Haven-Industries/open-swe.git synced 2026-10-01 19:03:18 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions

fix: treat ready-for-review as reviewer re-review (#1336)

Browse source 
Avoid reposting empty initial review summaries when a previously reviewed PR leaves draft without changing its reviewed head.
This commit is contained in:
Johannes du Plessis 2026-05-26 18:16:11 -07:00 • committed by GitHub
parent bef6dff2dc
commit e2abaf3787
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 104 additions and 10 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

48
agent/webapp.py
View file

@ -1694,6 +1694,33 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou
repo_config.get("owner", ""), repo_config.get("name", ""), pr_number
)
pr_meta: ReviewerPRMeta = {
"owner": repo_config.get("owner", ""),
"name": repo_config.get("name", ""),
"number": pr_number,
"url": pr_url,
"title": pr_title,
"head_ref": branch_name,
"base_ref": base_ref,
}
last_reviewed_sha = ""
if payload.get("action") == "ready_for_review":
metadata = await _get_thread_metadata_safe(thread_id)
if metadata is not None and metadata.get("kind") == REVIEWER_THREAD_KIND:
existing_last_reviewed_sha = metadata.get("last_reviewed_sha")
if isinstance(existing_last_reviewed_sha, str) and existing_last_reviewed_sha:
if existing_last_reviewed_sha == head_sha:
await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True)
logger.info(
"Skipping ready_for_review auto-review for %s/%s#%s: "
"head_sha unchanged from last_reviewed_sha",
repo_config.get("owner"),
repo_config.get("name"),
pr_number,
)
return
last_reviewed_sha = existing_last_reviewed_sha
app_token, app_token_expires_at = await get_github_app_installation_token_with_expiry()
if not app_token:
logger.warning("No GitHub App token available for reviewer dispatch")
@ -1709,18 +1736,17 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou
logger.warning("Could not persist bot token for reviewer thread %s", thread_id)
return
pr_meta: ReviewerPRMeta = {
"owner": repo_config.get("owner", ""),
"name": repo_config.get("name", ""),
"number": pr_number,
"url": pr_url,
"title": pr_title,
"head_ref": branch_name,
"base_ref": base_ref,
}
await set_reviewer_thread_metadata(thread_id, pr=pr_meta, watch=True)
prompt = build_github_pr_review_prompt(repo_config, pr_number, pr_url, base_sha, head_sha)
is_re_review = bool(last_reviewed_sha)
if is_re_review:
prompt = (
f"PR #{pr_number} has been marked ready for review. The new HEAD is "
f"{head_sha}. Reconcile existing findings against the new diff, add any "
f"net-new findings, and call `publish_review` once you're done."
)
else:
prompt = build_github_pr_review_prompt(repo_config, pr_number, pr_url, base_sha, head_sha)
configurable = _build_reviewer_configurable(
source=source,
github_login=github_login,
@ -1731,6 +1757,8 @@ async def _dispatch_first_review_from_pr_payload(payload: dict[str, Any], *, sou
base_sha=base_sha,
head_sha=head_sha,
branch_name=branch_name,
re_review=is_re_review,
last_reviewed_sha=last_reviewed_sha,
)
thread_active = await is_thread_active(thread_id)

66
tests/test_pr_ready_auto_review.py
View file

@ -61,6 +61,7 @@ async def test_pr_ready_for_review_triggers_run(monkeypatch: pytest.MonkeyPatch)
fake_client = MagicMock()
fake_client.runs.create = AsyncMock()
_patch_dispatch_deps(monkeypatch, fake_client)
monkeypatch.setattr(webapp, "_get_thread_metadata_safe", AsyncMock(return_value=None))
monkeypatch.setattr(webapp, "get_profile", AsyncMock(return_value=None))
monkeypatch.setattr(webapp, "get_team_settings", AsyncMock(return_value={}))
@ -69,6 +70,71 @@ async def test_pr_ready_for_review_triggers_run(monkeypatch: pytest.MonkeyPatch)
fake_client.runs.create.assert_awaited_once()
@pytest.mark.asyncio
async def test_pr_ready_for_review_skips_when_head_already_reviewed(
monkeypatch: pytest.MonkeyPatch,
) -> None:
fake_client = MagicMock()
fake_client.runs.create = AsyncMock()
set_metadata = AsyncMock()
get_token = AsyncMock(return_value=("token", None))
monkeypatch.setattr(webapp, "get_github_app_installation_token_with_expiry", get_token)
monkeypatch.setattr(webapp, "set_reviewer_thread_metadata", set_metadata)
monkeypatch.setattr(
webapp,
"_get_thread_metadata_safe",
AsyncMock(
return_value={
"kind": "reviewer",
"watch": False,
"last_reviewed_sha": "headsha",
}
),
)
monkeypatch.setattr(webapp, "get_client", lambda url: fake_client)
monkeypatch.setattr(webapp, "get_profile", AsyncMock(return_value=None))
monkeypatch.setattr(webapp, "get_team_settings", AsyncMock(return_value={}))
await webapp.process_github_pr_ready(_pr_payload(action="ready_for_review", draft=False))
fake_client.runs.create.assert_not_called()
get_token.assert_not_awaited()
set_metadata.assert_awaited_once()
assert set_metadata.await_args.kwargs["watch"] is True
@pytest.mark.asyncio
async def test_pr_ready_for_review_uses_re_review_after_previous_review(
monkeypatch: pytest.MonkeyPatch,
) -> None:
fake_client = MagicMock()
fake_client.runs.create = AsyncMock()
_patch_dispatch_deps(monkeypatch, fake_client)
monkeypatch.setattr(
webapp,
"_get_thread_metadata_safe",
AsyncMock(
return_value={
"kind": "reviewer",
"watch": False,
"last_reviewed_sha": "oldsha",
}
),
)
monkeypatch.setattr(webapp, "get_profile", AsyncMock(return_value=None))
monkeypatch.setattr(webapp, "get_team_settings", AsyncMock(return_value={}))
await webapp.process_github_pr_ready(_pr_payload(action="ready_for_review", draft=False))
fake_client.runs.create.assert_awaited_once()
_, kwargs = fake_client.runs.create.await_args
configurable = kwargs["config"]["configurable"]
assert configurable["re_review"] is True
assert configurable["last_reviewed_sha"] == "oldsha"
assert configurable["head_sha"] == "headsha"
assert "marked ready for review" in kwargs["input"]["messages"][0]["content"]
@pytest.mark.asyncio
async def test_pr_ready_draft_user_override_off_wins_over_team_on(
monkeypatch: pytest.MonkeyPatch,