Skip to content

Commit aa0e9c9

Browse files
authored
fix: avoid blocking active issue314 reviewer waits (#576)
1 parent 392fb9f commit aa0e9c9

4 files changed

Lines changed: 271 additions & 17 deletions

File tree

scripts/reviewer_bot_lib/issue314_state_health.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -435,6 +435,12 @@ def _live_head_sha(snapshot: dict[str, object], projection: StatusLabelProjectio
435435
return None
436436

437437

438+
def _has_unsafe_automated_reminder_risk(decision: ReviewerResponseDecision) -> bool:
439+
if decision.response_state == "awaiting_reviewer_response":
440+
return False
441+
return not decision.suppresses_overdue_reminder
442+
443+
438444
def _row_from_input(input: Issue314StateHealthClassificationInput, row: dict[str, object]) -> Issue314StateHealthRow:
439445
issue_number = int(row["issue_number"])
440446
review_data = row.get("review_data") if isinstance(row.get("review_data"), dict) else {}
@@ -469,9 +475,10 @@ def _row_from_input(input: Issue314StateHealthClassificationInput, row: dict[str
469475
reason = "closed_live_item_has_active_issue314_row"
470476
else:
471477
assert projection is not None
478+
assert decision is not None
472479
delta = projection.delta
473480
has_drift = bool(delta.labels_to_add or delta.labels_to_remove)
474-
automated_risk = projection.response_state == "awaiting_reviewer_response"
481+
automated_risk = _has_unsafe_automated_reminder_risk(decision)
475482
if automated_risk:
476483
health = "blocked"
477484
repair = "blocked"

tests/integration/reviewer_bot/test_app_preview_issue314_state_health.py

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -177,3 +177,73 @@ def test_execute_run_preview_issue314_state_health_blocks_unavailable_live_rows(
177177
"reviewer_response_unavailable",
178178
"status_projection_unavailable",
179179
]
180+
181+
182+
def test_execute_run_preview_issue314_state_health_keeps_aligned_awaiting_reviewer_response_healthy(
183+
monkeypatch,
184+
capsys,
185+
):
186+
harness = AppHarness(monkeypatch)
187+
harness.set_event(
188+
EVENT_NAME="workflow_dispatch",
189+
EVENT_ACTION="",
190+
MANUAL_ACTION="preview-issue314-state-health",
191+
ISSUE_NUMBER=314,
192+
VALIDATION_NONCE="nonce-issue314-preview",
193+
GITHUB_SHA="workflow-head",
194+
GITHUB_REPOSITORY="rustfoundation/safety-critical-rust-coding-guidelines",
195+
GITHUB_RUN_ID="1001",
196+
GITHUB_RUN_ATTEMPT="1",
197+
STATE_ISSUE_NUMBER=314,
198+
)
199+
200+
state = make_state()
201+
review = make_tracked_review_state(
202+
state,
203+
410,
204+
reviewer="cpetig",
205+
assigned_at="2026-01-01T00:00:00Z",
206+
active_cycle_started_at="2026-01-01T00:00:00Z",
207+
)
208+
review["active_head_sha"] = "head-live"
209+
routes = RouteGitHubApi().add_request(
210+
"GET",
211+
"issues/410",
212+
status_code=200,
213+
payload={
214+
"number": 410,
215+
"state": "open",
216+
"pull_request": {},
217+
"labels": [{"name": "status: awaiting reviewer response"}],
218+
},
219+
).add_request(
220+
"GET",
221+
"pulls/410",
222+
status_code=200,
223+
payload={
224+
**pull_request_payload(410, head_sha="head-live", author="contributor"),
225+
"requested_reviewers": [{"login": "cpetig"}],
226+
},
227+
).add_pull_request_reviews(410, []).add_request(
228+
"GET",
229+
"issues/410/comments?per_page=100&page=1",
230+
status_code=200,
231+
payload=[],
232+
)
233+
harness.runtime.github.stub(routes)
234+
harness.stub_load_state(lambda *, fail_on_unavailable=False: state)
235+
harness.stub_lock(acquire=lambda: (_ for _ in ()).throw(AssertionError("preview should not acquire lock")))
236+
harness.stub_save_state(lambda current: (_ for _ in ()).throw(AssertionError("preview should not save state")))
237+
harness.stub_sync_status_labels(lambda current, issue_numbers: (_ for _ in ()).throw(AssertionError("preview should not sync labels")))
238+
239+
result = harness.run_execute()
240+
241+
assert result.exit_code == 0
242+
payload = json.loads(capsys.readouterr().out)
243+
assert payload["active_rows_inspected"] == [410]
244+
assert payload["rows_blocked"] == []
245+
assert payload["rows_repairable"] == []
246+
row = payload["row_inventory"][0]
247+
assert row["health_classification"] == "healthy"
248+
assert row["automated_reminder_risk"] is False
249+
assert row["status_label_risk"] == "aligned"

tests/integration/reviewer_bot/test_app_repair_issue314_state_health.py

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,3 +162,81 @@ def test_execute_run_repair_issue314_state_health_requires_identity(monkeypatch,
162162

163163
assert result.exit_code == 1
164164
assert not summary_path.exists()
165+
166+
167+
def test_execute_run_repair_issue314_state_health_repairs_awaiting_reviewer_label_drift(
168+
monkeypatch,
169+
tmp_path,
170+
):
171+
harness = AppHarness(monkeypatch)
172+
summary_path = tmp_path / "repair" / "issue314-state-health-repair-summary.json"
173+
harness.set_event(
174+
EVENT_NAME="workflow_dispatch",
175+
EVENT_ACTION="",
176+
MANUAL_ACTION="repair-issue314-state-health",
177+
ISSUE_NUMBER=314,
178+
VALIDATION_NONCE="nonce-issue314-repair",
179+
GITHUB_SHA="workflow-head",
180+
GITHUB_REPOSITORY="rustfoundation/safety-critical-rust-coding-guidelines",
181+
GITHUB_RUN_ID="1004",
182+
GITHUB_RUN_ATTEMPT="1",
183+
STATE_ISSUE_NUMBER=314,
184+
)
185+
harness.stub_lock()
186+
monkeypatch.setenv("ISSUE314_STATE_HEALTH_REPAIR_SUMMARY_PATH", str(summary_path))
187+
188+
state = make_state()
189+
make_tracked_review_state(
190+
state,
191+
360,
192+
reviewer="AlexCeleste",
193+
assigned_at="2026-01-01T00:00:00Z",
194+
active_cycle_started_at="2026-01-01T00:00:00Z",
195+
)
196+
routes = RouteGitHubApi().add_request(
197+
"GET",
198+
"issues/360",
199+
status_code=200,
200+
payload={
201+
"number": 360,
202+
"state": "open",
203+
"labels": [
204+
{"name": "status: awaiting reviewer response"},
205+
{"name": "status: draft"},
206+
],
207+
"assignees": [{"login": "AlexCeleste"}],
208+
},
209+
).add_request(
210+
"GET",
211+
"issues/360/comments?per_page=100&page=1",
212+
status_code=200,
213+
payload=[],
214+
).add_request(
215+
"DELETE",
216+
"issues/360/labels/status%3A%20draft",
217+
status_code=204,
218+
payload=None,
219+
)
220+
harness.runtime.github.stub(routes)
221+
harness.runtime.github.post_comment_result = lambda issue_number, body: (_ for _ in ()).throw(
222+
AssertionError("issue314 repair should not post reviewer-facing reminders")
223+
)
224+
harness.stub_load_state(lambda *, fail_on_unavailable=False: state)
225+
harness.stub_save_state(lambda current: (_ for _ in ()).throw(AssertionError("label-only issue314 repair should not save state")))
226+
harness.stub_sync_status_labels(lambda current, issue_numbers: (_ for _ in ()).throw(AssertionError("issue314 repair should not broad-sync labels")))
227+
monkeypatch.setattr(
228+
app,
229+
"collect_status_projection_repair_items",
230+
lambda bot, state: (_ for _ in ()).throw(AssertionError("issue314 repair broadened through epoch repair")),
231+
)
232+
233+
result = harness.run_execute()
234+
235+
assert result.exit_code == 0
236+
payload = json.loads(summary_path.read_text(encoding="utf-8"))
237+
assert payload["active_rows_inspected"] == [360]
238+
assert payload["rows_repaired"] == [360]
239+
assert payload["rows_blocked"] == []
240+
assert payload["status_labels_changed"] == [360]
241+
assert payload["reviewer_facing_reminder_posts_attempted"] == 0
242+
assert payload["result"] == "changed"

tests/unit/reviewer_bot/test_issue314_state_health.py

Lines changed: 115 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -9,19 +9,26 @@
99
from scripts.reviewer_bot_lib.reminder_comments import scan_reviewer_reminder_comments
1010

1111

12-
def _projection(issue_number: int, actual_labels: tuple[str, ...], state: str):
13-
decision = to_reviewer_response_decision(
14-
{
15-
"issue_number": issue_number,
16-
"current_reviewer": "iglesias",
17-
"response_state": state,
18-
"suppression_reason": "legacy_duplicate_reminders_exhausted"
19-
if state == "reviewer_reassignment_needed"
20-
else None,
21-
"current_scope_key": "scope",
22-
"current_scope_basis": "reminder_cadence_exhausted",
23-
}
24-
)
12+
def _projection(
13+
issue_number: int,
14+
actual_labels: tuple[str, ...],
15+
state: str,
16+
*,
17+
suppresses_overdue_reminder: bool | None = None,
18+
):
19+
decision_payload = {
20+
"issue_number": issue_number,
21+
"current_reviewer": "iglesias",
22+
"response_state": state,
23+
"suppression_reason": "legacy_duplicate_reminders_exhausted"
24+
if state == "reviewer_reassignment_needed"
25+
else None,
26+
"current_scope_key": "scope",
27+
"current_scope_basis": "reminder_cadence_exhausted",
28+
}
29+
if suppresses_overdue_reminder is not None:
30+
decision_payload["suppresses_overdue_reminder"] = suppresses_overdue_reminder
31+
decision = to_reviewer_response_decision(decision_payload)
2532
return decision, reviews_projection.derive_status_label_projection(
2633
reviews_projection.StatusLabelProjectionInput(
2734
issue_number=issue_number,
@@ -135,13 +142,105 @@ def test_issue314_classifier_marks_pr264_stale_label_as_operator_action_without_
135142
assert payload["output_keys"] == sorted(payload.keys())
136143

137144

138-
def test_issue314_classifier_blocks_rows_with_automated_reminder_risk():
139-
decision, projection = _projection(42, (), "awaiting_reviewer_response")
145+
def test_issue314_classifier_keeps_aligned_awaiting_reviewer_response_healthy():
146+
decision, projection = _projection(
147+
42,
148+
("status: awaiting reviewer response",),
149+
"awaiting_reviewer_response",
150+
)
140151
input = issue314_state_health.Issue314StateHealthClassificationInput(
141152
state_issue_number=314,
142153
validation_nonce="nonce",
143154
active_review_rows=({"issue_number": 42, "review_data": {"current_reviewer": "alice"}},),
144-
live_snapshots={42: {"number": 42, "state": "open", "pull_request": {}, "labels": []}},
155+
live_snapshots={
156+
42: {
157+
"number": 42,
158+
"state": "open",
159+
"pull_request": {},
160+
"labels": [{"name": "status: awaiting reviewer response"}],
161+
}
162+
},
163+
reviewer_responses={42: decision},
164+
status_projections={42: projection},
165+
reminder_scans={42: scan_reviewer_reminder_comments([])},
166+
evaluated_repo="rustfoundation/safety-critical-rust-coding-guidelines",
167+
head_sha="head",
168+
evaluated_ref="head",
169+
workflow_path=".github/workflows/reviewer-bot-preview.yml",
170+
run_id="1",
171+
run_attempt="1",
172+
)
173+
174+
summary = issue314_state_health.classify_issue314_state_health(input)
175+
176+
assert summary.rows_blocked == ()
177+
assert summary.rows_repairable == ()
178+
row = summary.row_inventory[0]
179+
assert row.health_classification == "healthy"
180+
assert row.automated_reminder_risk is False
181+
assert row.status_label_risk == "aligned"
182+
183+
184+
def test_issue314_classifier_marks_awaiting_reviewer_response_label_drift_repairable():
185+
decision, projection = _projection(
186+
360,
187+
("status: awaiting reviewer response", "status: draft"),
188+
"awaiting_reviewer_response",
189+
)
190+
input = issue314_state_health.Issue314StateHealthClassificationInput(
191+
state_issue_number=314,
192+
validation_nonce="nonce",
193+
active_review_rows=({"issue_number": 360, "review_data": {"current_reviewer": "alice"}},),
194+
live_snapshots={
195+
360: {
196+
"number": 360,
197+
"state": "open",
198+
"labels": [
199+
{"name": "status: awaiting reviewer response"},
200+
{"name": "status: draft"},
201+
],
202+
}
203+
},
204+
reviewer_responses={360: decision},
205+
status_projections={360: projection},
206+
reminder_scans={360: scan_reviewer_reminder_comments([])},
207+
evaluated_repo="rustfoundation/safety-critical-rust-coding-guidelines",
208+
head_sha="head",
209+
evaluated_ref="head",
210+
workflow_path=".github/workflows/reviewer-bot-preview.yml",
211+
run_id="1",
212+
run_attempt="1",
213+
)
214+
215+
summary = issue314_state_health.classify_issue314_state_health(input)
216+
217+
assert summary.rows_blocked == ()
218+
assert summary.rows_repairable == (360,)
219+
row = summary.row_inventory[0]
220+
assert row.health_classification == "repairable"
221+
assert row.automated_reminder_risk is False
222+
assert row.status_label_risk == "repairable_drift"
223+
224+
225+
def test_issue314_classifier_blocks_contradictory_non_remindable_response():
226+
decision, projection = _projection(
227+
42,
228+
("status: reviewer reassignment needed",),
229+
"reviewer_reassignment_needed",
230+
suppresses_overdue_reminder=False,
231+
)
232+
input = issue314_state_health.Issue314StateHealthClassificationInput(
233+
state_issue_number=314,
234+
validation_nonce="nonce",
235+
active_review_rows=({"issue_number": 42, "review_data": {"current_reviewer": "alice"}},),
236+
live_snapshots={
237+
42: {
238+
"number": 42,
239+
"state": "open",
240+
"pull_request": {},
241+
"labels": [{"name": "status: reviewer reassignment needed"}],
242+
}
243+
},
145244
reviewer_responses={42: decision},
146245
status_projections={42: projection},
147246
reminder_scans={42: scan_reviewer_reminder_comments([])},

0 commit comments

Comments
 (0)