diff --git a/core/workflows/review_pr.py b/core/workflows/review_pr.py index 45a3e30..f57a007 100644 --- a/core/workflows/review_pr.py +++ b/core/workflows/review_pr.py @@ -523,6 +523,36 @@ def _reviewer_from_pr_assignee( return [] +def _reviewer_from_existing_review_request( + pr: Any, + *, + owner: str, + pr_author_login: str, +) -> list[str]: + for reviewer in get_field(pr, "requested_reviewers", []) or []: + if is_automation_user(reviewer): + continue + login = _normalize_reviewer_login( + get_login(reviewer), + pr_author_login=pr_author_login, + ) + if login: + logger.info( + "review-pr: using existing requested reviewer %s", + login, + ) + return [login] + for team in get_field(pr, "requested_teams", []) or []: + slug = str(get_field(team, "slug", "") or "").strip().lstrip("@") + if slug: + logger.info( + "review-pr: using existing requested team reviewer %s", + slug, + ) + return [f"{owner}/{slug}"] + return [] + + def _deterministic_reviewer_from_stakeholders( entries: list[dict[str, Any]], *, @@ -1366,10 +1396,16 @@ def apply_review_result( else "COMMENT" ) if is_non_member and verdict == _VERDICT_APPROVE: - recommended_reviewers = _reviewer_from_pr_assignee( + recommended_reviewers = _reviewer_from_existing_review_request( pr, + owner=owner, pr_author_login=pr_author_login, ) + if not recommended_reviewers: + recommended_reviewers = _reviewer_from_pr_assignee( + pr, + pr_author_login=pr_author_login, + ) if not recommended_reviewers: ownership_areas = [ OwnershipArea( diff --git a/tests/test_review_pr_reviewer_sampling.py b/tests/test_review_pr_reviewer_sampling.py index 87885a8..6fd5b3b 100644 --- a/tests/test_review_pr_reviewer_sampling.py +++ b/tests/test_review_pr_reviewer_sampling.py @@ -521,6 +521,75 @@ def test_approve_non_member_pr_prefers_existing_assignee(self) -> None: pr.create_review.assert_called_once() pr.create_review_request.assert_called_once_with(reviewers=["assigned-owner"]) + def test_approve_non_member_pr_prefers_existing_requested_reviewer(self) -> None: + pr = MagicMock() + pr.requested_reviewers = [SimpleNamespace(login="requested-owner")] + pr.assignees = [SimpleNamespace(login="assigned-owner")] + github = self._make_github(pr) + progress = MagicMock() + with patch("workflows.review_pr._resolve_recommended_reviewers") as resolve: + apply_review_result( + github, + context=self._make_context( + is_non_member=True, + ownership_areas=[ + { + "name": "Docs API", + "owners": ["api-owner"], + "matches": "API reference docs and generated documentation", + } + ], + ), + run=MagicMock(), + result={ + "verdict": "APPROVE", + "summary": "Looks good", + "comments": [], + "recommended_area": "Docs API", + }, + progress=progress, + ) + resolve.assert_not_called() + pr.create_review.assert_called_once() + pr.create_review_request.assert_called_once_with( + reviewers=["requested-owner"] + ) + + def test_approve_non_member_pr_prefers_existing_requested_team(self) -> None: + pr = MagicMock() + pr.requested_reviewers = [] + pr.requested_teams = [SimpleNamespace(slug="reviewers")] + pr.assignees = [SimpleNamespace(login="assigned-owner")] + github = self._make_github(pr) + progress = MagicMock() + with patch("workflows.review_pr._resolve_recommended_reviewers") as resolve: + apply_review_result( + github, + context=self._make_context( + is_non_member=True, + ownership_areas=[ + { + "name": "Docs API", + "owners": ["api-owner"], + "matches": "API reference docs and generated documentation", + } + ], + ), + run=MagicMock(), + result={ + "verdict": "APPROVE", + "summary": "Looks good", + "comments": [], + "recommended_area": "Docs API", + }, + progress=progress, + ) + resolve.assert_not_called() + pr.create_review.assert_called_once() + pr.create_review_request.assert_called_once_with( + team_reviewers=["reviewers"] + ) + def test_approve_member_pr_uses_comment_event_without_reviewer_request(self) -> None: pr = MagicMock() github = self._make_github(pr)