Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/reviewer-bot-tests/test_main.py
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,7 @@ def test_main_schedule_backfills_existing_transition_notice_without_duplicate_co
monkeypatch.setattr(reviewer_bot, "sync_members_with_queue", lambda current: (current, []))
monkeypatch.setattr(reviewer_bot.maintenance_module, "sweep_deferred_gaps", lambda bot, current: False)
monkeypatch.setattr(reviewer_bot.maintenance_module, "maybe_record_head_observation_repair", lambda bot, issue_number, review_data: False)
monkeypatch.setattr(reviewer_bot, "get_pull_request_reviews", lambda issue_number: [])
monkeypatch.setattr(reviewer_bot, "get_issue_or_pr_snapshot", lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []})
monkeypatch.setattr(reviewer_bot, "save_state", lambda current: True)
monkeypatch.setattr(reviewer_bot, "sync_status_labels_for_items", lambda current, issue_numbers: True)
Expand Down
152 changes: 152 additions & 0 deletions .github/reviewer-bot-tests/test_reviewer_bot.py
Original file line number Diff line number Diff line change
Expand Up @@ -396,6 +396,7 @@ def test_scheduled_check_backfills_transition_notice_without_reposting(monkeypat
review["transition_warning_sent"] = "2026-03-10T00:00:00Z"
monkeypatch.setattr(reviewer_bot.maintenance_module, "sweep_deferred_gaps", lambda bot, state: False)
monkeypatch.setattr(reviewer_bot.maintenance_module, "maybe_record_head_observation_repair", lambda bot, issue_number, review_data: False)
monkeypatch.setattr(reviewer_bot, "get_pull_request_reviews", lambda issue_number: [])
monkeypatch.setattr(reviewer_bot, "get_issue_or_pr_snapshot", lambda issue_number: {"pull_request": {}})
posted = []
monkeypatch.setattr(reviewer_bot, "post_comment", lambda issue_number, body: posted.append(body) or True)
Expand All @@ -418,6 +419,36 @@ def fake_api(method, endpoint, data=None):
assert posted == []


def test_scheduled_check_repairs_missing_reviewer_review_state(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
assert review is not None
review["current_reviewer"] = "alice"
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
monkeypatch.setattr(reviewer_bot.maintenance_module, "sweep_deferred_gaps", lambda bot, state: False)
monkeypatch.setattr(reviewer_bot.maintenance_module, "maybe_record_head_observation_repair", lambda bot, issue_number, review_data: False)
monkeypatch.setattr(reviewer_bot.maintenance_module, "check_overdue_reviews", lambda bot, state: [])
monkeypatch.setattr(reviewer_bot, "get_issue_or_pr_snapshot", lambda issue_number: {"pull_request": {}})
monkeypatch.setattr(
reviewer_bot,
"get_pull_request_reviews",
lambda issue_number: [
{
"id": 10,
"state": "COMMENTED",
"submitted_at": "2026-03-17T10:01:00Z",
"commit_id": "head-1",
"user": {"login": "alice"},
}
],
)
assert reviewer_bot.handle_scheduled_check(state) is True
accepted = review["reviewer_review"]["accepted"]
assert accepted is not None
assert accepted["semantic_key"] == "pull_request_review:10"
assert review["last_reviewer_activity"] == "2026-03-17T10:01:00Z"


def test_issue_edit_by_author_records_contributor_freshness(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
Expand Down Expand Up @@ -474,6 +505,127 @@ def test_project_status_labels_uses_commit_id_and_comment_freshness(monkeypatch)
assert metadata["reason"] == "review_head_stale"


def test_project_status_labels_uses_live_current_reviewer_review_when_channel_state_missing(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
assert review is not None
review["current_reviewer"] = "alice"
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
monkeypatch.setattr(
reviewer_bot,
"get_issue_or_pr_snapshot",
lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []},
)
monkeypatch.setattr(
reviewer_bot,
"github_api",
lambda method, endpoint, data=None: {"head": {"sha": "head-1"}} if endpoint == "pulls/42" else None,
)
monkeypatch.setattr(
reviewer_bot,
"get_pull_request_reviews",
lambda issue_number: [
{
"id": 10,
"state": "COMMENTED",
"submitted_at": "2026-03-17T10:01:00Z",
"commit_id": "head-1",
"user": {"login": "alice"},
}
],
)
desired_labels, metadata = reviewer_bot.project_status_labels_for_item(42, state)
assert desired_labels == {reviewer_bot.STATUS_AWAITING_CONTRIBUTOR_RESPONSE_LABEL}
assert metadata["reason"] == "completion_missing"


def test_project_status_labels_uses_live_review_fallback_for_stale_head(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
assert review is not None
review["current_reviewer"] = "alice"
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
monkeypatch.setattr(
reviewer_bot,
"get_issue_or_pr_snapshot",
lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []},
)
monkeypatch.setattr(
reviewer_bot,
"github_api",
lambda method, endpoint, data=None: {"head": {"sha": "head-2"}} if endpoint == "pulls/42" else None,
)
monkeypatch.setattr(
reviewer_bot,
"get_pull_request_reviews",
lambda issue_number: [
{
"id": 10,
"state": "COMMENTED",
"submitted_at": "2026-03-17T10:01:00Z",
"commit_id": "head-1",
"user": {"login": "alice"},
}
],
)
desired_labels, metadata = reviewer_bot.project_status_labels_for_item(42, state)
assert desired_labels == {reviewer_bot.STATUS_AWAITING_REVIEWER_RESPONSE_LABEL}
assert metadata["reason"] == "review_head_stale"


def test_project_status_labels_prefers_newer_contributor_comment_over_live_review_fallback(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
assert review is not None
review["current_reviewer"] = "alice"
review["active_cycle_started_at"] = "2026-03-17T09:00:00Z"
reviewer_bot.reviews_module.accept_channel_event(
review,
"contributor_comment",
semantic_key="issue_comment:20",
timestamp="2026-03-17T10:05:00Z",
actor="bob",
)
monkeypatch.setattr(
reviewer_bot,
"get_issue_or_pr_snapshot",
lambda issue_number: {"number": issue_number, "state": "open", "pull_request": {}, "labels": []},
)
monkeypatch.setattr(
reviewer_bot,
"github_api",
lambda method, endpoint, data=None: {"head": {"sha": "head-1"}} if endpoint == "pulls/42" else None,
)
monkeypatch.setattr(
reviewer_bot,
"get_pull_request_reviews",
lambda issue_number: [
{
"id": 10,
"state": "COMMENTED",
"submitted_at": "2026-03-17T10:01:00Z",
"commit_id": "head-1",
"user": {"login": "alice"},
}
],
)
desired_labels, metadata = reviewer_bot.project_status_labels_for_item(42, state)
assert desired_labels == {reviewer_bot.STATUS_AWAITING_REVIEWER_RESPONSE_LABEL}
assert metadata["reason"] == "contributor_comment_newer"


def test_record_reviewer_activity_does_not_regress_timestamp_on_legacy_backfill():
review = reviewer_bot.ensure_review_entry(make_state(), 42, create=True)
assert review is not None
review["last_reviewer_activity"] = "2026-03-20T10:00:00Z"
review["transition_warning_sent"] = "2026-03-21T10:00:00Z"
review["transition_notice_sent_at"] = "2026-03-22T10:00:00Z"
reviewer_bot.reviews_module.record_reviewer_activity(review, "2026-03-18T10:00:00Z")
assert review["last_reviewer_activity"] == "2026-03-20T10:00:00Z"
assert review["transition_warning_sent"] is None
assert review["transition_notice_sent_at"] is None


def test_project_status_labels_emits_awaiting_write_approval_only_after_completion(monkeypatch):
state = make_state()
review = reviewer_bot.ensure_review_entry(state, 42, create=True)
Expand Down
2 changes: 2 additions & 0 deletions scripts/reviewer_bot_lib/maintenance.py
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,8 @@ def handle_scheduled_check(bot, state: dict) -> bool:
issue_snapshot = bot.get_issue_or_pr_snapshot(issue_number)
if not isinstance(issue_snapshot, dict) or not isinstance(issue_snapshot.get("pull_request"), dict):
continue
if bot.reviews_module.repair_missing_reviewer_review_state(bot, issue_number, review_data):
changed = True
if maybe_record_head_observation_repair(bot, issue_number, review_data):
changed = True
overdue_reviews = check_overdue_reviews(bot, state)
Expand Down
36 changes: 10 additions & 26 deletions scripts/reviewer_bot_lib/reconcile.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,11 @@
_record_conversation_freshness,
classify_comment_payload,
)
from .reviews import find_triage_approval_after, get_latest_review_by_reviewer
from .reviews import (
accept_reviewer_review_from_live_review,
find_triage_approval_after,
get_latest_valid_current_reviewer_review_for_cycle,
)


def _now_iso(bot) -> str:
Expand Down Expand Up @@ -53,20 +57,10 @@ def _record_review_rebuild(bot, state: dict, issue_number: int, review_data: dic
completion, _ = bot.reviews_module.rebuild_pr_approval_state(bot, issue_number, review_data, pull_request=pull_request, reviews=reviews)
if completion is None:
raise RuntimeError(f"Unable to rebuild approval state for PR #{issue_number}")
latest = get_latest_review_by_reviewer(bot, reviews, str(review_data.get("current_reviewer", "")))
latest = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews)
if latest is not None:
commit_id = latest.get("commit_id")
submitted_at = latest.get("submitted_at")
if isinstance(commit_id, str) and isinstance(submitted_at, str):
bot.reviews_module.accept_channel_event(
review_data,
"reviewer_review",
semantic_key=f"pull_request_review:{latest.get('id')}",
timestamp=submitted_at,
actor=review_data.get("current_reviewer"),
reviewed_head_sha=commit_id,
source_precedence=1,
)
if accept_reviewer_review_from_live_review(review_data, latest, actor=review_data.get("current_reviewer")) and isinstance(submitted_at, str):
bot.reviews_module.record_reviewer_activity(review_data, submitted_at)
return bool(completion.get("completed"))

Expand All @@ -93,24 +87,14 @@ def reconcile_active_review_entry(
reviews = bot.get_pull_request_reviews(issue_number)
if reviews is None:
return f"❌ Failed to fetch reviews for PR #{issue_number}; cannot run `/rectify`.", False, False
latest_review = get_latest_review_by_reviewer(bot, reviews, assigned_reviewer)
latest_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews)
messages: list[str] = []
if latest_review is not None:
latest_state = str(latest_review.get("state", "")).upper()
commit_id = latest_review.get("commit_id")
submitted_at = latest_review.get("submitted_at")
if latest_state in {"APPROVED", "COMMENTED", "CHANGES_REQUESTED"} and isinstance(commit_id, str) and isinstance(submitted_at, str):
state_changed = bot.reviews_module.accept_channel_event(
review_data,
"reviewer_review",
semantic_key=f"pull_request_review:{latest_review.get('id')}",
timestamp=submitted_at,
actor=assigned_reviewer,
reviewed_head_sha=commit_id,
source_precedence=1,
) or state_changed
bot.reviews_module.record_reviewer_activity(review_data, submitted_at)
if accept_reviewer_review_from_live_review(review_data, latest_review, actor=assigned_reviewer) and isinstance(submitted_at, str):
state_changed = True
bot.reviews_module.record_reviewer_activity(review_data, submitted_at)
messages.append(f"latest review by @{assigned_reviewer} is `{latest_state}`")
if _record_review_rebuild(bot, state, issue_number, review_data):
state_changed = True
Expand Down
108 changes: 106 additions & 2 deletions scripts/reviewer_bot_lib/reviews.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,99 @@ def get_latest_review_by_reviewer(bot, reviews: list[dict], reviewer: str) -> di
return latest_review


def get_latest_valid_current_reviewer_review_for_cycle(
bot,
issue_number: int,
review_data: dict,
*,
reviews: list[dict] | None = None,
) -> dict | None:
current_reviewer = review_data.get("current_reviewer")
if not isinstance(current_reviewer, str) or not current_reviewer.strip():
return None
boundary = get_current_cycle_boundary(bot, review_data)
if boundary is None:
return None
if reviews is None:
reviews = bot.get_pull_request_reviews(issue_number)
if reviews is None:
return None
latest_review = None
latest_key = (datetime.min.replace(tzinfo=timezone.utc), "")
for review in reviews:
if not isinstance(review, dict):
continue
author = review.get("user", {}).get("login")
if not isinstance(author, str) or author.lower() != current_reviewer.lower():
continue
state = str(review.get("state", "")).upper()
if state not in {"APPROVED", "COMMENTED", "CHANGES_REQUESTED"}:
continue
submitted_at = parse_github_timestamp(review.get("submitted_at"))
if submitted_at is None or submitted_at < boundary:
continue
commit_id = review.get("commit_id")
if not isinstance(commit_id, str) or not commit_id.strip():
continue
review_id = str(review.get("id", ""))
review_key = (submitted_at, review_id)
if review_key >= latest_key:
latest_key = review_key
latest_review = review
return latest_review


def build_reviewer_review_record_from_live_review(review: dict, *, actor: str | None = None) -> dict | None:
if not isinstance(review, dict):
return None
review_id = review.get("id")
submitted_at = review.get("submitted_at")
commit_id = review.get("commit_id")
author = actor if isinstance(actor, str) and actor.strip() else review.get("user", {}).get("login")
if not isinstance(review_id, int) or not isinstance(submitted_at, str) or not isinstance(commit_id, str):
return None
if not isinstance(author, str) or not author.strip():
return None
return {
"semantic_key": f"pull_request_review:{review_id}",
"timestamp": submitted_at,
"actor": author,
"reviewed_head_sha": commit_id,
"source_precedence": 1,
"payload": {},
}


def accept_reviewer_review_from_live_review(review_data: dict, review: dict, *, actor: str | None = None) -> bool:
record = build_reviewer_review_record_from_live_review(review, actor=actor)
if record is None:
return False
return accept_channel_event(
review_data,
"reviewer_review",
semantic_key=record["semantic_key"],
timestamp=record["timestamp"],
actor=record["actor"],
reviewed_head_sha=record["reviewed_head_sha"],
source_precedence=record["source_precedence"],
payload=record["payload"],
)


def repair_missing_reviewer_review_state(bot, issue_number: int, review_data: dict, *, reviews: list[dict] | None = None) -> bool:
reviewer_review = review_data.get("reviewer_review", {}).get("accepted")
if isinstance(reviewer_review, dict):
return False
live_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data, reviews=reviews)
if live_review is None:
return False
submitted_at = live_review.get("submitted_at")
changed = accept_reviewer_review_from_live_review(review_data, live_review, actor=review_data.get("current_reviewer"))
if isinstance(submitted_at, str):
record_reviewer_activity(review_data, submitted_at)
return changed


def find_triage_approval_after(bot, reviews: list[dict], since: datetime | None) -> tuple[str, datetime] | None:
permission_cache: dict[str, bool] = {}
approvals: list[tuple[datetime, str, str]] = []
Expand Down Expand Up @@ -180,7 +273,10 @@ def clear_transition_timers(review_data: dict) -> None:


def record_reviewer_activity(review_data: dict, timestamp: str) -> None:
review_data["last_reviewer_activity"] = timestamp
current = parse_github_timestamp(review_data.get("last_reviewer_activity"))
candidate = parse_github_timestamp(timestamp)
if current is None or candidate is None or candidate >= current:
review_data["last_reviewer_activity"] = timestamp
clear_transition_timers(review_data)


Expand Down Expand Up @@ -554,6 +650,11 @@ def project_status_labels_for_item(
reviewer_review = review_data.get("reviewer_review", {}).get("accepted")
contributor_comment = review_data.get("contributor_comment", {}).get("accepted")

if not reviewer_comment and not reviewer_review and is_pr:
live_review = get_latest_valid_current_reviewer_review_for_cycle(bot, issue_number, review_data)
if live_review is not None:
reviewer_review = build_reviewer_review_record_from_live_review(live_review, actor=current_reviewer)

if not reviewer_comment and not reviewer_review:
return ({STATUS_AWAITING_REVIEWER_RESPONSE_LABEL}, {"state": "awaiting_reviewer_response", "reason": "no_reviewer_activity"})

Expand All @@ -572,7 +673,10 @@ def project_status_labels_for_item(
if not isinstance(latest_review_head, str) or latest_review_head != current_head:
return ({STATUS_AWAITING_REVIEWER_RESPONSE_LABEL}, {"state": "awaiting_reviewer_response", "reason": "review_head_stale"})

if _compare_cross_channel_conversation(contributor_comment, reviewer_comment) > 0:
latest_reviewer_response = reviewer_comment
if _compare_records(reviewer_review, latest_reviewer_response) > 0:
latest_reviewer_response = reviewer_review
if _compare_cross_channel_conversation(contributor_comment, latest_reviewer_response) > 0:
return ({STATUS_AWAITING_REVIEWER_RESPONSE_LABEL}, {"state": "awaiting_reviewer_response", "reason": "contributor_comment_newer"})

if is_pr:
Expand Down
Loading
Loading