Skip to content

Commit 2c711a0

Browse files
committed
fix: correct pr264 follow-up scope fallback
1 parent 2eeb056 commit 2c711a0

4 files changed

Lines changed: 57 additions & 72 deletions

File tree

scripts/reviewer_bot_core/reviewer_response_policy.py

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,19 @@ def _initial_cycle_boundary(review_data: dict) -> tuple[str | None, str | None]:
6363
return None, None
6464

6565

66+
def _alternate_current_head_cycle_boundary(review_data: dict, issue_snapshot: dict | None) -> str | None:
67+
if not isinstance(issue_snapshot, dict) or not isinstance(issue_snapshot.get("pull_request"), dict):
68+
return None
69+
if review_data.get("assignment_method") != "claim":
70+
return None
71+
for field in ("active_cycle_started_at", "cycle_started_at"):
72+
value = review_data.get(field)
73+
if isinstance(value, str) and value:
74+
return None
75+
created_at = issue_snapshot.get("created_at")
76+
return created_at if isinstance(created_at, str) and created_at else None
77+
78+
6679
def _scope_basis_and_anchor(review_data: dict, contributor_handoff: dict | None) -> tuple[str | None, str | None]:
6780
if isinstance(contributor_handoff, dict):
6881
semantic_key = str(contributor_handoff.get("semantic_key", ""))
@@ -97,8 +110,11 @@ def _current_scope_fields(
97110
contributor_handoff: dict | None,
98111
*,
99112
alternate_current_head_approval: bool = False,
113+
alternate_current_head_cycle_boundary: str | None = None,
100114
) -> dict[str, object]:
101115
_, cycle_boundary = _initial_cycle_boundary(review_data)
116+
if alternate_current_head_approval and alternate_current_head_cycle_boundary:
117+
cycle_boundary = alternate_current_head_cycle_boundary
102118
if alternate_current_head_approval:
103119
anchor_timestamp = None
104120
basis = "alternate_current_head_approval"
@@ -202,6 +218,7 @@ def derive_reviewer_response_state(
202218
approval_result: dict[str, object] | None = None,
203219
current_head_approval_authors: tuple[str, ...] | None = None,
204220
stored_reviewer_review: dict | None | object = _UNSET,
221+
alternate_current_head_cycle_boundary: str | None = None,
205222
) -> dict[str, object]:
206223
current_reviewer = review_data.get("current_reviewer")
207224
if not isinstance(current_reviewer, str) or not current_reviewer.strip():
@@ -379,6 +396,7 @@ def derive_reviewer_response_state(
379396
current_head,
380397
contributor_handoff,
381398
alternate_current_head_approval=True,
399+
alternate_current_head_cycle_boundary=alternate_current_head_cycle_boundary,
382400
),
383401
anchor_timestamp=latest_reviewer_response.get("timestamp") if isinstance(latest_reviewer_response, dict) else None,
384402
current_head_sha=current_head,
@@ -573,6 +591,7 @@ def compute_reviewer_response_state(
573591
reviews,
574592
parse_timestamp=bot.parse_iso8601_timestamp,
575593
)
594+
alternate_current_head_cycle_boundary = _alternate_current_head_cycle_boundary(review_data, issue_snapshot)
576595

577596
return derive_reviewer_response_state(
578597
review_data,
@@ -585,4 +604,5 @@ def compute_reviewer_response_state(
585604
approval_result=approval_result,
586605
current_head_approval_authors=approval_authors,
587606
stored_reviewer_review=stored_reviewer_review,
607+
alternate_current_head_cycle_boundary=alternate_current_head_cycle_boundary,
588608
)

scripts/reviewer_bot_lib/overdue.py

Lines changed: 2 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -210,32 +210,13 @@ def _transport_result(bot, *, failure_kind: str | None, status_code: int | None
210210
)
211211

212212

213-
def _normalize_review_data_scope_cycle(review_data: dict, issue_snapshot: dict | None) -> dict:
214-
if not isinstance(review_data, dict):
215-
return {}
216-
if not isinstance(issue_snapshot, dict) or not isinstance(issue_snapshot.get("pull_request"), dict):
217-
return review_data
218-
if not isinstance(review_data.get("current_reviewer"), str) or not review_data.get("current_reviewer"):
219-
return review_data
220-
if any(isinstance(review_data.get(field), str) and review_data.get(field) for field in ("active_cycle_started_at", "cycle_started_at")):
221-
return review_data
222-
if review_data.get("assignment_method") != "claim":
223-
return review_data
224-
created_at = issue_snapshot.get("created_at")
225-
if not isinstance(created_at, str) or not created_at:
226-
return review_data
227-
# Claim-era PR rows can miss explicit cycle timestamps; use PR creation to keep same-scope dedupe stable.
228-
return {**review_data, "active_cycle_started_at": created_at}
229-
230-
231213
def evaluate_overdue_review_preview(bot, state: dict, issue_number: int) -> dict[str, object]:
232214
active_reviews = state.get("active_reviews") if isinstance(state, dict) else None
233215
review_data = active_reviews.get(str(issue_number)) if isinstance(active_reviews, dict) else None
234216
if not isinstance(review_data, dict):
235217
review_data = {}
236218
issue_snapshot_result = bot.github.get_issue_or_pr_snapshot_result(issue_number)
237219
issue_snapshot = issue_snapshot_result.payload if issue_snapshot_result.ok and isinstance(issue_snapshot_result.payload, dict) else None
238-
normalized_review_data = _normalize_review_data_scope_cycle(review_data, issue_snapshot)
239220
is_pull_request = isinstance((issue_snapshot or {}).get("pull_request"), dict)
240221
authority = assignment_flow.resolve_reviewer_authority(
241222
bot,
@@ -245,7 +226,7 @@ def evaluate_overdue_review_preview(bot, state: dict, issue_number: int) -> dict
245226
)
246227
response_state = bot.adapters.review_state.compute_reviewer_response_state(
247228
issue_number,
248-
normalized_review_data,
229+
review_data,
249230
issue_snapshot=issue_snapshot,
250231
)
251232
response_name = str(response_state.get("response_state") or response_state.get("state") or "projection_failed")
@@ -351,7 +332,6 @@ def check_overdue_reviews(bot, state: dict) -> list[dict]:
351332
_clear_transport_failure(bot, review_data, issue_number, phase="issue_snapshot_read")
352333
if str(issue_snapshot.get("state", "")).lower() == "closed":
353334
continue
354-
normalized_review_data = _normalize_review_data_scope_cycle(review_data, issue_snapshot)
355335
authority = assignment_flow.resolve_reviewer_authority(
356336
bot,
357337
issue_number,
@@ -386,7 +366,7 @@ def check_overdue_reviews(bot, state: dict) -> list[dict]:
386366
_clear_transport_failure(bot, review_data, issue_number, phase="assignment_confirm_read")
387367
response_state = bot.adapters.review_state.compute_reviewer_response_state(
388368
issue_number,
389-
normalized_review_data,
369+
review_data,
390370
issue_snapshot=issue_snapshot,
391371
)
392372
response_name = str(response_state.get("response_state") or response_state.get("state") or "")

tests/unit/reviewer_bot/test_overdue.py

Lines changed: 0 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -375,56 +375,6 @@ def test_check_overdue_reviews_uses_contributor_comment_timestamp_when_turn_retu
375375
assert overdue[0]["days_overdue"] == 0
376376

377377

378-
def test_check_overdue_reviews_backfills_claim_cycle_from_pr_creation(monkeypatch):
379-
runtime = FakeReviewerBotRuntime(monkeypatch)
380-
now = runtime.datetime.now(runtime.timezone.utc)
381-
created_at = iso_z(now - timedelta(days=runtime.REVIEW_DEADLINE_DAYS + 20))
382-
assigned_at = iso_z(now - timedelta(days=runtime.REVIEW_DEADLINE_DAYS + 1))
383-
state = make_state()
384-
review = make_tracked_review_state(state, 42, reviewer="alice", assigned_at=assigned_at)
385-
review["assignment_method"] = "claim"
386-
routes = RouteGitHubApi().add_pull_request_snapshot(42, pull_request_payload(42, head_sha="head-1")).add_pull_request_reviews(42, [])
387-
runtime = _runtime(monkeypatch, routes)
388-
runtime.github.get_issue_or_pr_snapshot_result = lambda issue_number: runtime.GitHubApiResult(
389-
200,
390-
{**issue_snapshot(issue_number, state="open", is_pull_request=True), "created_at": created_at},
391-
{},
392-
"ok",
393-
True,
394-
None,
395-
0,
396-
None,
397-
)
398-
runtime.github.get_issue_assignees_result = lambda issue_number, is_pull_request=None: runtime.GitHubApiResult(
399-
200,
400-
["alice"],
401-
{},
402-
"ok",
403-
True,
404-
None,
405-
0,
406-
None,
407-
)
408-
monkeypatch.setattr(approval_policy, "compute_pr_approval_state_result", _approval_incomplete_result)
409-
410-
overdue = maintenance.check_overdue_reviews(runtime, state)
411-
412-
assert overdue == [
413-
{
414-
"issue_number": 42,
415-
"reviewer": "alice",
416-
"days_overdue": 1,
417-
"days_since_warning": 0,
418-
"needs_warning": True,
419-
"needs_transition": False,
420-
"anchor_reason": "no_reviewer_activity",
421-
"anchor_timestamp": created_at,
422-
"current_scope_key": f"reviewer=alice|head=head-1|cycle={created_at}|anchor={created_at}",
423-
"current_scope_basis": "active_cycle_started_at",
424-
}
425-
]
426-
427-
428378
def test_check_overdue_reviews_uses_contributor_revision_timestamp_when_head_changes_after_review(monkeypatch):
429379
runtime = FakeReviewerBotRuntime(monkeypatch)
430380
now = runtime.datetime.now(runtime.timezone.utc)

tests/unit/reviewer_bot/test_reviews_live_fetch.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -241,6 +241,41 @@ def test_compute_reviewer_response_state_reports_awaiting_write_approval_after_c
241241
assert response_state["reason"] == "current_head_alternate_approval_present"
242242

243243

244+
def test_compute_reviewer_response_state_uses_issue_created_at_for_claim_alternate_approval_scope(monkeypatch):
245+
state = make_state()
246+
review = make_tracked_review_state(
247+
state,
248+
42,
249+
reviewer="iglesias",
250+
assigned_at="2026-02-26T04:58:03.401345+00:00",
251+
)
252+
review["assignment_method"] = "claim"
253+
accept_reviewer_review(
254+
review,
255+
semantic_key="pull_request_review:99",
256+
timestamp="2026-03-18T01:09:05Z",
257+
actor="iglesias",
258+
reviewed_head_sha="head-old",
259+
source_precedence=1,
260+
)
261+
routes = RouteGitHubApi().add_pull_request_snapshot(42, pull_request_payload(42, head_sha="head-live")).add_pull_request_reviews(
262+
42,
263+
[review_payload(10, state="APPROVED", submitted_at="2026-03-18T12:10:42Z", commit_id="head-live", author="plaindocs")],
264+
)
265+
runtime = _runtime(monkeypatch, routes)
266+
runtime.github.get_issue_or_pr_snapshot = lambda issue_number: {
267+
**issue_snapshot(issue_number, state="open", is_pull_request=True),
268+
"created_at": "2026-02-10T17:20:07Z",
269+
}
270+
271+
response_state = reviews.compute_reviewer_response_state(runtime, 42, review)
272+
273+
assert response_state["state"] == "awaiting_contributor_response"
274+
assert response_state["reason"] == "current_head_alternate_approval_present"
275+
assert response_state["current_scope_basis"] == "alternate_current_head_approval"
276+
assert response_state["current_scope_key"] == "reviewer=iglesias|head=head-live|cycle=2026-02-10T17:20:07Z|anchor=none"
277+
278+
244279
def test_compute_reviewer_response_state_blocks_public_current_head_approval_contradiction_before_refresh(monkeypatch):
245280
state = make_state()
246281
review = make_tracked_review_state(state, 42, reviewer="alice", active_cycle_started_at="2026-03-17T09:00:00Z")

0 commit comments

Comments
 (0)