-
Notifications
You must be signed in to change notification settings - Fork 0
fix(gates): stop three gates from confusing "could not measure" with "measured zero" #121
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -191,6 +191,47 @@ def scoped_blocker_entries() -> dict: | |
| return {} | ||
|
|
||
|
|
||
| def scoped_blocker_source() -> str: | ||
| """WHY `scoped_blockers_live` is the number it is: `ok` / `absent` / `unreadable` / | ||
| `unparseable` / `wrong_shape`. | ||
|
|
||
| THE COUNT ALONE CANNOT SAY, and that is the whole reason this exists. | ||
| `scoped_blocker_entries()` returns `{}` for five different situations and only ONE of them | ||
| means "there are no blockers". The other four are failures to measure, and they render | ||
| identically: `scoped_blockers_live: 0`. That collapse — one value meaning both "measured | ||
| zero" and "could not measure" — is the ninth latched-gate instance in this workspace, and | ||
| scoped blockers are instance #2, where stale blockers emptied the fleet backlog for 78 days. | ||
| A field whose history is a 78-day silent outage should not report its healthy state and its | ||
| blind state with the same integer. | ||
|
|
||
| Deliberately NOT a behaviour change: `scoped_blocker_entries()` still returns `{}` on every | ||
| failure, so an unreadable sentinel still lets work proceed (fail toward motion, per | ||
| CLAUDE.md). This only makes the silence SPEAK. `wrong_shape` is the specific hole the | ||
| 2026-08-24 triage found: `handoff.sh` validates the sentinel with `jq -e .`, which accepts | ||
| any valid JSON including a bare array, so a structurally wrong sentinel is neither | ||
| reinitialised by the writer nor reported by the reader. | ||
| """ | ||
| try: | ||
| raw = SENTINEL.read_text() | ||
| except FileNotFoundError: | ||
| return "absent" | ||
| except OSError: | ||
| return "unreadable" | ||
| try: | ||
| data = json.loads(raw) | ||
| except Exception: # noqa: BLE001 — any decode failure is the same verdict | ||
| return "unparseable" | ||
| if not isinstance(data, dict): | ||
| return "wrong_shape" | ||
| stop = data.get("stop") | ||
| if stop is not None and not isinstance(stop, dict): | ||
| return "wrong_shape" | ||
| blockers = (stop or {}).get("scoped_blockers") | ||
| if blockers is not None and not isinstance(blockers, dict): | ||
| return "wrong_shape" | ||
| return "ok" | ||
|
|
||
|
|
||
| def expired_scoped_blockers(now: int | None = None) -> dict: | ||
| """Blockers past their own `expires_at` — i.e. ones that should no longer block.""" | ||
| current = int(now if now is not None else time.time()) | ||
|
|
@@ -445,6 +486,10 @@ def build_payload(items: list, live_blockers, expired, raised) -> dict: | |
| # precisely because this was invisible). | ||
| "scoped_blockers_live": len(live_blockers), | ||
| "scoped_blockers_expired": len(expired), | ||
| # The two counts above cannot distinguish "no blockers" from "could not read the | ||
| # sentinel" — both render as 0. This names which one it is, so a reader never has to | ||
| # guess whether a zero is health or blindness. See scoped_blocker_source(). | ||
| "scoped_blockers_source": scoped_blocker_source(), | ||
|
Comment on lines
+489
to
+492
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win Report the source status from the same sentinel snapshot as the counts.
Read and classify the sentinel once. Pass the resulting entries and source status through the payload construction. 🤖 Prompt for AI Agents |
||
| "owner_questions_raised": len(raised), | ||
| } | ||
|
|
||
|
|
@@ -688,12 +733,42 @@ def _selftest() -> None: | |
| ) | ||
| assert load_scoped_blockers(now=now) == {"o/r#9"}, "unparseable expiry must still block" | ||
| assert expired_scoped_blockers(now=now) == {}, "unparseable expiry is not 'expired'" | ||
|
|
||
| # ---- THE COUNT AND THE REASON ARE SEPARATE FACTS. `scoped_blocker_entries()` returns | ||
| # {} for five situations and only ONE means "there are no blockers"; the other four are | ||
| # failures to measure and used to render identically as `scoped_blockers_live: 0`. | ||
| # Scoped blockers are latched-gate instance #2 — stale ones emptied the fleet backlog for | ||
| # 78 days — so a zero here must never be ambiguous between health and blindness. | ||
| # Behaviour is deliberately UNCHANGED: every case still yields {} so work proceeds (fail | ||
| # toward motion); only the reported REASON is new. | ||
| SENTINEL = Path(td) / "gone.json" | ||
| assert scoped_blocker_source() == "absent", scoped_blocker_source() | ||
| assert scoped_blocker_entries() == {}, "absent still yields no blockers" | ||
|
|
||
| SENTINEL = Path(td) / "shape.json" | ||
| SENTINEL.write_text("[]") | ||
| assert scoped_blocker_source() == "wrong_shape", scoped_blocker_source() | ||
| assert scoped_blocker_entries() == {}, "a bare array still yields no blockers" | ||
|
|
||
| SENTINEL.write_text("{ not json") | ||
| assert scoped_blocker_source() == "unparseable", scoped_blocker_source() | ||
|
|
||
| SENTINEL.write_text(json.dumps({"stop": []})) | ||
| assert scoped_blocker_source() == "wrong_shape", "a non-dict `stop` is wrong shape" | ||
|
|
||
| SENTINEL.write_text(json.dumps({"stop": {"scoped_blockers": []}})) | ||
| assert scoped_blocker_source() == "wrong_shape", "non-dict blockers are wrong shape" | ||
|
|
||
| # and the healthy zero is DISTINGUISHABLE from all four above | ||
| SENTINEL.write_text(json.dumps({"stop": {"scoped_blockers": {}}})) | ||
| assert scoped_blocker_source() == "ok", scoped_blocker_source() | ||
| assert scoped_blocker_entries() == {}, "no blockers is also {} — hence the source field" | ||
|
Comment on lines
+744
to
+765
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Test the The new As per path instructions, “Flag new or changed behavior with no accompanying test.” 🧰 Tools🪛 ast-grep (0.45.2)[info] 755-755: use jsonify instead of json.dumps for JSON output (use-jsonify) [info] 758-758: use jsonify instead of json.dumps for JSON output (use-jsonify) [info] 762-762: use jsonify instead of json.dumps for JSON output (use-jsonify) 🤖 Prompt for AI AgentsSource: Path instructions |
||
| SENTINEL = _saved_sentinel | ||
|
|
||
| print( | ||
| "backlog.py selftest: OK (ready-issue + in-flight-agent-PR discovery, " | ||
| "body retention, label classification, open-PR-referenced-issue exclusion, " | ||
| "scoped-blocker filter + expiry honoured/fail-safe)" | ||
| "scoped-blocker filter + expiry honoured/fail-safe, sentinel source distinguishes unreadable from empty)" | ||
| ) | ||
|
|
||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -932,6 +932,9 @@ def _selftest_panel_backfill() -> None: | |
| # Blocking AND drainable quantity: a registered subject with no base commit is still | ||
| # not minable, so both counts are reported or neither is meaningful. | ||
| assert "with_base_sha" in result and "without_base_sha" in result, result | ||
| # The pair says WHAT; this says whether zero is reachable. Without it a reader | ||
| # sees a drainable-looking number whose only obvious drain corrupts provenance. | ||
| assert result["without_base_sha_is_permanent_for_discovered_panels"] is True, result | ||
|
|
||
| # A historical bundle must not inherit the current checkout's commit. | ||
| assert ( | ||
|
|
@@ -1354,6 +1357,18 @@ def backfill_panel_subjects(root: Path | None = None, *, apply: bool = False, co | |
| # cannot produce an acceptable completion event. Say both numbers or say neither. | ||
| "with_base_sha": len(minable), | ||
| "without_base_sha": len(registered) - len(minable), | ||
| # AND WHETHER THAT SECOND NUMBER CAN EVER REACH ZERO, which the pair alone does not say. | ||
| # For panels discovered on disk it CANNOT: `discover_historical_panels` sets | ||
| # `base_sha_unrecoverable` on every one, because none recorded the commit under review and | ||
| # inferring one from today's checkout would fuse two states of one app into a single | ||
| # subject (see that function's docstring, and the `must never borrow today's HEAD` | ||
| # assertion in the selftest). So `without_base_sha` here is a PERMANENT floor, not a | ||
| # backlog, and the only honest way to lower it is `--base-sha` on evidence that actually | ||
| # names a commit. Stated because a drainable-looking number invites someone to drain it, | ||
| # and the obvious way to drain this one corrupts provenance — CLAUDE.md §2. A gate that | ||
| # cannot reach its own success state is usually a defect; this one cannot ON PURPOSE, and | ||
| # the difference belongs in the output rather than only in a docstring two functions away. | ||
| "without_base_sha_is_permanent_for_discovered_panels": True, | ||
|
Comment on lines
+1360
to
+1371
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift Do not claim an unavailable The new field is always 🤖 Prompt for AI Agents |
||
| "detail": registered, | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -55,6 +55,7 @@ | |
| import re | ||
| import subprocess | ||
| import sys | ||
| import tempfile | ||
|
|
||
| # TWO ROOTS, and verify.py is the module that most needs them separated: it DISCOVERS modules | ||
| # (beside itself) and it READS repo files — the floor, the coverage artifacts — and RUNS pytest, | ||
|
|
@@ -383,12 +384,24 @@ def _format_absent_line(rep: dict) -> str | None: | |
| ) | ||
|
|
||
|
|
||
| FLOOR_UNREADABLE = "__unreadable__" | ||
|
|
||
|
|
||
| def load_floor() -> dict: | ||
| """The recorded floor, or `{}` when there is none. | ||
|
|
||
| A PRESENT-BUT-UNREADABLE floor is NOT the same as an absent one, and returning `{}` for both | ||
| made them indistinguishable: `floor_state` rendered each as `unset`, so a corrupted | ||
| .verify-floor.json read as "no floor agreed yet" and every count-based check silently stopped | ||
| applying. Same shape as the two other instances found on 2026-08-24 — one sentinel standing | ||
| for "measured nothing" and "could not measure". The unreadable case now carries a marker key | ||
| so callers can say which it is; it stays a dict so no caller breaks. | ||
| """ | ||
| if FLOOR.exists(): | ||
| try: | ||
| return json.loads(FLOOR.read_text(encoding="utf-8")) | ||
| except Exception: # noqa: BLE001 | ||
| return {} | ||
| return {FLOOR_UNREADABLE: True} | ||
|
Comment on lines
400
to
+404
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- README.md ---'
sed -n '1,220p' README.md
printf '%s\n' '--- src/verify.py outline ---'
ast-grep outline src/verify.py
printf '%s\n' '--- relevant source ---'
sed -n '360,440p' src/verify.py
printf '%s\n' '--- verify.py references ---'
rg -n "load_floor|FLOOR_UNREADABLE|\.get\(" src/verify.py
printf '%s\n' '--- tests and selftests ---'
rg -n "floor|unreadable|invalid JSON|json.loads|selftest" -g '*.py' -g '*.md' .Repository: stranske/Orchestrator Length of output: 50380 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- CLAUDE.md review rules ---'
sed -n '200,275p' CLAUDE.md
printf '%s\n' '--- load_floor and verifier use ---'
sed -n '387,410p' src/verify.py
sed -n '700,785p' src/verify.py
printf '%s\n' '--- floor-related selftests ---'
sed -n '900,940p' src/verify.py
sed -n '1245,1295p' src/verify.py
printf '%s\n' '--- changed-file summary ---'
git diff --stat -- src/verify.py
printf '%s\n' '--- floor-related diff ---'
git diff --unified=25 -- src/verify.py | sed -n '/FLOOR_UNREADABLE/,+100p'Repository: stranske/Orchestrator Length of output: 17766 🏁 Script executed: #!/bin/bash
set -eu
python3 - <<'PY'
import json
def load_floor_text(text):
try:
floor = json.loads(text)
return floor
except Exception:
return {"__unreadable__": True}
for text in ("[]", "null", '"text"', '{"collected": 1}'):
floor = load_floor_text(text)
print(f"{text}: type={type(floor).__name__}, get_available={hasattr(floor, 'get')}")
if text != '{"collected": 1}':
try:
floor.get("collected", 0)
except Exception as exc:
print(f" verifier access: {type(exc).__name__}: {exc}")
PY
printf '%s\n' '--- direct shape guards in load_floor ---'
python3 - <<'PY'
from pathlib import Path
text = Path("src/verify.py").read_text(encoding="utf-8")
start = text.index("def load_floor()")
end = text.index("\n\n# The drift message", start)
body = text[start:end]
print(body)
print("has isinstance dict guard:", "isinstance" in body and "dict" in body)
PYRepository: stranske/Orchestrator Length of output: 1456 Validate the parsed floor shape before returning it. If 🤖 Prompt for AI Agents |
||
| return {} | ||
|
|
||
|
|
||
|
|
@@ -640,23 +653,51 @@ def _floor_problems(floor: dict, py: dict) -> list[str]: | |
| return problems | ||
|
|
||
|
|
||
| def _exempt_ceiling_input(exempt: list[str] | None) -> int | None: | ||
| """The ratchet's ceiling input: the COUNT, or None when it could not be counted. PURE. | ||
|
|
||
| A ONE-LINE FUNCTION ON PURPOSE. This rule previously lived inline in `verify()` as | ||
| `len(exempt) if exempt is not None else 0` under a comment saying uncountable "must not read | ||
| as 0" — the comment was right and the code was not. Extracting it makes the rule TESTABLE at | ||
| the call site: a selftest asserting `_ceiling_problems` handles None proves the checker is | ||
| right while saying nothing about what the checker is fed, and it was the feeding that was | ||
| broken. Demonstrated: restoring the `else 0` coercion inline left every assertion green. | ||
| """ | ||
| return len(exempt) if exempt is not None else None | ||
|
|
||
|
|
||
| def _ceiling_problems(floor: dict, actual: dict) -> list[str]: | ||
| """Is anything skipping MORE than the agreed maximum? Pure, for the same reason. | ||
|
|
||
| An UNSET ceiling means "nothing agreed yet", not "zero" — reading a missing key as 0 would | ||
| condemn every machine that legitimately lacks a prerequisite. | ||
|
|
||
| AN UNCOUNTABLE QUANTITY UNDER A SET CEILING IS A FAILURE, not a pass. `actual[key] is None` | ||
| means the run could not measure that population at all; treating it as 0 lets the ceiling | ||
| clear by being BLIND, which is the same defect as a gate that cannot announce its own | ||
| success — one value standing for both "measured zero" and "could not measure". The ceiling is | ||
| an agreement about a number; if the number is unknown the agreement cannot be checked, and | ||
| silence is the wrong answer. | ||
| """ | ||
| problems = [] | ||
| for key, label in CEILINGS: | ||
| limit = floor.get(key) | ||
| if limit is None: | ||
| continue | ||
| if actual.get(key, 0) > int(limit): | ||
| observed = actual.get(key, 0) | ||
| if observed is None: | ||
| problems.append( | ||
| f"CEILING UNCHECKABLE: {label} could not be counted, but `{key}` is set to " | ||
| f"{limit}. A bounded quantity that cannot be measured is a failure, not a pass — " | ||
| f"fix whatever made it uncountable, or remove the ceiling deliberately." | ||
| ) | ||
| continue | ||
| if observed > int(limit): | ||
| problems.append( | ||
| # `CEILING`, not `SKIP CEILING`: the mypy exempt list is bounded by this same | ||
| # machinery and is not a skip, so the old wording sent a reader hunting for a skip | ||
| # that does not exist. The label already names WHICH population overflowed. | ||
| f"CEILING exceeded: {actual[key]} {label} > agreed maximum {limit}. " | ||
| f"CEILING exceeded: {observed} {label} > agreed maximum {limit}. " | ||
| f"This is bounded on purpose — either the new one is wrong, or raise " | ||
| f"`{key}` in .verify-floor.json deliberately and say why." | ||
| ) | ||
|
|
@@ -701,9 +742,17 @@ def verify( | |
| "skipped_max": py["skipped"], | ||
| "selftest_skipped_max": len(st["skipped"]), | ||
| "gate_skipped_max": sum(1 for r in gates.values() if r["skipped"]), | ||
| # None (uncountable) must not read as 0 — that would let the ceiling pass by being blind. | ||
| "mypy_exempt_max": len(exempt) if exempt is not None else 0, | ||
| # None (uncountable) must not read as 0 — that would let the ceiling pass by being | ||
| # blind. The rule lives in `_exempt_ceiling_input` rather than inline here, because | ||
| # inline it was untestable and therefore wrong for as long as it existed. | ||
| "mypy_exempt_max": _exempt_ceiling_input(exempt), | ||
| } | ||
| if floor.get(FLOOR_UNREADABLE): | ||
| problems.append( | ||
| "FLOOR UNREADABLE: .verify-floor.json exists but does not parse, so neither the " | ||
| "collection floor nor any ceiling can be checked. That is a failure, not an unset " | ||
| "floor — fix the file rather than letting a run pass unmeasured." | ||
| ) | ||
| problems += _ceiling_problems(floor, actual) | ||
|
|
||
| def _cap(key: str) -> str: | ||
|
|
@@ -715,15 +764,22 @@ def _cap(key: str) -> str: | |
| # fine at a glance; "floor 386 — 1 BEHIND" cannot be misread, which is the same house rule | ||
| # that makes each ceiling print its count against its limit. | ||
| floor_state = ( | ||
| "unset" | ||
| if not fc | ||
| # UNREADABLE IS NOT UNSET. A corrupted .verify-floor.json used to render as "unset", | ||
| # i.e. "no floor agreed yet" — so a run whose floor could not be parsed looked like a | ||
| # run that never had one, and the count checks stopped applying without saying so. | ||
| "UNREADABLE (.verify-floor.json exists but does not parse)" | ||
| if floor.get(FLOOR_UNREADABLE) | ||
| else ( | ||
| f"{fc}" | ||
| if py["collected"] == fc | ||
| "unset" | ||
| if not fc | ||
| else ( | ||
| f"{fc} — {py['collected'] - fc} BEHIND" | ||
| if py["collected"] > fc | ||
| else f"{fc} — NOT MET" | ||
| f"{fc}" | ||
| if py["collected"] == fc | ||
| else ( | ||
| f"{fc} — {py['collected'] - fc} BEHIND" | ||
| if py["collected"] > fc | ||
| else f"{fc} — NOT MET" | ||
| ) | ||
| ) | ||
| ) | ||
| ) | ||
|
|
@@ -868,7 +924,6 @@ def _selftest() -> None: | |
| # A SILENT ZERO-EXIT MUST BE A FAILURE. This is the exact hole that let 25 pytest-only files | ||
| # read as passing: they exited 0 having executed nothing. Point run_selftests at a module that | ||
| # does precisely that and confirm it is classified as failed, not ok. | ||
| import tempfile | ||
|
|
||
| saved, saved_mods = globals()["HERE"], globals()["MODULES"] | ||
| with tempfile.TemporaryDirectory(prefix="verify-") as td: | ||
|
|
@@ -1196,6 +1251,40 @@ def _selftest() -> None: | |
| mypy_exempt_modules() is not None | ||
| ), "pyproject.toml does not parse; the ratchet cannot be counted" | ||
|
|
||
| # ---- A CEILING MUST NOT PASS BY BEING BLIND. The renderer was fixed first and that was | ||
| # only half of it: `actual["mypy_exempt_max"]` coerced an uncountable None to 0, so the | ||
| # ENFORCEMENT cleared while the LINE said NOT COUNTED. Two readers of one quantity | ||
| # disagreeing, with the permissive one deciding the exit code. | ||
| # The FEED, asserted separately from the CHECKER — the checker was never the broken half. | ||
| assert _exempt_ceiling_input(None) is None, "uncountable must stay uncountable, not become 0" | ||
| assert _exempt_ceiling_input([]) == 0, "a drained ratchet counts as zero, not as unknown" | ||
| assert _exempt_ceiling_input(["a", "b"]) == 2 | ||
| _z = {k: 0 for k, _ in CEILINGS} | ||
| _blind = _ceiling_problems({"mypy_exempt_max": 0}, {**_z, "mypy_exempt_max": None}) | ||
| assert len(_blind) == 1 and "UNCHECKABLE" in _blind[0], _blind | ||
| # an UNSET ceiling over an uncountable quantity is still fine — nothing was agreed | ||
| assert _ceiling_problems({}, {**_z, "mypy_exempt_max": None}) == [] | ||
| # and the ordinary paths are untouched | ||
| assert _ceiling_problems({"mypy_exempt_max": 2}, {**_z, "mypy_exempt_max": 2}) == [] | ||
| assert len(_ceiling_problems({"mypy_exempt_max": 2}, {**_z, "mypy_exempt_max": 3})) == 1 | ||
|
|
||
| # ---- AN UNREADABLE FLOOR IS NOT AN UNSET ONE. Both used to return {} and render "unset", | ||
| # so a corrupt .verify-floor.json looked like a repo that had never agreed a floor, and every | ||
| # count check quietly stopped applying. | ||
| _saved_floor = FLOOR | ||
| try: | ||
| with tempfile.TemporaryDirectory(prefix="verify-floor-") as _td: | ||
| _bad = pathlib.Path(_td) / ".verify-floor.json" | ||
| _bad.write_text("{ not json", encoding="utf-8") | ||
| globals()["FLOOR"] = _bad | ||
| _got = load_floor() | ||
| assert _got.get(FLOOR_UNREADABLE) is True, _got | ||
| assert _got != {}, "unreadable must be distinguishable from absent" | ||
| globals()["FLOOR"] = pathlib.Path(_td) / "does-not-exist.json" | ||
| assert load_floor() == {}, "an absent floor is still simply absent" | ||
| finally: | ||
| globals()["FLOOR"] = _saved_floor | ||
|
|
||
| print( | ||
| "verify.py selftest: OK (count parsing, selftest discovery, silent-zero-exit is a " | ||
| "FAILURE, a loud skip is not a pass, skip ceiling fails when exceeded and holds when " | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Handle text decoding failures as unreadable sentinel input.
Path.read_text()can raiseUnicodeDecodeError. That exception is not anOSError, so it escapes this function.build_payload()now calls this function, so a non-UTF-8 sentinel makesbacklog.py --liveandbacklog.py --dry-runfail instead of reporting a source state.Catch
UnicodeErrorwith the read failures and return"unreadable". Add a regression case for invalid text encoding.🤖 Prompt for AI Agents