Classify top-level reviewer feedback with the author-action binary

.github/scripts/pull-request-dashboard/classification.py

@@ -167,6 +167,57 @@
 never affects any other item."""
 
 
+REVIEWER_FEEDBACK_PROMPT_TEMPLATE = (
+    """You are triaging top-level feedback items from pull request reviewers.
+
+"""
+    + BATCH_CONTRACT
+    + """
+
+Each item contains the reviewer's login in `requester`, the PR author's login in
+`pr_author`, and the comment text in `body`. First-person statements in `body`
+are the reviewer speaking, never the PR author.
+
+Question: does this item leave something unresolved that `pr_author` must handle
+before this pull request can merge?
+
+  - author_action: anything the author would answer or act on, including
+    questions, requests, objections, remarks that reject the pull request's
+    premise or necessity without asking for anything, an answer to a question
+    the author asked, and a statement that this pull request is blocked on
+    another pull request, release, or decision
+  - no_author_action: the item needs nothing from the PR author, such as pure
+    approval, thanks, a status summary, or a repository automation command (for
+    example "/workflow-approve", "/rerun", or "/easycla")
+
+Read the whole item before deciding. Approval is no_author_action however it is
+phrased ("LGTM", "I'm fine with the API changes", "looks good to me, feel free
+to merge"), and stays no_author_action when it carries a suggestion the reviewer
+explicitly leaves for later ("we can clean this up post submission", "an
+opportunity to refactor after a point fix release", "left one small
+maintainability comment").
+
+Compare every login and team mentioned in `body` against `pr_author`. An item
+asking a different person or team to review, decide, or weigh in is
+no_author_action even when it describes a concern with this pull request.
+
+Do not decide whether the author already responded. That is determined later
+from comment timestamps.
+
+When you cannot tell, answer author_action: ambiguity keeps the item with the
+author.
+
+Respond with a single JSON object and nothing else. Include exactly one result
+for every input discussion_id and copy each discussion_id exactly:
+{{"items": [{{"discussion_id": "input id", "verdict": "author_action" | "no_author_action", "reason": "short explanation grounded in this item"}}]}}
+
+---BEGIN TOP-LEVEL FEEDBACK---
+{discussions}
+---END TOP-LEVEL FEEDBACK---
+"""
+)
+
+
 PRAISE_PROMPT_TEMPLATE = (
     """You are triaging single comments left by reviewers on pull requests.
 
@@ -241,6 +292,7 @@
 TOP_LEVEL_DISCUSSION_ACTIONS = ("author", "none", "unclear")
 # Each binary lists its fail-safe verdict first: an unreadable answer keeps the
 # item with the author rather than handing the pull request to reviewers.
+REVIEWER_FEEDBACK_VERDICTS = ("author_action", "no_author_action")
 AUTHOR_REPLY_VERDICTS = ("deferral", "complete")
 PRAISE_VERDICTS = ("not_praise", "praise")
 
@@ -915,6 +967,7 @@ def review_thread_author_reply_input(discussion: dict[str, Any]) -> dict[str, An
 
 
 REVIEW_THREAD_REPLY_ACTIONS = {"deferral": "author", "complete": "reviewer"}
+REVIEWER_FEEDBACK_ACTIONS = {"author_action": "author", "no_author_action": "none"}
 PRAISE_ACTIONS = {"praise": "none", "not_praise": "author"}
 # Pure praise is short. The longest in a 441 pull request corpus was 13 characters,
 # so this is wide headroom, and anything longer stays the author's without a call.
@@ -929,7 +982,7 @@ def _could_be_praise(discussion: dict[str, Any]) -> bool:
     return len(" ".join((comments[-1].get("body") or "").split())) <= PRAISE_MAX_CHARS
 
 
-def _thread_record(record: dict[str, Any], actions: dict[str, str]) -> dict[str, Any]:
+def _verdict_record(record: dict[str, Any], actions: dict[str, str]) -> dict[str, Any]:
     """Restate a binary's verdict as the discussion action the dashboard routes on.
 
     A failed call keeps the thread with its author whatever verdict it still parsed.
@@ -1003,7 +1056,7 @@ def classify_review_threads(
     # a praise call that failed keeps its failure rather than becoming a clean
     # deterministic answer, and routes to the author like any other unusable verdict
     failed_praise: dict[str, dict[str, Any]] = {
-        discussion_id: _thread_record(record, PRAISE_ACTIONS)
+        discussion_id: _verdict_record(record, PRAISE_ACTIONS)
         for discussion_id, record in praise.items()
         if record.get("failed")
     }
@@ -1047,7 +1100,7 @@ def classify_review_threads(
         prompt_input=review_thread_author_reply_input,
     )
     for discussion_id, record in replies.items():
-        classifications_by_id[discussion_id] = _thread_record(record, REVIEW_THREAD_REPLY_ACTIONS)
+        classifications_by_id[discussion_id] = _verdict_record(record, REVIEW_THREAD_REPLY_ACTIONS)
     for discussion_id, since in since_by_id.items():
         if since and discussion_id in classifications_by_id:
             classifications_by_id[discussion_id]["since"] = since
@@ -1308,6 +1361,43 @@ def run_llm_for_verdict_batch(
     return records
 
 
+def classify_reviewer_feedback(
+    number: int,
+    discussions: list[dict[str, Any]],
+    model: str,
+    cache_in: dict[str, dict[str, Any]],
+    cache_out: dict[str, dict[str, Any]],
+) -> dict[str, dict[str, Any]]:
+    records = classify_top_level_items(
+        number,
+        discussions,
+        model,
+        cache_in,
+        cache_out,
+        prompt_template=REVIEWER_FEEDBACK_PROMPT_TEMPLATE,
+        prompt_input=top_level_reviewer_feedback_prompt_input,
+        run_batch=lambda batch, m: [
+            record
+            for items, prompt in verdict_prompt_batches(
+                batch,
+                REVIEWER_FEEDBACK_PROMPT_TEMPLATE,
+                top_level_reviewer_feedback_prompt_input,
+            )
+            for record in run_llm_for_verdict_batch(
+                items, m, prompt, REVIEWER_FEEDBACK_VERDICTS
+            )
+        ],
+        fallback_decision=lambda reason: unclear_verdict_decision(
+            reason, REVIEWER_FEEDBACK_VERDICTS
+        ),
+        warning_label="reviewer_feedback",
+    )
+    return {
+        discussion_id: _verdict_record(record, REVIEWER_FEEDBACK_ACTIONS)
+        for discussion_id, record in records.items()
+    }
+
+
 def classify_author_replies(
     number: int,
     discussions: list[dict[str, Any]],
@@ -1378,7 +1468,7 @@ def classify_discussion_domains(
     review_thread_classifications = classify_review_threads(
         number, review_threads, model, cache_in, cache_out
     )
-    top_level_classifications = classify_top_level_reviewer_feedback_items(
+    top_level_classifications = classify_reviewer_feedback(
         number, top_level_items, model, cache_in, cache_out
     )
     top_level_author_comment_classifications = classify_top_level_author_comments(

.github/scripts/pull-request-dashboard/eval/score_reviewer_feedback.py

@@ -39,12 +39,7 @@
     "reviewer_feedback": (
         "REVIEWER_FEEDBACK_PROMPT_TEMPLATE",
         ("verdict",),
-        {"substantive": "substantive", "noise": "noise"},
-    ),
-    "reviewer_feedback_confirm": (
-        "REVIEWER_FEEDBACK_CONFIRM_PROMPT_TEMPLATE",
-        ("verdict",),
-        {"other": "substantive", "confirmed": "noise"},
+        {"author_action": "substantive", "no_author_action": "noise"},
     ),
 }
 

.github/scripts/pull-request-dashboard/test_top_level_actions.py

@@ -22,6 +22,7 @@
 )
 from classification import (
     PRAISE_VERDICTS,
+    REVIEWER_FEEDBACK_VERDICTS,
     classify_discussion_domains,
     classify_review_threads,
     parse_discussion_decision,
@@ -67,6 +68,15 @@ def review_thread_discussion(discussion_id: str) -> dict:
     }
 
 
+def verdict_record(discussion_id: str, verdict: str = "author_action") -> dict:
+    return {
+        "discussion_id": discussion_id,
+        "discussion_kind": "top-level-feedback",
+        "failed": False,
+        "decision": {"verdict": verdict, "reason": "action requested"},
+    }
+
+
 def classification(discussion_id: str) -> dict:
     return {
         "discussion_id": discussion_id,
@@ -999,27 +1009,20 @@ def test_author_comment_batch_rejects_cross_discussion_feedback_key(
 
     @patch("classification.save_classification_cache")
     @patch("classification.load_classification_cache", return_value={})
-    @patch("classification.run_llm_for_top_level_reviewer_feedback_batch")
     @patch("classification.run_llm_for_verdict_batch")
     def test_a_thread_the_author_has_not_answered_needs_no_model(
         self,
-        run_inline,
         run_batch,
         _load_cache,
         save_cache,
     ) -> None:
-        run_batch.side_effect = lambda discussions, _model: [
-            {
-                "discussion_id": discussion["discussion_id"],
-                "discussion_kind": discussion["discussion_kind"],
-                "failed": False,
-                "decision": {
-                    "discussion_action": "author",
-                    "reason": "Reviewer asked a question",
-                },
-            }
-            for discussion in discussions
-        ]
+        asked = []
+
+        def batch(items, _model, _prompt, verdicts):
+            asked.append(verdicts)
+            return [verdict_record(item["discussion_id"]) for item in items]
+
+        run_batch.side_effect = batch
         thread = review_thread_discussion("inline")
         thread["discussion_facts"] = {"latest_comment_role": "reviewer"}
         # a real reviewer comment, too long to be a praise candidate, so no binary runs
@@ -1037,15 +1040,14 @@ def test_a_thread_the_author_has_not_answered_needs_no_model(
             classify_feedback_domains(123, [thread], [top_level_item("top-level")], "model")
         )
 
-        run_inline.assert_not_called()
+        self.assertEqual([REVIEWER_FEEDBACK_VERDICTS], asked)
         self.assertEqual(
             review_thread_classifications[0]["decision"]["discussion_action"], "author"
         )
         self.assertEqual(
             [record["discussion_id"] for record in top_level_classifications],
             ["top-level"],
         )
-
     @patch("classification.save_classification_cache")
     @patch("classification.load_classification_cache", return_value={})
     @patch("classification.run_llm_for_top_level_reviewer_feedback_batch", return_value=[])
@@ -1247,7 +1249,7 @@ def test_author_reply_expanded_prompt_calls_are_bounded(
 
     @patch("classification.save_classification_cache")
     @patch("classification.load_classification_cache", return_value={})
-    @patch("classification.run_llm_for_top_level_reviewer_feedback_batch")
+    @patch("classification.run_llm_for_verdict_batch")
     def test_later_run_classifies_only_failed_top_level_item(
         self,
         run_batch,
@@ -1257,15 +1259,12 @@ def test_later_run_classifies_only_failed_top_level_item(
         valid = top_level_item("valid")
         missing = top_level_item("missing")
         run_batch.return_value = [
-            classification("valid"),
+            verdict_record("valid"),
             {
                 "discussion_id": "missing",
                 "discussion_kind": "top-level-feedback",
                 "failed": True,
-                "decision": {
-                    "discussion_action": "unclear",
-                    "reason": "Missing result",
-                },
+                "decision": {"verdict": "author_action", "reason": "Missing result"},
             },
         ]
 
@@ -1275,7 +1274,7 @@ def test_later_run_classifies_only_failed_top_level_item(
         self.assertEqual(len(cached), 1)
         load_cache.return_value = cached
         run_batch.reset_mock()
-        run_batch.return_value = [classification("missing")]
+        run_batch.return_value = [verdict_record("missing")]
 
         classify_feedback_domains(123, [], [valid, missing], "model")
 
@@ -1286,16 +1285,15 @@ def test_later_run_classifies_only_failed_top_level_item(
 
     @patch("classification.save_classification_cache")
     @patch("classification.load_classification_cache", return_value={})
-    @patch("classification.run_llm_for_top_level_reviewer_feedback_batch")
+    @patch("classification.run_llm_for_verdict_batch")
     def test_top_level_cache_ignores_mutable_facts_but_includes_body(
         self,
         run_batch,
         load_cache,
         save_cache,
     ) -> None:
-        run_batch.side_effect = lambda discussions, _model: [
-            classification(discussion["discussion_id"])
-            for discussion in discussions
+        run_batch.side_effect = lambda items, _m, _p, _v: [
+            verdict_record(item["discussion_id"]) for item in items
         ]
         discussion = top_level_item("top-level")
         discussion["comments"] = [{"body": "Could you clarify this?"}]
@@ -1319,24 +1317,15 @@ def test_top_level_cache_ignores_mutable_facts_but_includes_body(
     @patch("classification.TOP_LEVEL_CLASSIFICATION_BATCH_SIZE", 10)
     @patch("classification.save_classification_cache")
     @patch("classification.load_classification_cache", return_value={})
-    @patch("classification.run_llm_for_top_level_reviewer_feedback_batch")
+    @patch("classification.run_llm_for_verdict_batch")
     def test_uncached_top_level_classification_is_batched_and_bounded(
         self,
         run_batch,
         _load_cache,
         save_cache,
     ) -> None:
-        run_batch.side_effect = lambda discussions, _model: [
-            {
-                "discussion_id": discussion["discussion_id"],
-                "discussion_kind": discussion["discussion_kind"],
-                "failed": False,
-                "decision": {
-                    "discussion_action": "none",
-                    "reason": "No action",
-                },
-            }
-            for discussion in discussions
+        run_batch.side_effect = lambda items, _m, _p, _v: [
+            verdict_record(item["discussion_id"], "no_author_action") for item in items
         ]
         discussions = [top_level_item(f"item-{index}") for index in range(23)]
 
@@ -1349,7 +1338,7 @@ def test_uncached_top_level_classification_is_batched_and_bounded(
         self.assertEqual(len(classifications), 23)
         self.assertEqual(
             [record["decision"]["discussion_action"] for record in classifications],
-            ["none"] * 20 + ["unclear"] * 3,
+            ["none"] * 20 + ["author"] * 3,
         )
         self.assertEqual(
             [bool(record.get("failed")) for record in classifications],
@@ -1395,7 +1384,7 @@ def test_over_limit_changes_requested_fails_instead_of_guessing(
         self.assertEqual(
             classifications[0]["decision"],
             {
-                "discussion_action": "unclear",
+                "discussion_action": "author",
                 "reason": "Exceeded per-PR classification limit",
             },
         )