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
12 changes: 8 additions & 4 deletions scripts/check_review_health.py
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,10 @@ def window_health(rows: list[dict], *, now: float, hours: int = WINDOW_HOURS) ->
recent = [r for r in rows if isinstance(r.get("ts"), (int, float)) and r["ts"] >= since]
reviewed = [r for r in recent if r.get("event") == "reviewed"]
incomplete = [r for r in reviewed if not r.get("complete", True)]
structural = [r for r in reviewed if r.get("structural_unavailable")]
# Outages only: a pass the feature cap or budget cut short ran and kept its findings — counting it
# here is how cap partials got read as protoPatch being down (pr-reviewer-plugin#274). Rows
# written before #274 carry `structural_unavailable` on partials too, so exclude by `structural_partial`.
structural = [r for r in reviewed if r.get("structural_unavailable") and not r.get("structural_partial")]
exhaustions = [r for r in recent if r.get("event") == "exhaustion"]
retries = [r for r in recent if r.get("event") == "panel_retry"]
problems: list[str] = []
Expand All @@ -128,7 +131,7 @@ def window_health(rows: list[dict], *, now: float, hours: int = WINDOW_HOURS) ->
for r in incomplete:
for lane in (r.get("incomplete_finders") or []) + (r.get("degraded") or []):
lanes[str(lane)] = lanes.get(str(lane), 0) + 1
if r.get("structural_unavailable"):
if r.get("structural_unavailable") or r.get("structural_partial"):
lanes["find_structural"] = lanes.get("find_structural", 0) + 1
worst = ", ".join(f"{k}×{v}" for k, v in sorted(lanes.items(), key=lambda kv: -kv[1])[:3])
problems.append(
Expand Down Expand Up @@ -173,8 +176,9 @@ def structural_share_health(
) -> tuple[list[str], str]:
"""(problems, summary) for the share of structural-lane gaps in the last ``hours``. Pure.

A partial pass carries ``structural_unavailable`` too (the lane is a gap either way), so the two
are split by ``structural_partial`` for the message: an outage and a capped/cut-short pass are
A gap is either flag. Since pr-reviewer-plugin#274 a partial pass carries only
``structural_partial``; rows written before it carry ``structural_unavailable`` too. The two are
split by ``structural_partial`` for the message: an outage and a capped/cut-short pass are
different fixes."""
since = now - hours * 3600
rounds = [
Expand Down
22 changes: 20 additions & 2 deletions tests/test_watchdog_checks.py
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,23 @@ def test_structural_outages_alarm_with_their_classes(self):
self.assertIn("structural lane unavailable on 4 rounds", problems[0])
self.assertIn("exit-4:gateway-timeout", problems[0])

def test_a_cut_short_pass_is_not_a_structural_outage(self):
# pr-reviewer-plugin#274: a feature-cap partial ran and kept its findings. Both row shapes —
# before #274 (both flags) and after (`structural_partial` alone) — stay out of the outage count.
legacy = dict(structural_unavailable=True, structural_partial=True, structural_reason="feature-cap")
current = dict(structural_partial=True, structural_reason="feature-cap")
rows = [{**reviewed(10 * i, complete=False), **(legacy if i % 2 else current)} for i in range(1, 6)]
rows += [reviewed(500 + i) for i in range(10)]
problems, summary = window_health(rows, now=NOW)
self.assertEqual(problems, [])
self.assertIn("incomplete=5 structural_out=0", summary)

def test_a_cut_short_pass_still_names_its_lane_when_the_rate_trips(self):
rows = [{**reviewed(10 * i, complete=False), "structural_partial": True} for i in range(1, 6)]
problems, _ = window_health(rows, now=NOW)
self.assertEqual(len(problems), 1)
self.assertIn("find_structural×5", problems[0])

def test_exhaustions_and_retries_are_counted_from_their_own_events(self):
rows = [{"event": "exhaustion", "ts": NOW - 100 * i, "repo": "o/terminal-plugin", "pr": 10} for i in range(3)]
rows += [{"event": "panel_retry", "ts": NOW - 50 * i, "repo": "o/r", "pr": i, "failed": ["verify"]} for i in range(5)]
Expand All @@ -270,10 +287,11 @@ def structural_round(ts_ago_s, *, repo="o/protoAgent", out=False, partial=False,
"recipe": "code-review-structural",
"step_s": {"find_structural": 900.0},
}
if out or partial:
if out:
row.update(structural_unavailable=True, structural_reason=reason, complete=False)
if partial:
row["structural_partial"] = True
# The row shape since pr-reviewer-plugin#274: a partial no longer also says "unavailable".
row.update(structural_partial=True, structural_reason=reason, complete=False)
return row


Expand Down
Loading