diff --git a/.github/scripts/pull-request-dashboard/RATIONALE.md b/.github/scripts/pull-request-dashboard/RATIONALE.md index 3e7252bb4acd..6fc416561f12 100644 --- a/.github/scripts/pull-request-dashboard/RATIONALE.md +++ b/.github/scripts/pull-request-dashboard/RATIONALE.md @@ -530,6 +530,10 @@ the implementation understandable and operationally cheap. stale review is the ordinary state between a push and the next re-review, and an icon that is always present says nothing about which PRs are actually waiting. +- Copilot appears in the reviewers column only for unresolved inline feedback + or a pending review that holds the PR. It reviews nearly every PR, so completed + participation alone adds noise rather than identifying work that remains. + This is a display filter only; review state and routing still use all feedback. - An effective reviewer-routing override is a break-glass handoff. It bypasses required checks, Copilot review, merge conflicts, and discussion actions across later pushes. The approval threshold determines whether it waits on reviewers diff --git a/.github/scripts/pull-request-dashboard/render.py b/.github/scripts/pull-request-dashboard/render.py index 7793c5597a1c..134a6ad690a5 100644 --- a/.github/scripts/pull-request-dashboard/render.py +++ b/.github/scripts/pull-request-dashboard/render.py @@ -185,10 +185,15 @@ def copilot_review_pending(facts: DashboardFacts) -> bool: def display_reviewers(facts: DashboardFacts) -> list[ReviewerSummary]: - # Copilot only joins the reviewer list once it has reviewed, so a review - # that is still in flight has to be added for the wait to be visible at all. - reviewers = list(facts.reviewers) - if not copilot_review_pending(facts): + copilot_pending = copilot_review_pending(facts) + reviewers = [ + reviewer + for reviewer in facts.reviewers + if not is_copilot_reviewer_login(reviewer.login) + or reviewer.unresolved_thread + or copilot_pending + ] + if not copilot_pending: return reviewers for reviewer in reviewers: if is_copilot_reviewer_login(reviewer.login): diff --git a/.github/scripts/pull-request-dashboard/test_render.py b/.github/scripts/pull-request-dashboard/test_render.py index e9d3c9395b24..d2abd75d6eed 100644 --- a/.github/scripts/pull-request-dashboard/test_render.py +++ b/.github/scripts/pull-request-dashboard/test_render.py @@ -86,6 +86,57 @@ def test_human_rereview_keeps_the_changes_requested_badge(self) -> None: self.assertEqual("reviewer ⏳\u2060🔴", cell) + def test_copilot_without_unresolved_feedback_is_hidden(self) -> None: + for login in ( + "copilot", + "Copilot", + "copilot[bot]", + "copilot-pull-request-reviewer", + "copilot-pull-request-reviewer[bot]", + ): + for status in ( + {}, + {"approved": True}, + {"approved_non_team": True}, + {"changes_requested": True}, + {"top_level_feedback": True}, + {"pending_review": True}, + ): + with self.subTest(login=login, status=status): + cell = reviewers_cell_text(dashboard_facts( + reviewers=[ + {"login": login, **status}, + {"login": "reviewer"}, + ], + copilot_review_exists=True, + )) + + self.assertEqual("reviewer", cell) + + def test_copilot_with_unresolved_feedback_remains_visible(self) -> None: + for login in ( + "copilot", + "Copilot", + "copilot[bot]", + "copilot-pull-request-reviewer", + "copilot-pull-request-reviewer[bot]", + ): + with self.subTest(login=login): + cell = reviewers_cell_text(dashboard_facts( + reviewers=[{"login": login, "unresolved_thread": True}], + )) + + self.assertEqual("Copilot 💬", cell) + + def test_copilot_is_hidden_after_its_last_thread_is_resolved(self) -> None: + cell = reviewers_cell_text(dashboard_facts( + reviewers=[{"login": "copilot-pull-request-reviewer"}], + copilot_review_exists=True, + copilot_review_outstanding=True, + )) + + self.assertEqual("", cell) + def test_requested_copilot_review_is_listed_as_pending(self) -> None: cell = reviewers_cell_text(dashboard_facts( reviewers=[{"login": "reviewer", "approved": True}], @@ -95,12 +146,25 @@ def test_requested_copilot_review_is_listed_as_pending(self) -> None: self.assertEqual("Copilot ⏳
reviewer ✅", cell) + def test_requested_copilot_review_keeps_its_entry_without_open_feedback(self) -> None: + cell = reviewers_cell_text(dashboard_facts( + reviewers=[{"login": "Copilot"}], + copilot_review_exists=True, + copilot_review_outstanding=True, + copilot_review_requested=True, + )) + + self.assertEqual("Copilot ⏳", cell) + def test_requested_copilot_review_on_an_ungated_branch_is_not_pending(self) -> None: # Nothing holds the pull request there, and a requested human reviewer # who has not responded is left off the row, so Copilot gets no row # either. The gate is what makes the wait someone's turn. cell = reviewers_cell_text(dashboard_facts( - reviewers=[{"login": "reviewer", "approved": True}], + reviewers=[ + {"login": "copilot-pull-request-reviewer"}, + {"login": "reviewer", "approved": True}, + ], copilot_review_outstanding=False, copilot_review_requested=True, )) diff --git a/pull-request-dashboard/README.md b/pull-request-dashboard/README.md index 2ace005f80f0..3ec680fdd226 100644 --- a/pull-request-dashboard/README.md +++ b/pull-request-dashboard/README.md @@ -108,6 +108,7 @@ The dashboard groups open non-draft pull requests by who is expected to act next - 🔴 requested changes - ⏳ a review is in flight (a human reviewer other than the effective author was requested again after reviewing, or a Copilot review is pending while it holds the PR) - Icons combine when multiple states apply. For example, 💬📌 means the reviewer has unresolved inline feedback and top-level feedback that still needs author action; ✅ may accompany either or both. + - Copilot appears only with unresolved inline feedback (💬) or a pending review that holds the PR (⏳). Completed reviews alone do not add Copilot to the column. - **CI** — Aggregate check status across the PR's required status checks. Optional checks do not affect this column: - ✅ all required checks passing - ⏳ at least one required check pending, none failing