Count the Unread Files Rather Than Assuming There Is One

scripts/pr_review.py

@@ -617,15 +617,29 @@ def report_verdict(pr: dict) -> int:
               f'is hosted there and the fix lands there. Merging this pull request anyway is the '
               f'maintainer\'s decision to take and not this script\'s, and not the agent\'s.')
         return 43
-    state, _ = head_coverage(pr)
+    state, line = head_coverage(pr)
     if state == PARTIAL:
-        print('status=COVERAGE_IS_PARTIAL the review covering the head read fewer files than the '
-              'pull request changed, so part of the diff has no review at all. Every partial on '
-              'record stayed partial at the identical ratio across every later round, so a '
-              're-request is not the remedy it reads as, and the reviewer names no file list in '
-              'these rounds, so which file went unread cannot be read from the API. Splitting is '
-              'the remedy where it applies, and it does not apply to a promotion. Otherwise this '
-              'is the maintainer\'s call, taken knowing one file of the diff has no review')
+        # The count comes from the line that decided PARTIAL, not from what past rounds skipped.
+        # Every partial measured here skipped exactly one file, across seven rounds.
+        # That is a measurement of those rounds rather than a property the state carries.
+        # Asserting it would be a claim this script cannot check on the run it prints for.
+        # None is unreachable while PARTIAL is set only where this same line parsed.
+        # It is narrowed rather than indexed, since the alternative is a crash on the gate's path.
+        # A crash there reads as this script being broken rather than as a round that read part.
+        counts = read_coverage(line)
+        if counts is None:
+            gap = 'how much of the diff went unread, which could not be re-read from that line'
+        else:
+            unread = counts[1] - counts[0]
+            gap = (f'{unread} of the {counts[1]} changed files '
+                   f'{"has" if unread == 1 else "have"} no review')
+        print(f'status=COVERAGE_IS_PARTIAL the review covering the head read fewer files than the '
+              f'pull request changed, so part of the diff has no review at all. Every partial on '
+              f'record stayed partial at the identical ratio across every later round, so a '
+              f're-request is not the remedy it reads as, and the reviewer names no file list in '
+              f'these rounds, so which file went unread cannot be read from the API. Splitting is '
+              f'the remedy where it applies, and it does not apply to a promotion. Otherwise this '
+              f'is the maintainer\'s call, taken knowing {gap}')
         return 42
     return 0
 

scripts/test_pr_review.py

@@ -870,6 +870,33 @@ def test_status_exits_forty_two_on_a_round_that_read_part_of_the_diff(self) -> N
         self.assertEqual(42, pr_review.main(['status', '7', '--repo', 'o/r']))
         self.assertIn('status=COVERAGE_IS_PARTIAL', self.out.getvalue())
 
+    def test_the_partial_message_counts_the_unread_files_rather_than_assuming_one(self) -> None:
+        """Every partial on record skipped exactly one file, which is a measurement of seven
+        rounds rather than a property the state carries, so the line reads the run it prints."""
+        for reviewed, changed, phrase in ((2, 3, '1 of the 3 changed files has'),
+                                          (5, 9, '4 of the 9 changed files have')):
+            with self.subTest(reviewed=reviewed, changed=changed):
+                out = self.enterContext(contextlib.redirect_stdout(io.StringIO()))
+                self.answer(payload([review(
+                    body=OVERVIEW + f'\nCopilot reviewed {reviewed} out of {changed} changed '
+                                    f'files in this pull request and generated no comments.')]))
+                self.assertEqual(42, pr_review.main(['status', '7', '--repo', 'o/r']))
+                self.assertIn(phrase, out.getvalue())
+
+    def test_the_partial_message_still_blocks_where_the_counts_cannot_be_re_read(self) -> None:
+        """Unreachable by construction, since PARTIAL is set only where that line parsed, so the
+        counts are withheld from `report_verdict` directly. A crash on the blocking path would
+        read to a caller as this script being broken rather than as a round that read part."""
+        pr = payload([self.partial()])
+        unparseable = 'a coverage line carrying no counts at all'
+        with mock.patch.object(pr_review, 'head_coverage',
+                               return_value=(pr_review.PARTIAL, unparseable)):
+            self.assertEqual(42, pr_review.report_verdict(pr))
+        out = self.out.getvalue()
+        self.assertIn('status=COVERAGE_IS_PARTIAL', out)
+        self.assertIn('could not be re-read', out)
+        self.assertNotIn('changed files', out)
+
     def test_status_exits_forty_three_on_a_wording_it_does_not_read(self) -> None:
         self.answer(payload([review(body=OVERVIEW +
                                      '\nCopilot reviewed some of the changed files.')]))