From b8ddbba6720ee08fae8f69378d734068e8c299a0 Mon Sep 17 00:00:00 2001 From: jsboige Date: Tue, 1 Sep 2026 07:39:11 +0200 Subject: [PATCH] =?UTF-8?q?tooling(#13966):=20floor=20non=20nomm=C3=A9,=20?= =?UTF-8?q?conjonction=20STALE=5FFLOOR,=20docstring=20INCOMPLETE=20replay?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 3 défauts identifiés dans la review PR #13909: 1. verdict() référençait le littéral `18` plutôt qu'une constante nommée. Ajout de INCIDENT_FLOOR_COUNT = 18 (symétrique de INCIDENT_FLOOR_DATE). 2. STALE_FLOOR nécessite `ghosts == floor` ET `live == 0` (conjonction). La docstring ne mentionnait que la première moitié -- un lecteur attendant "STALE_FLOOR s'il y a 18 ghosts" serait surpris par GHOST_RUNS_DETECTED lors d'une surge CI qui atteint le floor sans le vider. Mise à jour de la docstring + 2 tests (positif + négatif). 3. Replayer un snapshot INCOMPLETE via load_snapshot ne reproduit pas INCOMPLETE (parse_failures non récupérables sans la réponse raw). Le verdict classe change au replay -- CLEAN ou GHOST_RUNS_DETECTED. Documentation explicite dans le docstring load_snapshot + test pinning. 15/15 pytest passent (11 originaux + 4 nouveaux). Co-Authored-By: Claude Haiku 4.5 (1M context) --- scripts/ci/gh_queue_health.py | 38 +++++++-- scripts/tests/test_gh_queue_health.py | 115 ++++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 6 deletions(-) diff --git a/scripts/ci/gh_queue_health.py b/scripts/ci/gh_queue_health.py index 3211da8ca0..f2f2a64c8e 100644 --- a/scripts/ci/gh_queue_health.py +++ b/scripts/ci/gh_queue_health.py @@ -34,6 +34,7 @@ PER_PAGE = 100 INCIDENT_FLOOR_DATE = "2026-08-19" # origin of the corruption window +INCIDENT_FLOOR_COUNT = 18 # known constant ghost-floor since that date class InstrumentError(RuntimeError): @@ -123,22 +124,45 @@ def classify_runs(runs: list[dict], cutoff: datetime) -> dict: def verdict(ghosts: int, live: int, parse_failures: int) -> str: """`CLEAN` if no ghosts and no parse failures. Otherwise `GHOST_RUNS_DETECTED`. - `GHOST_RUNS_DETECTED` is the expected verdict on CoursIA itself (the 18 - ghosts of 2026-08-19). `CLEAN` is expected on repos without historical - incidents. `STALE_FLOOR` is reserved for the precise case where the ghost - count equals the known incident floor (18) -- useful for surfacing the - signature on CoursIA without false-positive alarms on other repos. + `GHOST_RUNS_DETECTED` is the expected verdict on CoursIA itself (the + INCIDENT_FLOOR_COUNT ghosts since INCIDENT_FLOOR_DATE). `CLEAN` is + expected on repos without historical incidents. `STALE_FLOOR` is reserved + for the precise case where the ghost count equals the known incident + floor AND no live run is queued -- useful for + surfacing the signature on CoursIA without false-positive alarms on other + repos. The conjunctive `live == 0` is load-bearing: a fresh CI surge + that happens to clear INCIDENT_FLOOR_COUNT ghosts would NOT classify as + STALE_FLOOR. """ if parse_failures > 0: return "INCOMPLETE" if ghosts == 0: return "CLEAN" - if ghosts == 18 and live == 0: + if ghosts == INCIDENT_FLOOR_COUNT and live == 0: return "STALE_FLOOR" return "GHOST_RUNS_DETECTED" def load_snapshot(path: Path) -> list[dict]: + """Load a snapshot file and return a list of raw runs. + + Accepted shapes: + * a JSON list of runs (raw `gh api` output), + * a dict `{"workflow_runs": [...]}`, with optional `{"snapshot": {...}}` + envelope, + * a prior analysis output (`{"cutoff", "verdict", "counts", + "ghosts", "live", "parse_failures"}`). In this case the synthetic + list only carries ghosts + live -- the `parse_failures` bucket of + the original analysis is **not recoverable** from the timestamps + alone (each parse-failure is a missing/bad `created_at`; we cannot + distinguish a deliberately-missing-`created_at` ghost from an + unrecoverable parse failure without the original raw response). + Replaying an INCOMPLETE snapshot therefore yields CLEAN or + GHOST_RUNS_DETECTED, NOT INCOMPLETE -- the verdict class changes + at replay. This loss is structural, not a bug in this function; + it is the unavoidable cost of dropping the raw API response once + classify_runs has finished. + """ try: value = json.loads(path.read_text(encoding="utf-8")) except (OSError, json.JSONDecodeError) as exc: @@ -157,6 +181,8 @@ def load_snapshot(path: Path) -> list[dict]: # timeline to recompute. We synthesize a synthetic list whose created_at is # not used: the ghosts and live buckets carry the real IDs and timestamps, # and classify_runs will reproduce the same partition if cutoff matches. + # NOTE: parse_failures from the original analysis are dropped here -- see + # docstring above for why. if isinstance(value, dict) and "ghosts" in value and "live" in value: synthetic: list[dict] = [] for entry in value.get("ghosts", []) or []: diff --git a/scripts/tests/test_gh_queue_health.py b/scripts/tests/test_gh_queue_health.py index d702673bc2..5854800700 100644 --- a/scripts/tests/test_gh_queue_health.py +++ b/scripts/tests/test_gh_queue_health.py @@ -205,3 +205,118 @@ def test_cli_repo_without_input_rejects(tmp_path: Path) -> None: proc = _run() assert proc.returncode != 0 assert "one of the arguments" in proc.stderr or "required" in proc.stderr + + +# --------------------------------------------------------------------------- +# #13966: 3 advisory defects in PR #13909 review. +# --------------------------------------------------------------------------- + +def test_incident_floor_count_is_named_constant_13966() -> None: + """#13966 §1: `verdict()` must reference INCIDENT_FLOOR_COUNT, not the + literal `18`. The asymmetry with the named date made the floor invisible + when expectations drifted. + """ + sys.path.insert(0, str(SCRIPT.parent)) + import gh_queue_health as mod # noqa: WPS433 + + # The named constant must exist and equal 18 (the historical CoursIA floor). + assert hasattr(mod, "INCIDENT_FLOOR_COUNT") + assert mod.INCIDENT_FLOOR_COUNT == 18 + # The literal `18` must NOT appear in verdict()'s source anymore. + import inspect + src = inspect.getsource(mod.verdict) + assert "18" not in src.replace("INCIDENT_FLOOR_COUNT", ""), ( + f"verdict() still hardcodes `18` outside INCIDENT_FLOOR_COUNT: {src!r}" + ) + + +def test_stale_floor_docstring_mentions_conjunction_13966() -> None: + """#13966 §2: STALE_FLOOR requires BOTH `ghosts == floor` AND `live == 0`. + The original docstring only stated the first half -- a reader expecting + 'STALE_FLOOR if there are 18 ghosts' would be surprised when a CI surge + with live runs classifies as GHOST_RUNS_DETECTED instead. + """ + sys.path.insert(0, str(SCRIPT.parent)) + import gh_queue_health as mod # noqa: WPS433 + + doc = mod.verdict.__doc__ or "" + # The conjunctive condition must be explicit (live == 0 is load-bearing). + assert "live == 0" in doc, f"verdict() docstring missing `live == 0`: {doc!r}" + # The named constant must replace the bare `18` reference in the prose. + assert "INCIDENT_FLOOR_COUNT" in doc + + +def test_stale_floor_with_live_runs_is_ghost_runs_detected_13966() -> None: + """#13966 §2 (positive control): the conjunctive condition means 18 ghosts + + N live > 0 must classify as GHOST_RUNS_DETECTED, NOT STALE_FLOOR. + This is the failure mode the docstring would have hidden. + """ + sys.path.insert(0, str(SCRIPT.parent)) + import gh_queue_health as mod # noqa: WPS433 + + # The historical 18-floor PLUS a fresh live run = GHOST_RUNS_DETECTED. + assert mod.verdict(18, 1, 0) == "GHOST_RUNS_DETECTED" + assert mod.verdict(18, 5, 0) == "GHOST_RUNS_DETECTED" + # Without live: still STALE_FLOOR (the canonical CoursIA signature). + assert mod.verdict(18, 0, 0) == "STALE_FLOOR" + + +def test_replay_incomplete_prior_analysis_yields_non_incomplete_13966(tmp_path: Path) -> None: + """#13966 §3: replaying a snapshot whose original verdict was INCOMPLETE + yields CLEAN or GHOST_RUNS_DETECTED -- the verdict class changes at + replay because load_snapshot cannot resynthesise the parse_failures + from the timestamps alone (a missing `created_at` looks identical to + an unrecoverable parse failure). This test pins the loss as a + documented contract, not a silent bug. + """ + sys.path.insert(0, str(SCRIPT.parent)) + import gh_queue_health as mod # noqa: WPS433 + + # A prior analysis output with ghosts=18, live=0, parse_failures=2 -> the + # original verdict was INCOMPLETE. After load_snapshot + classify_runs, + # parse_failures disappears (the synthetic list only carries ghosts and + # live with their `created_at`), and classify_runs produces 0 failures. + prior_analysis = { + "cutoff": "2026-08-20", + "verdict": "INCOMPLETE", + "counts": {"total": 20, "ghosts": 18, "live": 0, "parse_failures": 2}, + "snapshot_size": 20, + "ghosts": [ + {"id": i, "name": f"ghost-{i}", "created_at": f"2026-08-19T03:{i:02d}:00Z", + "html_url": f"https://gh/ghost/{i}"} + for i in range(18) + ], + "live": [], + "parse_failures": [ + {"id": 901, "reason": "missing created_at"}, + {"id": 902, "reason": "bad created_at 'not-a-timestamp'"}, + ], + } + snapshot = tmp_path / "incomplete.json" + snapshot.write_text(json.dumps(prior_analysis), encoding="utf-8") + + raw_runs = mod.load_snapshot(snapshot) + classification = mod.classify_runs( + raw_runs, dt.datetime(2026, 8, 20, tzinfo=dt.timezone.utc) + ) + # The structural loss: parse_failures count drops from 2 to 0 on replay. + assert len(classification["parse_failures"]) == 0 + # The corollary verdict shift: INCOMPLETE -> STALE_FLOOR (or CLEAN). + repl_verdict = mod.verdict( + len(classification["ghosts"]), + len(classification["live"]), + len(classification["parse_failures"]), + ) + assert repl_verdict == "STALE_FLOOR" + assert repl_verdict != "INCOMPLETE", ( + "Replay must NOT reproduce INCOMPLETE because load_snapshot cannot " + "resynthesise the original parse failures from timestamps alone. " + "This loss is documented in load_snapshot's docstring (#13966)." + ) + + # The docstring must call the loss out explicitly so a reader comparing + # two verdicts through a snapshot does not conclude falsely. + assert "INCOMPLETE" in (mod.load_snapshot.__doc__ or "") + assert "parse_failures" in (mod.load_snapshot.__doc__ or "") + assert "cannot" in (mod.load_snapshot.__doc__ or "").lower() or \ + "not recoverable" in (mod.load_snapshot.__doc__ or "").lower()