fix: close reviewer feedback handoff gaps

Author: PLeVasseur

REVIEWING.md

@@ -74,6 +74,19 @@ Use this to release your assignment from an issue/PR without automatically assig
 @guidelines-bot /release Need to focus on other priorities
 ```
 
+### Record Feedback for Contributor Follow-Up
+
+```
+@guidelines-bot /feedback
+```
+
+Use this after you have provided reviewer feedback and are waiting for the contributor to respond. This keeps reviewer-bot's review state aligned without marking the review complete or changing the assigned reviewer.
+
+**Example:**
+```
+@guidelines-bot /feedback
+```
+
 ### Rectify Review State
 
 ```
@@ -200,6 +213,7 @@ Life happens! Any of these actions will reset the 14-day clock:
 - **Post a review comment** - Any substantive feedback counts
 - **Use `/pass [reason]`** - Pass the review to the next person if you can't review it
 - **Use `/away YYYY-MM-DD [reason]`** - Step away temporarily (e.g., "On vacation until 2025-02-15")
+- **Use `/feedback`** - Record that reviewer feedback is ready for contributor follow-up
 - **Use `/rectify`** - Reconcile PR review state when review activity happened but bot state is stale
 
 #### Before You Pass: Consider the Learning Opportunity

scripts/reviewer_bot_core/review_state_machine.py

@@ -165,6 +165,13 @@ def _reset_cycle_state(review_data: dict) -> None:
     review_data["overdue_anchor"] = None
 
 
+def clear_current_cycle_reviewer_handoff(review_data: dict) -> bool:
+    if review_data.get("current_cycle_reviewer_handoff") is None:
+        return False
+    review_data["current_cycle_reviewer_handoff"] = None
+    return True
+
+
 def set_current_reviewer(
     state: dict,
     issue_number: int,
@@ -205,9 +212,7 @@ def clear_current_reviewer(state: dict, issue_number: int) -> bool:
     if review_data.get("assignment_method") is not None:
         review_data["assignment_method"] = None
         changed = True
-    if review_data.get("current_cycle_reviewer_handoff") is not None:
-        review_data["current_cycle_reviewer_handoff"] = None
-        changed = True
+    changed = clear_current_cycle_reviewer_handoff(review_data) or changed
     clear_transition_timers(review_data)
     return changed
 

scripts/reviewer_bot_lib/commands.py

@@ -170,9 +170,6 @@ def handle_pass_command(
     reviewer_authority: dict[str, object] | None = None,
 ) -> tuple[str, bool]:
     assignment_request = request or build_assignment_request(bot, issue_number=issue_number)
-    issue_data = review_state.ensure_review_entry(state, issue_number, create=True)
-    if issue_data is None:
-        return "❌ Unable to load review state.", False
     authority = reviewer_authority or assignment_flow.resolve_reviewer_command_authority(
         bot,
         state,
@@ -181,6 +178,9 @@ def handle_pass_command(
     )
     if not authority.get("authorized"):
         return _reviewer_command_authority_error("pass", authority), False
+    issue_data = authority.get("review_data")
+    if not isinstance(issue_data, dict):
+        return "❌ Unable to load review state.", False
     passed_reviewer = str(authority["tracked_reviewer"])
     current_assignees = list(authority.get("live_control_plane_reviewers") or [])
     skipped = list(issue_data["skipped"])

scripts/reviewer_bot_lib/comment_application.py

@@ -8,6 +8,7 @@
 from scripts.reviewer_bot_core import (
     comment_command_policy,
     comment_freshness_policy,
+    live_review_support,
     privileged_command_policy,
 )
 
@@ -262,6 +263,26 @@ def _execute_rectify(bot, state: dict, request: CommentEventRequest, decision) -
     )
 
 
+def _handoff_order_key(handoff: dict) -> tuple[object, str] | None:
+    timestamp = live_review_support.parse_github_timestamp(handoff.get("timestamp"))
+    if timestamp is None:
+        return None
+    source_event_key = handoff.get("source_event_key")
+    return timestamp, source_event_key if isinstance(source_event_key, str) else ""
+
+
+def _feedback_handoff_should_replace(existing: object, candidate: dict) -> bool:
+    if not isinstance(existing, dict):
+        return True
+    candidate_key = _handoff_order_key(candidate)
+    existing_key = _handoff_order_key(existing)
+    if candidate_key is None:
+        return existing_key is None
+    if existing_key is None:
+        return True
+    return candidate_key >= existing_key
+
+
 def handle_feedback_command(bot, state: dict, request: CommentEventRequest, decision) -> CommandExecutionResult:
     if request.issue_state.lower() != "open":
         return CommandExecutionResult(
@@ -302,6 +323,12 @@ def handle_feedback_command(bot, state: dict, request: CommentEventRequest, deci
         "reviewed_head_sha": reviewed_head_sha,
     }
     before = review_data.get("current_cycle_reviewer_handoff")
+    if not _feedback_handoff_should_replace(before, handoff):
+        return CommandExecutionResult(
+            response="ℹ️ Ignored stale `/feedback` handoff because a newer reviewer handoff is already recorded.",
+            success=True,
+            state_changed=False,
+        )
     review_data["current_cycle_reviewer_handoff"] = handoff
     activity_changed = record_reviewer_activity(review_data, request.comment_created_at)
     state_changed = before != handoff or activity_changed
@@ -370,11 +397,11 @@ def apply_comment_command(
         return False
 
     issue_number = request.issue_number
-    review_data = ensure_review_entry(state, issue_number, create=True)
-    if review_data is None:
-        return False
     source_event_key = request.comment_source_event_key or f"issue_comment:{request.comment_id}"
     if isinstance(decision, comment_command_policy.DeferPrivilegedHandoffDecision):
+        review_data = ensure_review_entry(state, issue_number, create=True)
+        if review_data is None:
+            return False
         permission_status = bot.github.get_user_permission_status(request.comment_author, "triage")
         handoff = privileged_command_policy.validate_accept_no_fls_changes_handoff(
             request,

scripts/reviewer_bot_lib/lifecycle.py

@@ -13,6 +13,7 @@
 from .config import CODING_GUIDELINE_LABEL, TRANSITION_NOTICE_MARKER_PREFIX
 from .review_state import (
     accept_channel_event,
+    clear_current_cycle_reviewer_handoff,
     ensure_review_entry,
     mark_review_complete,
     record_transition_notice_sent,
@@ -439,6 +440,9 @@ def handle_pull_request_target_synchronize(bot, state: dict) -> bool:
     previous_review_completed_by = review_data.get("review_completed_by")
     previous_review_completion_source = review_data.get("review_completion_source")
     review_data["active_head_sha"] = head_sha
+    handoff_changed = False
+    if previous_head_sha != head_sha:
+        handoff_changed = clear_current_cycle_reviewer_handoff(review_data)
     timestamp = request.event_created_at
     changed = accept_channel_event(
         review_data,
@@ -456,7 +460,7 @@ def handle_pull_request_target_synchronize(bot, state: dict) -> bool:
         or previous_review_completed_by != review_data.get("review_completed_by")
         or previous_review_completion_source != review_data.get("review_completion_source")
     )
-    return changed or previous_head_sha != review_data.get("active_head_sha") or approval_changed
+    return changed or previous_head_sha != review_data.get("active_head_sha") or approval_changed or handoff_changed
 
 
 def maybe_record_head_observation_repair(bot, issue_number: int, review_data: dict) -> HeadObservationRepairResult:
@@ -521,6 +525,7 @@ def maybe_record_head_observation_repair(bot, issue_number: int, review_data: di
     contributor_revision = review_data.get("contributor_revision", {}).get("accepted")
     if isinstance(contributor_revision, dict) and contributor_revision.get("reviewed_head_sha") == head_sha:
         review_data["active_head_sha"] = head_sha
+        clear_current_cycle_reviewer_handoff(review_data)
         return HeadObservationRepairResult(changed=True, outcome="changed")
     changed = accept_channel_event(
         review_data,
@@ -531,6 +536,7 @@ def maybe_record_head_observation_repair(bot, issue_number: int, review_data: di
         source_precedence=0,
     )
     review_data["active_head_sha"] = head_sha
+    clear_current_cycle_reviewer_handoff(review_data)
     review_data["current_cycle_completion"] = {}
     review_data["current_cycle_write_approval"] = {}
     review_data["review_completed_at"] = None

scripts/reviewer_bot_lib/review_state.py

@@ -51,6 +51,10 @@ def clear_current_reviewer(state: dict, issue_number: int) -> bool:
     return review_state_machine.clear_current_reviewer(state, issue_number)
 
 
+def clear_current_cycle_reviewer_handoff(review_data: dict) -> bool:
+    return review_state_machine.clear_current_cycle_reviewer_handoff(review_data)
+
+
 def semantic_key_seen(review_data: dict, channel_name: str, semantic_key: str) -> bool:
     return review_state_machine.semantic_key_seen(review_data, channel_name, semantic_key)
 

scripts/reviewer_bot_lib/reviews.py

@@ -117,7 +117,10 @@ def apply_pr_approval_state(
     write_approval: dict,
     current_head_sha: str,
 ) -> None:
+    previous_head_sha = review_data.get("active_head_sha")
     review_data["active_head_sha"] = current_head_sha
+    if previous_head_sha != current_head_sha:
+        review_state.clear_current_cycle_reviewer_handoff(review_data)
     review_data["current_cycle_completion"] = completion
     review_data["current_cycle_write_approval"] = write_approval
     if completion.get("completed"):

tests/contract/reviewer_bot/test_review_state_contract.py

@@ -175,6 +175,7 @@ def test_review_state_mutation_inventory_freezes_overlap_classification_and_live
         "- set_current_reviewer: local-state-only mutation",
         "- update_reviewer_activity: local-state-only mutation",
         "- mark_review_complete: local-state-only mutation",
+        "- clear_current_cycle_reviewer_handoff: local-state-only mutation",
         "- get_current_cycle_boundary: read-only helper",
         "- clear_transition_timers: local-state-only mutation",
         "- semantic_key_seen: read-only helper",
@@ -269,6 +270,7 @@ def test_review_state_module_exposes_named_mutation_surface():
         "set_current_reviewer",
         "update_reviewer_activity",
         "mark_review_complete",
+        "clear_current_cycle_reviewer_handoff",
         "get_current_cycle_boundary",
         ]:
         assert hasattr(review_state, name)

tests/fixtures/equivalence/review_state/api_inventory.md

@@ -7,6 +7,7 @@ Local-State-Only Mutation APIs
 - update_reviewer_activity: local-state-only mutation
 - mark_review_complete: local-state-only mutation
 - clear_transition_timers: local-state-only mutation
+- clear_current_cycle_reviewer_handoff: local-state-only mutation
 
 Live-Read-Assisted Mutation APIs
 - accept_reviewer_review_from_live_review: live-read-assisted mutation; C1c in-scope

tests/unit/reviewer_bot/test_commands.py

@@ -825,6 +825,80 @@ def test_feedback_command_rejects_triage_actor_who_is_not_current_reviewer(monke
     assert side_effects.reactions == [(100, "eyes")]
 
 
+def test_feedback_command_does_not_materialize_review_entry_when_no_active_review(monkeypatch):
+    harness = CommandHarness(monkeypatch)
+    state = make_state()
+    harness.stub_assignees([])
+    side_effects = harness.capture_comment_side_effects()
+    request = harness.typed_comment_request(
+        issue_number=42,
+        actor="alice",
+        body="@guidelines-bot /feedback",
+        issue_author="dana",
+        is_pull_request=False,
+    )
+
+    changed = comment_application.apply_comment_command(
+        harness.runtime,
+        state,
+        request,
+        {"command": "feedback", "args": [], "command_count": 1},
+        classify_issue_comment_actor=lambda current_request: "repo_user_principal",
+    )
+
+    assert changed is False
+    assert "42" not in state["active_reviews"]
+    assert side_effects.comments == [(42, "❌ No active tracked review exists for this issue/PR.")]
+    assert side_effects.reactions == [(100, "eyes")]
+
+
+def test_feedback_command_does_not_overwrite_newer_handoff(monkeypatch):
+    harness = CommandHarness(monkeypatch)
+    state = make_state()
+    review = review_state.ensure_review_entry(state, 42, create=True)
+    assert review is not None
+    review["current_reviewer"] = "alice"
+    review["current_cycle_reviewer_handoff"] = {
+        "source_event_key": "issue_comment:101",
+        "timestamp": "2026-03-17T11:00:00Z",
+        "actor": "alice",
+        "command_name": "feedback",
+        "reviewed_head_sha": None,
+    }
+    harness.stub_assignees(["alice"])
+    side_effects = harness.capture_comment_side_effects()
+    request = harness.typed_comment_request(
+        issue_number=42,
+        actor="alice",
+        body="@guidelines-bot /feedback",
+        issue_author="dana",
+        is_pull_request=False,
+        comment_id=100,
+        created_at="2026-03-17T10:00:00Z",
+    )
+
+    changed = comment_application.apply_comment_command(
+        harness.runtime,
+        state,
+        request,
+        {"command": "feedback", "args": [], "command_count": 1},
+        classify_issue_comment_actor=lambda current_request: "repo_user_principal",
+    )
+
+    assert changed is False
+    assert review["current_cycle_reviewer_handoff"] == {
+        "source_event_key": "issue_comment:101",
+        "timestamp": "2026-03-17T11:00:00Z",
+        "actor": "alice",
+        "command_name": "feedback",
+        "reviewed_head_sha": None,
+    }
+    assert side_effects.comments == [
+        (42, "ℹ️ Ignored stale `/feedback` handoff because a newer reviewer handoff is already recorded.")
+    ]
+    assert side_effects.reactions == [(100, "eyes"), (100, "+1")]
+
+
 def test_feedback_command_plus_text_does_not_record_reviewer_comment_channel(monkeypatch):
     harness = CommentRoutingHarness(monkeypatch)
     state = make_state()