From 421e7c693904a450e570804c213f33c03735c58b Mon Sep 17 00:00:00 2001 From: GitHub CI Date: Sat, 10 Oct 2026 11:38:40 -0700 Subject: [PATCH] fix(health): a cut-short structural pass is not a structural outage MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The 6 h window counted every `structural_unavailable` row as "structural_out" and alarmed on them as "structural lane unavailable" — but pr-reviewer stamps that flag on feature-cap / budget partials too (rows before plugin#274), so a pass that ran and kept its findings read as protoPatch being down. Outages are now `structural_unavailable and not structural_partial` (correct for both row shapes); the incomplete-lane tally still names find_structural for either. Co-Authored-By: Claude Opus 5.5 --- scripts/check_review_health.py | 12 ++++++++---- tests/test_watchdog_checks.py | 22 ++++++++++++++++++++-- 2 files changed, 28 insertions(+), 6 deletions(-) diff --git a/scripts/check_review_health.py b/scripts/check_review_health.py index ee9a778..f665774 100755 --- a/scripts/check_review_health.py +++ b/scripts/check_review_health.py @@ -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] = [] @@ -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( @@ -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 = [ diff --git a/tests/test_watchdog_checks.py b/tests/test_watchdog_checks.py index 70d8211..21b796a 100644 --- a/tests/test_watchdog_checks.py +++ b/tests/test_watchdog_checks.py @@ -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)] @@ -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