diff --git a/scripts/pr_review.py b/scripts/pr_review.py index fc182598..3c056035 100755 --- a/scripts/pr_review.py +++ b/scripts/pr_review.py @@ -189,6 +189,7 @@ ) from pr_review_rounds import ( ACK_MARKER, + CARRIED_PARTIAL, CODERABBIT_LOGIN, FILES_UNREAD, KNOWN_REVIEWERS, @@ -994,12 +995,21 @@ def report_verdict(pr: dict) -> int: return 0 state, why = coverage(pr) if state == "partial": - print( - f"status=COVERAGE_IS_PARTIAL the review covering the head states it read fewer files " - f"than the pull request changed ({why}), so part of the diff has no review at all. A " - "re-request is not the remedy it reads as. Splitting is the remedy where it applies, " - "and it does not apply to a promotion. Otherwise this is the maintainer's call" - ) + if why.startswith(CARRIED_PARTIAL): + print( + f"status=COVERAGE_IS_PARTIAL the head reads as partial because {why}, so part of " + "the diff may have no review. A round on the head stating full coverage clears " + "this, unless a round on it already states a reading this cannot believe. " + "Otherwise this is the maintainer's call" + ) + else: + print( + f"status=COVERAGE_IS_PARTIAL the review covering the head states it read fewer " + f"files than the pull request changed ({why}), so part of the diff has no review " + "at all. A re-request is not the remedy it reads as. Splitting is the remedy where " + "it applies, and it does not apply to a promotion. Otherwise this is the " + "maintainer's call" + ) return 42 if flagged := uncounted_verdict(pr): print( diff --git a/scripts/pr_review_rounds.py b/scripts/pr_review_rounds.py index 8788670e..6275758c 100644 --- a/scripts/pr_review_rounds.py +++ b/scripts/pr_review_rounds.py @@ -31,6 +31,7 @@ ) TRUSTED_ASSOCIATIONS = frozenset({"OWNER", "MEMBER", "COLLABORATOR"}) ACK_MARKER = "" +CARRIED_PARTIAL = "an earlier round states partial coverage" def reviewer_requested(pr: dict) -> bool: @@ -437,7 +438,7 @@ def coverage(pr: dict) -> tuple[str, str]: if worst else "no round on the head states any" ) - return "partial", f"an earlier round states partial coverage, and {none}" + return "partial", f"{CARRIED_PARTIAL}, and {none}" if worst: return worst why = "" diff --git a/tests/test_pr_review.py b/tests/test_pr_review.py index 7f96954b..a31a9cdb 100755 --- a/tests/test_pr_review.py +++ b/tests/test_pr_review.py @@ -1989,12 +1989,27 @@ def test_a_head_round_stating_partial_coverage_exits_forty_two(self) -> None: self.assertEqual(42, self.status(payload([review(body=part)], files=["a.py"]))) out = self.out.getvalue() self.assertIn("coverage=PARTIAL ", out) - self.assertIn("status=COVERAGE_IS_PARTIAL", out) + self.assertIn("status=COVERAGE_IS_PARTIAL the review covering the head states it", out) + self.assertNotIn("may have no review", out) def test_an_earlier_partial_carries_to_a_head_stating_none(self) -> None: part = f"{OVERVIEW}\n" rounds = [review(oid=OLD, body=part, rid="A"), review(body=OVERVIEW, rid="B")] self.assertEqual(42, self.status(payload(rounds, files=["a.py"]))) + line = next(ln for ln in self.out.getvalue().splitlines() if ln.startswith("status=")) + self.assertIn("because an earlier round states partial coverage", line) + self.assertNotIn("the review covering the head states", line) + self.assertIn("A round on the head stating full coverage clears this", line) + self.assertNotIn("A re-request is not the remedy", line) + + def test_an_earlier_partial_carried_past_an_unbelievable_head_says_so(self) -> None: + part = f"{OVERVIEW}\n" + odd = f"{OVERVIEW}\n" + rounds = [review(oid=OLD, body=part, rid="A"), review(body=odd, rid="B")] + self.assertEqual(42, self.status(payload(rounds, files=["a.py"]))) + line = next(ln for ln in self.out.getvalue().splitlines() if ln.startswith("status=")) + self.assertIn("a round on the head states a reading this cannot believe", line) + self.assertNotIn("the review covering the head states", line) def test_a_prose_partial_on_an_earlier_round_keeps_a_table_from_standing_in(self) -> None: part = f"{OVERVIEW}\nCopilot reviewed 1 out of 2 changed files.\n"