From 17598d1636b8b2f7e210e2cdce888652d649fedd Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 12:37:31 -0400 Subject: [PATCH 1/6] fix: grade an absent rehearsal as STALE, not RED (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Heart reported RED `release validation FAILED` against a Release Integrate run that was entirely green: 661 passed, 0 failed, `failures: []`. The tick's auto-ingest (`release_run.py`) folds the latest release-integrate run's stage report on its own, with no merge base. That artifact is integrate-only — `to_stage_report()` emits a single top-level `stage` — so it can never carry a `rehearse` stage, and `_Accumulator.release_ready()` requires one. `release_ready` was therefore false by construction on this path however green the run was, and `readiness.py` mapped every false to the RED axis. One boolean cannot answer both "did anything fail?" and "is the evidence complete?". Add `validation_outcome: pass|fail|incomplete` and read that for severity; `release_ready` is unchanged beside it for compatibility. - validate.py: emit `validation_outcome`. `fail` covers any adverse evidence — a failed stage, positive `totals.failed`/`totals.timeout`, or a non-empty `failures` list — not just a stage literally marked "fail", which `release_ready()` alone tested. Also apply `_norm_status` in `add_report`, which previously copied stage statuses verbatim, so a merged report's "failure" never normalised and read as not-a-failure. - readiness.py: `fail` -> RED (unchanged), `incomplete` -> STALE. A report predating the field falls back to `release_ready` and stays RED — fail closed. The STALE text keeps the literal "release validation" so the Health Agent classifier routes it to the `validate` capability instead of `unknown`. - dashboard.py: render `incomplete` as WARN, not a FAIL row beside a stale header; add `stale_reasons` to `to_dict()`, the surface the Health Agent and mobile read, so a reason moved onto the stale axis is re-reported rather than disappearing. - release_run.py: stop calling every non-true ingest "FAILED", and re-fold once when the stored report predates the discriminator — otherwise the RED would persist behind the run-id cache. That migration is skipped when the report already holds a `rehearse` stage, which this check's artifact cannot reproduce, so a release drive's manual multi-stage ingest is never regressed. Verified end-to-end against the real state and the real artifact, with no CI re-dispatched: RED 45 / `release validation FAILED` -> YELLOW 70 / `release validation incomplete: no rehearsal for current source`, red_reasons empty. The remaining yellows are the pre-existing workspace-validation and manifest-drift ones. Regression tests pin the fail-closed cases a naive "no stage says fail" test would have softened: positive failed counts, positive timeout counts, a non-empty failures list, a "failure" status synonym arriving via `add_report`, and a legacy report with no discriminator. The pre-existing `test_validation_failed_is_red` is unmodified and serves as the control. Diagnosis and plan were cross-reviewed by Codex, which rejected an earlier readiness-only fix; its load-bearing findings were re-verified against the code before being adopted. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- docs/release_validation.md | 5 +- health_agent/capabilities.yaml | 21 ++++-- heart/checks/release_run.py | 71 +++++++++++++++++--- heart/dashboard.py | 17 ++++- heart/readiness.py | 36 ++++++++--- heart/validate.py | 94 ++++++++++++++++++++++++--- tests/test_dashboard.py | 57 +++++++++++++++++ tests/test_readiness.py | 74 +++++++++++++++++++++ tests/test_release_run.py | 112 +++++++++++++++++++++++++++++++- tests/test_validate.py | 114 +++++++++++++++++++++++++++++++++ 10 files changed, 562 insertions(+), 39 deletions(-) diff --git a/docs/release_validation.md b/docs/release_validation.md index b129fad..9b81722 100644 --- a/docs/release_validation.md +++ b/docs/release_validation.md @@ -138,7 +138,10 @@ a failing local artifact remains RED. `heart/validate.py` records `profile` and per-repo `commit_shas` in `validation_report.json`; `heart/readiness.py` then requires, for GREEN: -- `release_ready == true` (no stage failed — else RED), +- `validation_outcome == pass` (else RED for `fail`; **STALE** for `incomplete`, + which means nothing failed and the rehearsal evidence is simply absent — a + report predating the field falls back to `release_ready == false` → RED, so + the gate fails closed on evidence it cannot classify), - `profile == release` (else YELLOW — a smoke-fidelity run is not a release gate), - `commit_shas` matching the current `main` HEADs (else YELLOW — stale source), - freshness (a rehearsal older than `VALIDATION_STALE_DAYS` is YELLOW). diff --git a/health_agent/capabilities.yaml b/health_agent/capabilities.yaml index a9a72d6..876e3e6 100644 --- a/health_agent/capabilities.yaml +++ b/health_agent/capabilities.yaml @@ -128,14 +128,21 @@ release_validation: `release-stage-report` artifact emitted by the release-integrate.yml channel — collected and handed over by the Brain Release Agent. produces: >- - validation_report.json (schema_version 1): release_ready, testpypi_version, - profile, per-repo commit_shas, per-stage status, per-project counts, - failures, run URLs, timestamp — persisted in Heart state + a - validation_history/ archive. + validation_report.json (schema_version 1): validation_outcome + (pass|fail|incomplete), release_ready (legacy boolean, kept for + compatibility), testpypi_version, profile, per-repo commit_shas, per-stage + status, per-project counts, failures, run URLs, timestamp — persisted in + Heart state + a validation_history/ archive. gate_role: >- - HARD readiness gate. release_ready==false → RED; absent / stale-by-age / - commit_shas not matching current main HEADs / profile != release → YELLOW; - fresh pass matching current source under the release profile → GREEN. + HARD readiness gate. Read validation_outcome, NOT release_ready — the + boolean collapses "a stage failed" and "no rehearsal evidence" into one + false, and the tick's integrate-only auto-ingest always lands on that + false however green the run was. validation_outcome==fail → RED; + ==incomplete → STALE (an evidence gap, nothing known-bad); absent / + stale-by-age / commit_shas not matching current main HEADs / profile != + release → YELLOW; fresh pass matching current source under the release + profile → GREEN. A report predating validation_outcome falls back to + release_ready==false → RED (fail closed). orchestrated_by: "PyAutoBrain Release Agent (agents/release/) — dispatch/poll/download" boundary: "ingest-and-judge only; never dispatches release.yml or workspace-validation.yml" diff --git a/heart/checks/release_run.py b/heart/checks/release_run.py index 993c310..2039e45 100644 --- a/heart/checks/release_run.py +++ b/heart/checks/release_run.py @@ -16,11 +16,17 @@ - a fresher local ingest is never regressed (report ts vs run creation time); - a run already ingested (sidecar-cached id) is never re-downloaded; -- a FAILED rehearsal ingests too: ``release_ready: false`` is evidence, not an - evidence gap — readiness then shows the accurate ``release validation +- a FAILED rehearsal ingests too: ``validation_outcome: "fail"`` is evidence, + not an evidence gap — readiness then shows the accurate ``release validation FAILED (stage integrate)`` instead of week-old STALE, and it self-clears on the next green night. +The artifact this check downloads is an **integrate-only** stage report, so the +folded report can never carry a ``rehearse`` stage and its ``release_ready`` is +``false`` by construction — however green the run was. That is why severity is +read from ``validation_outcome`` (``incomplete``, an evidence gap → STALE) and +not from the boolean, which would report a failure that never happened. + ``decide()`` is pure and no-network: the gh-backed callables are injected only by the ``main()`` tick/CLI entrypoint (the #83/#120 discipline). """ @@ -119,6 +125,12 @@ def decide( Actions: no-runs · in-progress · cached (already ingested) · local-fresher (never regress a newer local ingest) · ingest. + + A report that predates ``validation_outcome`` is re-ingested once even when + the run id is cached: without that, a report already folded by the old code + would keep its missing discriminator forever, and the readiness gate — which + fails closed on reports it cannot classify — would stay RED until some + unrelated future run happened to come along. """ if not run_record: return {"action": "no-runs"} @@ -131,15 +143,46 @@ def decide( } if run_record.get("status") != "completed": return {**out, "action": "in-progress"} - if isinstance(sidecar, dict) and sidecar.get("last_ingested_run_id") == run_id: - return {**out, "action": "cached"} - report_ts = _parse_ts((current_report or {}).get("ts")) - created = _parse_ts(out["created"]) - if report_ts is not None and created is not None and report_ts >= created: - return {**out, "action": "local-fresher"} + # A one-time re-fold for reports written before `validation_outcome` existed. + # + # Skipped when the current report already carries a `rehearse` stage: that is + # evidence this check's integrate-only artifact cannot reproduce (it comes + # from a manual multi-stage ingest during a release drive), so re-folding + # would throw it away and turn a `pass` into an `incomplete`. Note the test + # is the rehearsal, NOT `local-fresher` — every ingest necessarily happens + # after the run it ingests, so a report being "fresher than the run" says + # nothing about where it came from. + stages = (current_report or {}).get("stages") + has_rehearsal = isinstance(stages, dict) and "rehearse" in stages + stale_schema = ( + isinstance(current_report, dict) + and bool(current_report) + and not has_rehearsal + and current_report.get("validation_outcome") not in ("pass", "fail", "incomplete") + ) + if not stale_schema: + if isinstance(sidecar, dict) and sidecar.get("last_ingested_run_id") == run_id: + return {**out, "action": "cached"} + report_ts = _parse_ts((current_report or {}).get("ts")) + created = _parse_ts(out["created"]) + if report_ts is not None and created is not None and report_ts >= created: + return {**out, "action": "local-fresher"} return {**out, "action": "ingest"} +def resolve_outcome(ingested: dict[str, Any] | None) -> str: + """``pass`` | ``fail`` | ``incomplete`` for an ingested report. + + Pure, like ``decide()``, so the tick's wording is testable without the + network. Reports predating ``validation_outcome`` fall back to the legacy + boolean and fail closed. + """ + outcome = (ingested or {}).get("validation_outcome") + if outcome in ("pass", "fail", "incomplete"): + return str(outcome) + return "pass" if (ingested or {}).get("release_ready") is True else "fail" + + def main(argv: list[str] | None = None) -> int: sys.path.insert(0, str(HEART_HOME)) from heart import state, validate @@ -160,6 +203,7 @@ def main(argv: list[str] | None = None) -> int: "last_ingested_run_id": decision.get("run_id"), "ingested_ts": ingested.get("ts"), "release_ready": ingested.get("release_ready"), + "validation_outcome": ingested.get("validation_outcome"), "run_url": decision.get("url"), }) @@ -167,9 +211,16 @@ def main(argv: list[str] | None = None) -> int: label_id = decision.get("run_id", "?") if action == "ingest": - ready = (ingested or {}).get("release_ready") - if ready is True: + outcome = resolve_outcome(ingested) + if outcome == "pass": glyph, label = glyph_ok(), c_ok(f"rehearsal ingested (run {label_id}: pass)") + elif outcome == "incomplete": + # This is the ordinary state for this path: the artifact is an + # integrate-only stage report, so it carries no rehearsal evidence. + # Nothing failed — do not say FAILED. + glyph, label = glyph_warn(), c_warn( + f"integrate ingested (run {label_id}: no rehearsal evidence)" + ) else: glyph, label = glyph_fail(), c_fail(f"rehearsal ingested (run {label_id}: FAILED)") elif action in ("cached", "local-fresher"): diff --git a/heart/dashboard.py b/heart/dashboard.py index d6afdb2..cece2e8 100644 --- a/heart/dashboard.py +++ b/heart/dashboard.py @@ -571,9 +571,18 @@ def build_board( profile = vr.get("profile") or "?" stages = vr.get("stages") or {} meta = f"v{ver} profile={profile} ({vr.get('ts', '?')})" - if ready is False: + # Mirror the readiness gate: read `validation_outcome`, falling back to + # the legacy boolean. `incomplete` is an evidence gap (WARN), not a + # failure (FAIL) — a green integrate-only ingest lands there, and a FAIL + # row beside a stale header verdict reads as a broken release. + outcome = vr.get("validation_outcome") + if outcome not in ("pass", "fail", "incomplete"): + outcome = "fail" if ready is False else ("pass" if ready is True else None) + if outcome == "fail": st, summary = FAIL, f"NOT release_ready — {meta}" - elif ready is True: + elif outcome == "incomplete": + st, summary = WARN, f"incomplete — no rehearsal evidence — {meta}" + elif outcome == "pass": st, summary = OK, f"release_ready — {meta}" else: st, summary = WARN, f"release_ready unknown — {meta}" @@ -798,6 +807,10 @@ def to_dict(board: Board) -> dict[str, Any]: "stale": board.stale, "red_reasons": board.red_reasons, "yellow_reasons": board.yellow_reasons, + # Evidence gaps belong on this surface too: the Health Agent and mobile + # read it, and a reason that moves from the red axis to the stale one + # would otherwise vanish from both rather than being re-classified. + "stale_reasons": board.stale_reasons, "pages_url": PAGES_URL, "sections": [ { diff --git a/heart/readiness.py b/heart/readiness.py index 45afb86..e8daef8 100644 --- a/heart/readiness.py +++ b/heart/readiness.py @@ -16,8 +16,10 @@ exceeds the newest released version of its library (UNSATISFIABLE — no installable release can satisfy it), or an unparseable (BAD) floor/tag; or the deep install verification last reported ``ready == false``; or the - release-validation report last ingested reports ``release_ready == false`` (a - stage failed). + release-validation report last ingested reports ``validation_outcome == + "fail"`` (a stage failed, or failing/timed-out counts were recorded). A report + predating that field falls back to ``release_ready == false`` → RED, so the + gate fails closed on evidence it cannot classify. - **YELLOW** (caution) for soft signals: workspace-validation not passing (the workspace scripts/notebooks carry standing debt, so this is advisory — never a hard block), script-timing regressions, stale open PRs, stale parked scripts, a @@ -30,7 +32,9 @@ - **STALE** (an evidence gap, the freshness tier) when nothing is known-bad but some evidence is *missing or expired*: a check that was never run, a passing-but-aged report, a rehearsal whose ``commit_shas`` no longer match - ``main``, an unknown repo/version status. The remedy for a stale reason is to + ``main``, a validation report whose ``validation_outcome`` is ``incomplete`` + (nothing failed; the rehearsal evidence is absent), an unknown repo/version + status. The remedy for a stale reason is to **re-run the check**, never to fix code — which is exactly what separates it from yellow. The tier is not a skip lever: evidence whose *last known result was adverse* stays yellow/red until a fresh run says otherwise; only @@ -453,13 +457,29 @@ def scope_local(msg: str, key: str) -> None: # This is the M2 gate: the report proves the exact source about to ship was # built, published to TestPyPI, installed from the wheel, and exercised at # release fidelity. Absent/stale/source-not-matching → YELLOW ("no release - # rehearsal for current source"); failing → RED. Pass/fail (release_ready) - # is the RED axis; fidelity+freshness (profile / commit_shas / age) is the - # YELLOW axis — a passing-but-stale report is a caution, not a blocker. + # rehearsal for current source"); failing → RED. `validation_outcome` is the + # RED axis; fidelity+freshness (profile / commit_shas / age) is the YELLOW + # axis — a passing-but-stale report is a caution, not a blocker. + # + # Read `validation_outcome`, NOT `release_ready`: the boolean collapses + # "something failed" and "the rehearsal evidence is missing" into one + # `false`, and the tick's integrate-only auto-ingest always lands on that + # `false` (see heart/validate.py). Grading it RED reported a failure that + # had not happened. A report predating the field carries no discriminator, + # so `false` there stays RED — fail closed. vr = snapshot.get("validation_report") if isinstance(vr, dict) and vr: ready = vr.get("release_ready") - if ready is False: + outcome = vr.get("validation_outcome") + if outcome not in ("pass", "fail", "incomplete"): + outcome = "fail" if ready is False else ("pass" if ready is True else None) + if outcome == "incomplete": + # Nothing is wrong; the rehearsal evidence is simply absent. The + # wording must contain "release validation" — the Health Agent + # classifier matches on that string to route the remedy. + stale.append("release validation incomplete: no rehearsal for current source") + hit("validation_absent") + elif outcome == "fail": failed_stages = [ n for n, s in (vr.get("stages") or {}).items() if isinstance(s, dict) and s.get("status") == "fail" @@ -469,7 +489,7 @@ def scope_local(msg: str, key: str) -> None: + (f" (stage {', '.join(failed_stages)})" if failed_stages else "") ) hit("validation_failed") - elif ready is True: + elif outcome == "pass": commit_shas = vr.get("commit_shas") or {} mismatched: list[str] = [] unconfirmed: list[str] = [] diff --git a/heart/validate.py b/heart/validate.py index c073428..d2ad7eb 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -35,7 +35,8 @@ { "schema_version": 1, - "release_ready": true, # top-level pass/fail axis (no stage failed) + "release_ready": true, # legacy boolean, kept for compatibility + "validation_outcome": "pass", # pass | fail | incomplete — the real axis "testpypi_version": "2026.6.30.1.dev64501", "profile": "release", # env profile the integration tier ran under "commit_shas": { # per-repo HEAD the rehearsal was built from @@ -60,14 +61,32 @@ "ts": "2026-06-30T12:00:00+00:00" } -``release_ready`` is the **pass/fail** axis only: it is ``false`` if any ran -stage failed. Release *fidelity* and *freshness* (``profile == release``, -``commit_shas`` matching the current ``main`` HEADs, age) are judged separately -by the readiness gate (``heart/readiness.py``) — a passing-but-stale or -passing-but-wrong-profile report is YELLOW there, not GREEN, while a failing one -is RED. Keeping the axes separate is what lets an M2 rehearsal-only report be -faithfully ``release_ready`` yet still gate YELLOW until M3 wires the -release-fidelity integration. +``validation_outcome`` is the **pass/fail/incomplete** axis, and it is the one to +read: + +- ``fail`` — something adverse was ingested: a stage reported ``fail``, or + ``totals.failed``/``totals.timeout`` is positive, or ``failures`` is non-empty, + or a merged base report said so explicitly. +- ``incomplete`` — nothing is wrong, but the ``rehearse`` evidence is absent, so + the report cannot attest that anything was built. This is an **evidence gap**, + which the readiness gate grades STALE — *not* a failure. +- ``pass`` — no adverse evidence and the rehearsal passed. + +``release_ready`` is the older boolean, kept unchanged for compatibility. It +collapses ``fail`` and ``incomplete`` into a single ``false``, which is exactly +why it must not be used to decide RED: the tick's auto-ingest +(``heart/checks/release_run.py``) folds an **integrate-only** stage report, which +can never carry a ``rehearse`` stage, so it lands on ``false`` by construction +however green the run was. Consumers deciding severity read +``validation_outcome``; a report predating the field has no discriminator and is +treated as a failure (fail closed). + +Release *fidelity* and *freshness* (``profile == release``, ``commit_shas`` +matching the current ``main`` HEADs, age) are judged separately by the readiness +gate (``heart/readiness.py``) — a passing-but-stale or passing-but-wrong-profile +report is YELLOW there, not GREEN. Keeping the axes separate is what lets an M2 +rehearsal-only report be faithfully ``pass`` yet still gate YELLOW until M3 wires +the release-fidelity integration. Recognised input artifacts (files, or directories scanned for them): @@ -198,6 +217,7 @@ def __init__(self) -> None: # persists it to the separate sidecar path readiness reads. self.verify_install: dict[str, Any] | None = None self._explicit_ready: bool | None = None + self._explicit_outcome: str | None = None # True once a real stage artifact (add_stage) has contributed counts. # add_report() consults this so merging an old validation_report.json # as a "base" never double-counts totals/per_project/failures that a @@ -307,7 +327,13 @@ def add_report(self, data: dict[str, Any]) -> None: self.add_commit_shas(data.get("commit_shas")) for name, entry in (data.get("stages") or {}).items(): if isinstance(entry, dict) and name not in self.stages: - self.stages[name] = dict(entry) + merged = dict(entry) + # Normalise exactly as add_stage does. Without this a merged + # report carrying a synonym ("failure", "timed_out") would keep + # a status that is not literally "fail", and every downstream + # "did a stage fail?" test would read it as not-a-failure. + merged["status"] = _norm_status(entry.get("status")) + self.stages[name] = merged if not self._stage_counts_seen: if isinstance(data.get("totals"), dict): self._add_counts(self.totals, data["totals"]) @@ -319,6 +345,50 @@ def add_report(self, data: dict[str, Any]) -> None: self.run_urls.setdefault(str(k), str(v)) if isinstance(data.get("release_ready"), bool): self._explicit_ready = data["release_ready"] + outcome = data.get("validation_outcome") + if outcome in ("pass", "fail", "incomplete"): + self._explicit_outcome = str(outcome) + + def _has_adverse_evidence(self) -> bool: + """True if anything ingested is actually *bad* (not merely missing). + + Deliberately wider than "a stage said fail". ``release_ready`` never + consulted ``totals``/``failures``, so an artifact claiming + ``status: pass`` while carrying failing counts used to slip through the + stage test — and an unrecognised status token normalises to ``skip``, + never ``fail``. Anything adverse here forces ``fail``. + """ + if any(s.get("status") == "fail" for s in self.stages.values()): + return True + if self.totals.get("failed", 0) or self.totals.get("timeout", 0): + return True + return bool(self.failures) + + def validation_outcome(self) -> str: + """``pass`` | ``fail`` | ``incomplete`` — the two axes, separated. + + ``release_ready`` is a single boolean that has to answer two different + questions ("did anything fail?" and "is the evidence complete?"), so a + report with nothing built is indistinguishable from a report where + something broke. This is the discriminator; ``release_ready`` is kept + beside it, unchanged, for compatibility. + + Fails closed: anything adverse, or an explicit ``fail``/``false`` from a + merged base report, is ``fail``. ``incomplete`` is reserved for the case + where nothing is wrong and the rehearsal evidence is simply absent. + """ + if self._has_adverse_evidence(): + return "fail" + if self._explicit_outcome is not None: + return self._explicit_outcome + if self._explicit_ready is not None: + # A legacy base report with no discriminator: false means "not + # ready" and we cannot tell why, so treat it as a failure. + return "pass" if self._explicit_ready else "fail" + rehearse = self.stages.get("rehearse") + if rehearse and rehearse.get("status") == "pass": + return "pass" + return "incomplete" def release_ready(self) -> bool: """True iff no ran stage failed AND the rehearse stage passed. @@ -326,6 +396,9 @@ def release_ready(self) -> bool: The rehearse stage is mandatory: a report with nothing built is not release-ready. An explicit ``release_ready`` from a merged base report is honoured only when no stage contradicts it with a failure. + + Unchanged and kept for compatibility. It cannot distinguish "failed" + from "incomplete" — read ``validation_outcome()`` for that. """ if any(s.get("status") == "fail" for s in self.stages.values()): return False @@ -400,6 +473,7 @@ def _distill(acc: _Accumulator, now: datetime.datetime | None = None) -> dict[st return { "schema_version": SCHEMA_VERSION, "release_ready": acc.release_ready(), + "validation_outcome": acc.validation_outcome(), "testpypi_version": acc.testpypi_version, "profile": acc.profile, "commit_shas": dict(sorted(acc.commit_shas.items())), diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index b9be269..b76769b 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -301,3 +301,60 @@ def test_main_no_cache_badge_emits_unknown_payload(monkeypatch, tmp_path, capsys assert payload["schemaVersion"] == 1 assert payload["message"] == "unknown" assert payload["color"] == "lightgrey" + + +# --- release-validation row + the stale axis on the machine surface --------- + + +def _section(board, key): + return next(s for s in board.sections if s.key == key) + + +def test_validation_incomplete_renders_warn_not_fail(): + """`incomplete` is an evidence gap; a FAIL row would contradict the header. + + The row is a projection of the same report the readiness gate reads, so it + has to make the same fail/incomplete distinction — otherwise a stale verdict + is rendered beside a red-looking validation row. + """ + snap = make_snapshot(validation_report={ + "release_ready": False, "validation_outcome": "incomplete", + "testpypi_version": "2026.6.1.1.dev100", "profile": "release", + "stages": {"integrate": {"status": "pass"}}, "ts": TS, + }) + board = dashboard.build_board(snap, make_verdict("stale", 85), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.WARN + + +def test_validation_fail_still_renders_fail(): + snap = make_snapshot(validation_report={ + "release_ready": False, "validation_outcome": "fail", + "testpypi_version": "2026.6.1.1.dev100", "profile": "release", + "stages": {"integrate": {"status": "fail"}}, "ts": TS, + }) + board = dashboard.build_board(snap, make_verdict("red", 45), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.FAIL + + +def test_validation_legacy_false_without_discriminator_renders_fail(): + snap = make_snapshot(validation_report={ + "release_ready": False, + "testpypi_version": "2026.6.1.1.dev100", "profile": "release", + "stages": {"integrate": {"status": "pass"}}, "ts": TS, + }) + board = dashboard.build_board(snap, make_verdict("red", 45), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.FAIL + + +def test_machine_surface_carries_stale_reasons(): + """The Health Agent and mobile read `to_dict()`. + + Without this key a reason re-classified from the red axis to the stale one + would vanish from those surfaces entirely rather than being re-reported. + """ + verdict = make_verdict("stale", 85) + verdict["stale_reasons"] = ["release validation incomplete: no rehearsal for current source"] + board = dashboard.build_board(make_snapshot(), verdict, now=FRESH_NOW) + payload = dashboard.to_dict(board) + assert "stale_reasons" in payload + assert payload["stale_reasons"] == verdict["stale_reasons"] diff --git a/tests/test_readiness.py b/tests/test_readiness.py index 24b12d1..b2b0d9d 100644 --- a/tests/test_readiness.py +++ b/tests/test_readiness.py @@ -838,3 +838,77 @@ def test_default_profile_output_is_unchanged_shape(): assert v["verdict"] == "green" assert v["profile"] == "default" assert v["na_reasons"] == [] + + +# --- release validation: the fail/incomplete split -------------------------- +# +# `test_validation_failed_is_red` above is the control for these: it sets a +# genuinely failed stage and must keep reporting RED, unmodified. + + +def test_validation_incomplete_is_stale_not_red(): + """A green integrate-only ingest is an evidence gap, not a failure. + + The tick's auto-ingest folds an integrate-only stage report, which can never + carry a `rehearse` stage, so `release_ready` is false by construction. This + used to be graded RED `release validation FAILED` on a run where nothing + failed at all. + """ + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "incomplete" + report["stages"] = {"integrate": {"status": "pass", "profile": "release"}} + report["failures"] = [] + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] != "red" + assert not any("release validation FAILED" in r for r in v["red_reasons"]) + assert any("release validation incomplete" in r for r in v["stale_reasons"]) + + +def test_validation_incomplete_reason_routes_in_health_agent(): + """The reason text must stay matchable by the Health Agent classifier. + + `health.sh` maps a reason to the `validate` capability by matching + "release validation" / "validation report"; a reason matching neither falls + through to `unknown` and recommends a bare tick, which cannot repair this. + """ + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "incomplete" + v = compute(make_snapshot(validation_report=report)) + reasons = [r for r in v["stale_reasons"] if "incomplete" in r] + assert reasons + assert all("release validation" in r for r in reasons) + + +def test_validation_outcome_fail_is_red(): + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "fail" + report["stages"]["integrate"] = {"status": "fail", "profile": "release"} + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" + assert any("release validation FAILED" in r for r in v["red_reasons"]) + + +def test_validation_legacy_false_without_discriminator_stays_red(): + """No `validation_outcome` field at all: fail closed. + + Pre-existing reports carry no discriminator, so `false` cannot be told apart + from a real failure and must keep blocking. + """ + report = _green_validation_report() + report["release_ready"] = False + report.pop("validation_outcome", None) + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" + assert any("release validation FAILED" in r for r in v["red_reasons"]) + + +def test_validation_outcome_overrides_stale_release_ready_true(): + """An explicit `fail` blocks even if the legacy boolean says ready.""" + report = _green_validation_report() + report["release_ready"] = True + report["validation_outcome"] = "fail" + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" diff --git a/tests/test_release_run.py b/tests/test_release_run.py index 9a09faa..7b23adf 100644 --- a/tests/test_release_run.py +++ b/tests/test_release_run.py @@ -45,7 +45,10 @@ def test_already_ingested_run_is_cached(): def test_fresher_local_ingest_is_never_regressed(): - current = {"ts": "2026-07-30T17:53:15+00:00"} # local ingest AFTER the run + # local ingest AFTER the run. Carries `validation_outcome`, i.e. every report + # written since the tri-state landed; a report predating it is re-folded once + # (test_live_shape_migrates_integrate_only_report_written_by_the_old_code). + current = {"ts": "2026-07-30T17:53:15+00:00", "validation_outcome": "pass"} d = rr.decide(current, None, _run(created="2026-07-30T05:16:28Z")) assert d["action"] == "local-fresher" @@ -66,3 +69,110 @@ def test_new_run_id_supersedes_old_cache(): d = rr.decide({"ts": "2026-07-29T00:00:00+00:00"}, {"last_ingested_run_id": 6}, _run(run_id=7)) assert d["action"] == "ingest" + + +# --- how an ingested report is described in the tick line ------------------ +# +# The artifact this check downloads is integrate-only, so it can never carry a +# rehearsal and its `release_ready` is false by construction. Reporting that as +# FAILED asserted a failure that had not happened. + + +def test_green_integrate_only_ingest_is_incomplete_not_failed(): + assert rr.resolve_outcome({ + "release_ready": False, + "validation_outcome": "incomplete", + "stages": {"integrate": {"status": "pass"}}, + }) == "incomplete" + + +def test_failed_ingest_still_reads_fail(): + assert rr.resolve_outcome({ + "release_ready": False, + "validation_outcome": "fail", + "stages": {"integrate": {"status": "fail"}}, + }) == "fail" + + +def test_passing_ingest_reads_pass(): + assert rr.resolve_outcome({"release_ready": True, "validation_outcome": "pass"}) == "pass" + + +def test_legacy_ingest_without_discriminator_fails_closed(): + assert rr.resolve_outcome({"release_ready": False}) == "fail" + assert rr.resolve_outcome({"release_ready": True}) == "pass" + assert rr.resolve_outcome(None) == "fail" + assert rr.resolve_outcome({}) == "fail" + + +# --- one-time re-ingest for reports predating validation_outcome ----------- + + +def _report(ts="2026-07-30T06:00:00Z", **kw): + return {"ts": ts, **kw} + + +def test_cached_run_is_reingested_once_when_report_predates_discriminator(): + """Otherwise the RED this fix removes would never clear on its own. + + The report was folded by the old code, so it has no `validation_outcome`; + the gate fails closed on that and stays RED. The run id is already cached, + so nothing would re-fold it. + """ + d = rr.decide(_report(), {"last_ingested_run_id": 7}, _run(run_id=7)) + assert d["action"] == "ingest" + + +def test_report_holding_rehearsal_evidence_is_never_re_folded(): + """A manual multi-stage ingest must survive the schema migration. + + Its `rehearse` stage cannot be reproduced from this check's integrate-only + artifact, so re-folding would discard the evidence a release drive collected + and turn a `pass` into an `incomplete`. + """ + d = rr.decide( + _report(stages={"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}), + {"last_ingested_run_id": 7}, + _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_cached_still_holds_once_the_report_carries_the_discriminator(): + d = rr.decide(_report(validation_outcome="incomplete"), + {"last_ingested_run_id": 7}, _run(run_id=7)) + assert d["action"] == "cached" + + +def test_local_fresher_still_holds_once_the_report_carries_the_discriminator(): + d = rr.decide(_report(ts="2026-08-01T00:00:00Z", validation_outcome="pass"), + None, _run(run_id=7)) + assert d["action"] == "local-fresher" + + +def test_live_shape_migrates_integrate_only_report_written_by_the_old_code(): + """The exact state this fix has to clear on the dev box. + + An integrate-only report, ingested after its own run (so "fresher" than it), + run id already cached, and no discriminator — the RED would otherwise persist + until some unrelated future run came along. + """ + d = rr.decide( + _report(ts="2026-08-14T15:53:14+00:00", + stages={"integrate": {"status": "pass"}}), + {"last_ingested_run_id": 31769743408}, + _run(run_id=31769743408, created="2026-08-14T04:23:59Z"), + ) + assert d["action"] == "ingest" + + +def test_absent_report_is_unaffected_by_the_schema_check(): + """No report at all is the ordinary first-ingest path, not a stale schema.""" + assert rr.decide(None, None, _run(run_id=7))["action"] == "ingest" + assert rr.decide({}, {"last_ingested_run_id": 7}, _run(run_id=7))["action"] == "cached" + + +def test_in_progress_still_never_ingests_even_with_a_stale_schema(): + d = rr.decide(_report(), {"last_ingested_run_id": 7}, + _run(run_id=7, status="in_progress", conclusion=None)) + assert d["action"] == "in-progress" diff --git a/tests/test_validate.py b/tests/test_validate.py index a1dd7f8..8439654 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -570,3 +570,117 @@ def test_run_keeps_newest_verify_install_across_artifacts(tmp_path, monkeypatch) importlib.reload(state_mod) importlib.reload(v_mod) + + +# --- validation_outcome: the fail/incomplete split -------------------------- +# +# `release_ready` collapses "something failed" and "nothing was built" into one +# `false`. These pin the discriminator, and in particular that everything +# ambiguous or adverse lands on `fail` rather than being softened. + + +def test_outcome_integrate_only_green_is_incomplete(tmp_path): + """The tick's auto-ingest shape: green, integrate-only, no rehearsal. + + This is the case that used to be graded a release FAILURE. + """ + _write(tmp_path / "stage_report.json", dict(INTEGRATE)) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "incomplete" + assert report["release_ready"] is False # legacy boolean unchanged + assert report["stages"]["integrate"]["status"] == "pass" + assert report["failures"] == [] + + +def test_outcome_rehearsal_plus_green_integrate_is_pass(tmp_path): + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict(INTEGRATE)) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "pass" + assert report["release_ready"] is True + + +def test_outcome_failed_stage_is_fail(tmp_path): + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict(INTEGRATE, status="fail")) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "fail" + + +def test_outcome_failed_counts_without_failed_stage_is_fail(tmp_path): + """An artifact claiming `pass` while carrying failing counts must not soften. + + `release_ready()` never consulted `totals`, so the stage test alone would + read this as "nothing failed" and, with no rehearsal, call it incomplete. + """ + _write(tmp_path / "stage_report.json", dict( + INTEGRATE, status="pass", + summary={"passed": 100, "failed": 5, "skipped": 0, "timeout": 0}, + )) + report = validate.ingest([tmp_path]) + assert report["totals"]["failed"] == 5 + assert report["stages"]["integrate"]["status"] == "pass" + assert report["validation_outcome"] == "fail" + + +def test_outcome_timeout_counts_without_failed_stage_is_fail(tmp_path): + _write(tmp_path / "stage_report.json", dict( + INTEGRATE, status="pass", + summary={"passed": 100, "failed": 0, "skipped": 0, "timeout": 2}, + )) + report = validate.ingest([tmp_path]) + assert report["totals"]["timeout"] == 2 + assert report["validation_outcome"] == "fail" + + +def test_outcome_failures_list_without_failed_stage_is_fail(tmp_path): + _write(tmp_path / "stage_report.json", dict( + INTEGRATE, status="pass", + failures=[{"project": "autolens_workspace", "script": "x.py"}], + )) + report = validate.ingest([tmp_path]) + assert report["failures"] + assert report["validation_outcome"] == "fail" + + +def test_add_report_normalises_stage_status_synonyms(tmp_path): + """A merged base report's `"failure"` must normalise to `"fail"`. + + `_norm_status` ran only in `add_stage`, so a synonym arriving through + `add_report` kept a status that no "did a stage fail?" test would match. + """ + _write(tmp_path / "validation_report.json", { + "schema_version": 1, + "release_ready": False, + "stages": {"integrate": {"status": "failure"}}, + "totals": {"passed": 0, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + "ts": "2026-06-01T00:00:00+00:00", + }) + report = validate.ingest([tmp_path]) + assert report["stages"]["integrate"]["status"] == "fail" + assert report["validation_outcome"] == "fail" + + +def test_outcome_legacy_report_without_discriminator_fails_closed(tmp_path): + """`release_ready: false` and no `validation_outcome`, nothing else adverse. + + We cannot tell whether it failed or was merely incomplete, so it stays a + failure — the gate must never soften evidence it cannot classify. + """ + _write(tmp_path / "validation_report.json", { + "schema_version": 1, + "release_ready": False, + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 10, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + "ts": "2026-06-01T00:00:00+00:00", + }) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "fail" + + +def test_outcome_nothing_ingested_is_incomplete(tmp_path): + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "incomplete" + assert report["release_ready"] is False From a0ebf2674add3da2b4959bd4a02ff389f82d90b5 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 12:57:07 -0400 Subject: [PATCH 2/6] fix: close the fail-open holes a Codex review found (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Six findings from an adversarial review of the first commit. Each was reproduced against the branch before being fixed, and each now has a regression test. Two were regressions this PR introduced; the rest were pre-existing holes the new `_has_adverse_evidence` docstring claimed to close. Introduced here, now fixed: - The schema migration could erase a real RED. A legacy report that failed BEFORE reaching the rehearsal has no `rehearse` stage and no discriminator, so the rehearsal guard alone let the migration overwrite it with a green integrate-only artifact — turning a genuine failure into an evidence gap. The migration is now additionally blocked by any adverse evidence in the stored report (failed stage, failing/timed-out counts, failures list). - A run whose conclusion is not `success` could read as `incomplete` when its uploaded artifact said `integrate: pass` — the workflow can break outside what the stage report captures, and that report is written by a step that may have run before the break. `validate.ingest`/`run` take `force_fail`, and the tick passes the run's own conclusion. Pre-existing, closed while the predicate is being touched: - `per_project` failed/timeout counts were never consulted, so a report could carry a failing project while its top-level totals read clean, and still reach `pass` and GREEN. - A stage that ran without passing now yields `incomplete`, not `pass`. `_norm_status` maps every unrecognised token to `skip`, so an adverse status Heart does not know could otherwise be read as success on the strength of the rehearsal alone. - A malformed discriminator, or one contradicting the `release_ready` beside it, now fails closed in both the producer and the readiness consumer. It previously fell through to the optimistic boolean. - `_print_summary` rendered only the legacy boolean, so the CLI could print a green `✓ release_ready` for a report whose outcome was `fail`. It now reports the tri-state. `bin/pyauto-heart` help updated to match. Test-quality gaps the review also named: - `test_validation_incomplete_is_stale_not_red` set `release_ready: False`, so it would have passed with the readiness change reverted. It now leaves the boolean True, pinning that the discriminator drives the verdict, and asserts the exact verdict rather than "not red". - Added a green-path test on the NATIVE schema — the baseline fixture predates the field, so every other green test exercised only the legacy fallback — and it re-checks that the `pass` branch still performs its SHA and profile fidelity checks. Verified by mutation: reverting the readiness change fails 4 of the new tests. Live end-to-end re-checked against the real state and artifact, unchanged: YELLOW 70, `red_reasons: []`. Suite 398 passed. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- bin/pyauto-heart | 11 ++-- heart/checks/release_run.py | 44 ++++++++++---- heart/readiness.py | 10 +++- heart/validate.py | 97 +++++++++++++++++++++++------- tests/test_readiness.py | 64 +++++++++++++++++++- tests/test_release_run.py | 52 ++++++++++++++++ tests/test_validate.py | 114 ++++++++++++++++++++++++++++++++++++ 7 files changed, 350 insertions(+), 42 deletions(-) diff --git a/bin/pyauto-heart b/bin/pyauto-heart index 4354ac6..d3836bc 100755 --- a/bin/pyauto-heart +++ b/bin/pyauto-heart @@ -260,12 +260,13 @@ This NEVER dispatches a build, talks to GitHub, or mutates any repo. All dispatching/polling/downloading is the Brain Release Agent's job; Heart is spec + ingest + verdict, credential-free. -The report carries release_ready (pass/fail axis), testpypi_version, profile, -per-repo commit_shas, per-stage status, per-project counts, failures and run -URLs. \`pyauto-heart readiness\` consumes it as a HARD gate: GREEN-for-release +The report carries validation_outcome (pass|fail|incomplete — the severity axis), +release_ready (the legacy boolean, kept for compatibility), testpypi_version, +profile, per-repo commit_shas, per-stage status, per-project counts, failures and +run URLs. \`pyauto-heart readiness\` consumes it as a HARD gate: GREEN-for-release now requires a fresh passing report whose commit_shas match the current main -HEADs under the 'release' profile; absent/stale/mismatch → YELLOW; a failed -stage → RED. +HEADs under the 'release' profile; absent/stale/mismatch → YELLOW; incomplete +(nothing failed, no rehearsal evidence) → STALE; a failure → RED. With no --ingest, prints the currently persisted validation_report.json. EOF diff --git a/heart/checks/release_run.py b/heart/checks/release_run.py index 2039e45..c719f47 100644 --- a/heart/checks/release_run.py +++ b/heart/checks/release_run.py @@ -145,19 +145,34 @@ def decide( return {**out, "action": "in-progress"} # A one-time re-fold for reports written before `validation_outcome` existed. # - # Skipped when the current report already carries a `rehearse` stage: that is - # evidence this check's integrate-only artifact cannot reproduce (it comes - # from a manual multi-stage ingest during a release drive), so re-folding - # would throw it away and turn a `pass` into an `incomplete`. Note the test - # is the rehearsal, NOT `local-fresher` — every ingest necessarily happens - # after the run it ingests, so a report being "fresher than the run" says - # nothing about where it came from. - stages = (current_report or {}).get("stages") - has_rehearsal = isinstance(stages, dict) and "rehearse" in stages + # Strictly limited to reports this check's own integrate-only artifact can + # reproduce in full, because the re-fold OVERWRITES the canonical report: + # + # - skipped when a `rehearse` stage is present — evidence from a manual + # multi-stage ingest during a release drive, which this artifact cannot + # reproduce; re-folding would turn its `pass` into an `incomplete`; + # - skipped when the stored report carries ANY adverse evidence — a failed + # stage, failing/timed-out counts, or a failures list. Those can come from + # a run that broke before it ever reached the rehearsal, and re-folding a + # green artifact over them would silently convert a real RED into a STALE. + # + # Note the test is neither `cached` nor `local-fresher`: every ingest happens + # after the run it ingests, so "the report is fresher than the run" says + # nothing about where the report came from. + stages = (current_report or {}).get("stages") or {} + stages = stages if isinstance(stages, dict) else {} + totals = (current_report or {}).get("totals") or {} + totals = totals if isinstance(totals, dict) else {} + stored_adverse = ( + any(isinstance(s, dict) and s.get("status") == "fail" for s in stages.values()) + or bool(totals.get("failed", 0) or totals.get("timeout", 0)) + or bool((current_report or {}).get("failures")) + ) stale_schema = ( isinstance(current_report, dict) and bool(current_report) - and not has_rehearsal + and "rehearse" not in stages + and not stored_adverse and current_report.get("validation_outcome") not in ("pass", "fail", "incomplete") ) if not stale_schema: @@ -197,7 +212,14 @@ def main(argv: list[str] | None = None) -> int: if report_path is None: action = decision["action"] = "artifact-unavailable" else: - ingested = validate.run([td]) + # A run whose conclusion is not `success` is a failure even if + # the stage report it uploaded says otherwise — the workflow can + # break outside anything that artifact captures, and the artifact + # is written by a step that may have run before the break. + ingested = validate.run( + [td], + force_fail=str(decision.get("conclusion") or "").lower() != "success", + ) # Record the ingest so the tick never re-downloads this run. state.atomic_write_json(SIDECAR, { "last_ingested_run_id": decision.get("run_id"), diff --git a/heart/readiness.py b/heart/readiness.py index e8daef8..053cfb0 100644 --- a/heart/readiness.py +++ b/heart/readiness.py @@ -472,7 +472,15 @@ def scope_local(msg: str, key: str) -> None: ready = vr.get("release_ready") outcome = vr.get("validation_outcome") if outcome not in ("pass", "fail", "incomplete"): - outcome = "fail" if ready is False else ("pass" if ready is True else None) + if "validation_outcome" in vr: + # Present but unrecognised: a malformed gate artifact is + # untrustworthy evidence, never a pass. + outcome = "fail" + else: + outcome = "fail" if ready is False else ("pass" if ready is True else None) + elif outcome == "pass" and ready is False: + # The two fields contradict each other; believe the pessimistic one. + outcome = "fail" if outcome == "incomplete": # Nothing is wrong; the rehearsal evidence is simply absent. The # wording must contain "release validation" — the Health Agent diff --git a/heart/validate.py b/heart/validate.py index d2ad7eb..ee81b99 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -64,13 +64,19 @@ ``validation_outcome`` is the **pass/fail/incomplete** axis, and it is the one to read: -- ``fail`` — something adverse was ingested: a stage reported ``fail``, or - ``totals.failed``/``totals.timeout`` is positive, or ``failures`` is non-empty, - or a merged base report said so explicitly. -- ``incomplete`` — nothing is wrong, but the ``rehearse`` evidence is absent, so - the report cannot attest that anything was built. This is an **evidence gap**, - which the readiness gate grades STALE — *not* a failure. -- ``pass`` — no adverse evidence and the rehearsal passed. +- ``fail`` — something adverse was ingested: a stage reported ``fail``; + ``failed``/``timeout`` is positive in ``totals`` **or in any ``per_project`` + entry**; ``failures`` is non-empty; the caller passed ``force_fail`` (e.g. the + producing run's own conclusion was not ``success``, whatever its artifact + claims); a merged base report said so explicitly; or the discriminator is + malformed or contradicts the ``release_ready`` beside it. +- ``incomplete`` — nothing adverse and nothing contradictory, but the report + cannot attest that everything ran: the ``rehearse`` evidence is absent, or a + stage ran without passing (``skip`` — which is also where ``_norm_status`` + puts any token it does not recognise, so an unknown status can never read as a + pass). This is an **evidence gap**, which the readiness gate grades STALE — + *not* a failure. +- ``pass`` — no adverse evidence, every stage passed, and the rehearsal passed. ``release_ready`` is the older boolean, kept unchanged for compatibility. It collapses ``fail`` and ``incomplete`` into a single ``false``, which is exactly @@ -218,6 +224,8 @@ def __init__(self) -> None: self.verify_install: dict[str, Any] | None = None self._explicit_ready: bool | None = None self._explicit_outcome: str | None = None + self._outcome_invalid = False + self._force_fail = False # True once a real stage artifact (add_stage) has contributed counts. # add_report() consults this so merging an old validation_report.json # as a "base" never double-counts totals/per_project/failures that a @@ -345,22 +353,38 @@ def add_report(self, data: dict[str, Any]) -> None: self.run_urls.setdefault(str(k), str(v)) if isinstance(data.get("release_ready"), bool): self._explicit_ready = data["release_ready"] - outcome = data.get("validation_outcome") - if outcome in ("pass", "fail", "incomplete"): - self._explicit_outcome = str(outcome) + if "validation_outcome" in data: + outcome = data.get("validation_outcome") + if outcome in ("pass", "fail", "incomplete"): + self._explicit_outcome = str(outcome) + else: + # Present but not a value we recognise: the report is malformed, + # and a malformed gate artifact must never read as a pass. + self._outcome_invalid = True + + @staticmethod + def _counts_adverse(counts: Any) -> bool: + return bool( + isinstance(counts, dict) + and (counts.get("failed", 0) or counts.get("timeout", 0)) + ) def _has_adverse_evidence(self) -> bool: """True if anything ingested is actually *bad* (not merely missing). Deliberately wider than "a stage said fail". ``release_ready`` never - consulted ``totals``/``failures``, so an artifact claiming - ``status: pass`` while carrying failing counts used to slip through the - stage test — and an unrecognised status token normalises to ``skip``, - never ``fail``. Anything adverse here forces ``fail``. + consulted the counts at all, so an artifact claiming ``status: pass`` + while carrying failing ones slipped through the stage test — and an + unrecognised status token normalises to ``skip``, never ``fail``. + Per-project counts are checked too: they are merged independently of + ``totals``, so a report can carry a failing project while its top-level + totals read clean. Anything adverse here forces ``fail``. """ if any(s.get("status") == "fail" for s in self.stages.values()): return True - if self.totals.get("failed", 0) or self.totals.get("timeout", 0): + if self._counts_adverse(self.totals): + return True + if any(self._counts_adverse(c) for c in self.per_project.values()): return True return bool(self.failures) @@ -373,12 +397,26 @@ def validation_outcome(self) -> str: something broke. This is the discriminator; ``release_ready`` is kept beside it, unchanged, for compatibility. - Fails closed: anything adverse, or an explicit ``fail``/``false`` from a - merged base report, is ``fail``. ``incomplete`` is reserved for the case - where nothing is wrong and the rehearsal evidence is simply absent. + Fails closed at every step. ``incomplete`` is reserved for the single + benign case: nothing adverse, nothing contradictory, and the rehearsal + evidence is simply absent. """ - if self._has_adverse_evidence(): + if self._force_fail or self._has_adverse_evidence(): + return "fail" + # A malformed discriminator, or one contradicting the boolean beside it, + # is untrustworthy evidence — not an evidence gap. + if self._outcome_invalid: + return "fail" + if ( + self._explicit_outcome == "pass" + and self._explicit_ready is False + ): return "fail" + # A stage that RAN and did not pass is not evidence of passing. `skip` + # covers both a deliberately skipped stage and any status token + # `_norm_status` did not recognise, so neither can be read as a pass. + if any(s.get("status") != "pass" for s in self.stages.values()): + return "incomplete" if self._explicit_outcome is not None: return self._explicit_outcome if self._explicit_ready is not None: @@ -414,6 +452,7 @@ def _fold( profile: str | None = None, testpypi_version: str | None = None, commit_shas: dict[str, str] | None = None, + force_fail: bool = False, ) -> _Accumulator: """Fold the given artifacts into an accumulator (reads only; no writes). @@ -423,6 +462,7 @@ def _fold( carry it and ``run`` persists it from here instead. """ acc = _Accumulator() + acc._force_fail = bool(force_fail) if commit_shas: acc.add_commit_shas(commit_shas) @@ -493,6 +533,7 @@ def ingest( testpypi_version: str | None = None, commit_shas: dict[str, str] | None = None, now: datetime.datetime | None = None, + force_fail: bool = False, ) -> dict[str, Any]: """Fold the given artifacts into a single ``validation_report`` dict. @@ -507,6 +548,7 @@ def ingest( profile=profile, testpypi_version=testpypi_version, commit_shas=commit_shas, + force_fail=force_fail, ), now=now, ) @@ -647,6 +689,7 @@ def run( commit_shas: dict[str, str] | None = None, out: Path | None = None, now: datetime.datetime | None = None, + force_fail: bool = False, ) -> dict[str, Any]: """Ingest, persist ``validation_report.json`` + a history copy, and return it. @@ -665,6 +708,7 @@ def run( profile=profile, testpypi_version=testpypi_version, commit_shas=commit_shas, + force_fail=force_fail, ) report = _distill(acc, now=now) target = out or VALIDATION_REPORT_FILE @@ -690,11 +734,20 @@ def _print_summary(report: dict[str, Any]) -> None: c_fail, c_info, c_meta, c_ok, c_warn, glyph_fail, glyph_ok, glyph_warn, ) + # Report the tri-state, not the legacy boolean: the two can legitimately + # disagree (a stage saying pass while carrying failing counts is + # `release_ready: true` but `validation_outcome: "fail"`), and printing the + # boolean alone rendered a green tick over a failing report. + outcome = report.get("validation_outcome") ready = report.get("release_ready") - if ready is True: + if outcome not in ("pass", "fail", "incomplete"): + outcome = "pass" if ready is True else ("fail" if ready is False else None) + if outcome == "pass": glyph, label = glyph_ok(), c_ok("release_ready") - elif ready is False: - glyph, label = glyph_fail(), c_fail("NOT release_ready") + elif outcome == "fail": + glyph, label = glyph_fail(), c_fail("NOT release_ready (validation FAILED)") + elif outcome == "incomplete": + glyph, label = glyph_warn(), c_warn("incomplete — no rehearsal evidence") else: glyph, label = glyph_warn(), c_warn("release_ready unknown") t = report.get("totals", {}) or {} diff --git a/tests/test_readiness.py b/tests/test_readiness.py index b2b0d9d..180cc08 100644 --- a/tests/test_readiness.py +++ b/tests/test_readiness.py @@ -855,13 +855,16 @@ def test_validation_incomplete_is_stale_not_red(): failed at all. """ report = _green_validation_report() - report["release_ready"] = False + # release_ready deliberately left True: this pins that the *discriminator* + # drives the verdict. With it False the legacy fallback would reach STALE + # even if the readiness change were reverted, so the test would prove + # nothing. report["validation_outcome"] = "incomplete" report["stages"] = {"integrate": {"status": "pass", "profile": "release"}} report["failures"] = [] v = compute(make_snapshot(validation_report=report)) - assert v["verdict"] != "red" - assert not any("release validation FAILED" in r for r in v["red_reasons"]) + assert v["verdict"] == "stale" + assert v["red_reasons"] == [] assert any("release validation incomplete" in r for r in v["stale_reasons"]) @@ -912,3 +915,58 @@ def test_validation_outcome_overrides_stale_release_ready_true(): report["validation_outcome"] = "fail" v = compute(make_snapshot(validation_report=report)) assert v["verdict"] == "red" + + +def test_validation_incomplete_is_stale_even_when_release_ready_is_false(): + """The common shape: both fields set, discriminator decides.""" + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "incomplete" + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "stale" + assert v["red_reasons"] == [] + + +def test_validation_outcome_pass_contradicting_release_ready_false_is_red(): + """Contradictory fields are untrustworthy evidence — believe the pessimist.""" + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "pass" + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" + assert any("release validation FAILED" in r for r in v["red_reasons"]) + + +def test_validation_malformed_discriminator_is_red_even_with_legacy_true(): + """Present-but-unrecognised must not fall back to the optimistic boolean.""" + report = _green_validation_report() + report["release_ready"] = True + report["validation_outcome"] = "PASS" + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" + assert any("release validation FAILED" in r for r in v["red_reasons"]) + + +def test_native_pass_report_is_green_and_keeps_the_fidelity_checks(): + """The green path on the NATIVE schema, not via the legacy fallback. + + The baseline fixture predates `validation_outcome`, so without this the + green-path tests would only ever exercise the compatibility branch. + """ + report = _green_validation_report() + report["validation_outcome"] = "pass" + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "green" + + # ...and the fidelity checks the `pass` branch owns still fire. + stale_src = _green_validation_report() + stale_src["validation_outcome"] = "pass" + stale_src["commit_shas"]["PyAutoLens"] = "9" * 40 + v2 = compute(make_snapshot(validation_report=stale_src)) + assert any("source moved since rehearsal" in r for r in v2["stale_reasons"]) + + wrong_profile = _green_validation_report() + wrong_profile["validation_outcome"] = "pass" + wrong_profile["profile"] = "smoke" + v3 = compute(make_snapshot(validation_report=wrong_profile)) + assert any("is not 'release'" in r for r in v3["stale_reasons"]) diff --git a/tests/test_release_run.py b/tests/test_release_run.py index 7b23adf..4914798 100644 --- a/tests/test_release_run.py +++ b/tests/test_release_run.py @@ -176,3 +176,55 @@ def test_in_progress_still_never_ingests_even_with_a_stale_schema(): d = rr.decide(_report(), {"last_ingested_run_id": 7}, _run(run_id=7, status="in_progress", conclusion=None)) assert d["action"] == "in-progress" + + +def test_migration_never_re_folds_a_report_carrying_adverse_evidence(): + """A legacy RED that never reached the rehearsal must not be softened. + + Such a report has no `rehearse` stage and no discriminator, so the rehearsal + guard alone would let the migration overwrite it with a green integrate-only + artifact — converting a real failure into an evidence gap. + """ + legacy_red = { + "ts": "2026-08-14T20:00:00+00:00", + "release_ready": False, + "stages": {"unit": {"status": "fail"}}, + "totals": {"passed": 0, "failed": 3, "skipped": 0, "timeout": 0}, + "failures": [{"project": "x", "script": "y.py"}], + } + d = rr.decide(legacy_red, {"last_ingested_run_id": 7}, _run(run_id=7)) + assert d["action"] == "cached" + + +def test_migration_blocked_by_failing_totals_alone(): + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", "release_ready": False, + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 9, "failed": 1, "skipped": 0, "timeout": 0}}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_migration_blocked_by_a_failures_list_alone(): + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", "release_ready": False, + "stages": {"integrate": {"status": "pass"}}, + "failures": [{"project": "x", "script": "y.py"}]}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_migration_survives_a_malformed_stages_or_totals_field(): + """Defensive: a corrupt report must not crash the tick.""" + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", "stages": "not-a-dict", "totals": None}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] in ("ingest", "cached") + + +def test_resolve_outcome_reads_incomplete_over_the_legacy_boolean(): + assert rr.resolve_outcome({"release_ready": False, + "validation_outcome": "incomplete"}) == "incomplete" diff --git a/tests/test_validate.py b/tests/test_validate.py index 8439654..c66d733 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -684,3 +684,117 @@ def test_outcome_nothing_ingested_is_incomplete(tmp_path): report = validate.ingest([tmp_path]) assert report["validation_outcome"] == "incomplete" assert report["release_ready"] is False + + +# --- validation_outcome: the fail-closed edges ----------------------------- +# +# Every case below reached `pass` or `incomplete` at some point during review. +# They are the reason the predicate is wider than "did a stage say fail". + + +def test_outcome_per_project_failures_with_clean_totals_is_fail(tmp_path): + """per_project is merged independently of totals, so it can disagree.""" + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict( + INTEGRATE, + summary={"passed": 10, "failed": 0, "skipped": 0, "timeout": 0}, + per_project={"autolens_workspace": + {"passed": 7, "failed": 3, "skipped": 0, "timeout": 0}}, + failures=[], + )) + report = validate.ingest([tmp_path]) + assert report["totals"]["failed"] == 0 + assert report["validation_outcome"] == "fail" + + +def test_outcome_per_project_timeouts_with_clean_totals_is_fail(tmp_path): + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict( + INTEGRATE, + summary={"passed": 10, "failed": 0, "skipped": 0, "timeout": 0}, + per_project={"autolens_workspace": + {"passed": 7, "failed": 0, "skipped": 0, "timeout": 2}}, + failures=[], + )) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_outcome_skipped_stage_is_incomplete_not_pass(tmp_path): + """A stage that ran without passing is not evidence of passing.""" + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict(INTEGRATE, status="skipped")) + report = validate.ingest([tmp_path]) + assert report["stages"]["integrate"]["status"] == "skip" + assert report["validation_outcome"] == "incomplete" + + +def test_outcome_unknown_status_token_is_incomplete_not_pass(tmp_path): + """`_norm_status` folds unrecognised tokens to `skip`, never `fail`. + + So an adverse-sounding token Heart does not know must still not read as a + pass just because the rehearsal succeeded. + """ + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict(INTEGRATE, status="completed_with_failures")) + report = validate.ingest([tmp_path]) + assert report["stages"]["integrate"]["status"] == "skip" + assert report["validation_outcome"] == "incomplete" + + +def test_outcome_contradicting_explicit_fields_fails_closed(tmp_path): + """`validation_outcome: pass` beside `release_ready: false` is not trustworthy.""" + _write(tmp_path / "validation_report.json", { + "schema_version": 1, + "release_ready": False, + "validation_outcome": "pass", + "stages": {"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + "ts": "2026-06-01T00:00:00+00:00", + }) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_outcome_malformed_discriminator_fails_closed(tmp_path): + """Present-but-unrecognised is a malformed artifact, not a legacy report.""" + _write(tmp_path / "validation_report.json", { + "schema_version": 1, + "release_ready": True, + "validation_outcome": "PASS", # wrong case → not a value we accept + "stages": {"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + "ts": "2026-06-01T00:00:00+00:00", + }) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_outcome_absent_discriminator_with_legacy_true_still_passes(tmp_path): + """The compatibility case: no field at all + a genuine legacy `true`.""" + _write(tmp_path / "validation_report.json", { + "schema_version": 1, + "release_ready": True, + "stages": {"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + "ts": "2026-06-01T00:00:00+00:00", + }) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +def test_force_fail_overrides_a_passing_artifact(tmp_path): + """The producing run's own conclusion outranks what its artifact claims. + + A workflow can break outside anything the stage report captures, and that + report is written by a step that may have run before the break. + """ + _write(tmp_path / "stage_report.json", dict(INTEGRATE)) + assert validate.ingest([tmp_path])["validation_outcome"] == "incomplete" + assert validate.ingest([tmp_path], force_fail=True)["validation_outcome"] == "fail" + + +def test_force_fail_beats_even_a_complete_passing_report(tmp_path): + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "integrate.json", dict(INTEGRATE)) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + assert validate.ingest([tmp_path], force_fail=True)["validation_outcome"] == "fail" From f97834c5f75b2dc70e7780223684fdf5d89e44d4 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 13:16:32 -0400 Subject: [PATCH 3/6] fix: one shared outcome normaliser + precedence and migration fixes (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Second Codex review round. Four findings, each reproduced before being fixed and each now mutation-tested. Outcome precedence (one of these was a regression from the previous commit): - An explicit `fail` from a merged base was overridden to `incomplete` when a stage read `skip`, because the not-passed-stage check ran first. That laundered RED into STALE. An explicit `fail` now outranks it. - An explicit `pass` could stand in for rehearsal evidence that was not present, reaching GREEN off a report with only `integrate: pass`. `pass` now always requires a passing `rehearse` stage. - A base's stale `incomplete` survived a rehearsal supplied in the SAME ingest, a genuine false-STALE. Base reports are folded after fresh artifacts, so the explicit value no longer overrides evidence that just arrived. Only an explicit `fail` (and a contradictory/malformed one) short-circuits. Migration guard: - It ignored `per_project` counts, so a legacy RED whose only adverse signal was a failing project could still be overwritten by a green artifact. The guard now uses the same definition of "adverse" as `_Accumulator._has_adverse_evidence`. - It triggered on "outcome not valid" rather than "field absent", so a present-but-malformed discriminator — which readiness deliberately grades RED — also forced migration and could be overwritten. - `decide()` called `.get()` before its `isinstance` check, so a persisted non-object JSON (string/list/number) raised AttributeError and aborted the tick. Consumer drift — the root cause of the presentation findings: Four places re-derived severity from the report inline (readiness, dashboard, the validate CLI summary, the tick status line), so they drifted apart: readiness rejected a malformed discriminator, or `pass` beside `release_ready: false`, while the dashboard row and the CLI beside it still rendered a green `release_ready` for the same report. Added `validate.report_outcome()` as the single normaliser and routed all four through it. Tests — the gaps the review named: - The malformed-input migration test accepted either action, so it passed with the guard reverted; it now asserts exactly. - The F1 fixture combined three adverse signals, so removing any single check would not fail it. Split into one-signal-each tests, plus the missing per-project, timeout-only and malformed-outcome cases. - The force_fail tests only exercised `ingest`, so reverting the tick->`run` hop left them passing. Added a test that drives `rr.main()` and asserts what the tick actually passes, plus its negative case. - Added a `_print_summary` regression test, dashboard tests for the malformed and contradictory reports, a `report_outcome` truth table, and made the rehearsal-only test assert `validation_outcome == "pass"` so the M2 intent is pinned rather than assumed. Mutation-verified: reverting each of the seven new guards individually fails between 1 and 4 tests. Suite 421 passed. Live end-to-end unchanged against the real state and artifact: YELLOW 70, `red_reasons: []`, dashboard row WARN. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- heart/checks/release_run.py | 45 +++++++++----- heart/dashboard.py | 13 ++-- heart/readiness.py | 15 +---- heart/validate.py | 64 ++++++++++++++----- tests/test_dashboard.py | 20 ++++++ tests/test_release_run.py | 120 ++++++++++++++++++++++++++++++++++-- tests/test_validate.py | 94 ++++++++++++++++++++++++++++ 7 files changed, 314 insertions(+), 57 deletions(-) diff --git a/heart/checks/release_run.py b/heart/checks/release_run.py index c719f47..eb1f7f1 100644 --- a/heart/checks/release_run.py +++ b/heart/checks/release_run.py @@ -152,33 +152,48 @@ def decide( # multi-stage ingest during a release drive, which this artifact cannot # reproduce; re-folding would turn its `pass` into an `incomplete`; # - skipped when the stored report carries ANY adverse evidence — a failed - # stage, failing/timed-out counts, or a failures list. Those can come from - # a run that broke before it ever reached the rehearsal, and re-folding a - # green artifact over them would silently convert a real RED into a STALE. + # stage, failing/timed-out counts in `totals` OR in any `per_project` + # entry, or a failures list. Those can come from a run that broke before + # it ever reached the rehearsal, and re-folding a green artifact over them + # would silently convert a real RED into a STALE. The definition of + # "adverse" must match `validate._Accumulator._has_adverse_evidence`; + # when it did not, per-project failures were an escape hatch. + # - triggered only when the field is genuinely ABSENT, not merely invalid. + # A present-but-malformed discriminator is graded RED on purpose, so + # treating it as "predates the schema" would let the migration overwrite + # the very report that RED rests on. # # Note the test is neither `cached` nor `local-fresher`: every ingest happens # after the run it ingests, so "the report is fresher than the run" says # nothing about where the report came from. - stages = (current_report or {}).get("stages") or {} + report = current_report if isinstance(current_report, dict) else {} + stages = report.get("stages") stages = stages if isinstance(stages, dict) else {} - totals = (current_report or {}).get("totals") or {} - totals = totals if isinstance(totals, dict) else {} + + def _adverse_counts(counts: Any) -> bool: + return bool( + isinstance(counts, dict) + and (counts.get("failed", 0) or counts.get("timeout", 0)) + ) + + per_project = report.get("per_project") + per_project = per_project if isinstance(per_project, dict) else {} stored_adverse = ( any(isinstance(s, dict) and s.get("status") == "fail" for s in stages.values()) - or bool(totals.get("failed", 0) or totals.get("timeout", 0)) - or bool((current_report or {}).get("failures")) + or _adverse_counts(report.get("totals")) + or any(_adverse_counts(c) for c in per_project.values()) + or bool(report.get("failures")) ) stale_schema = ( - isinstance(current_report, dict) - and bool(current_report) + bool(report) and "rehearse" not in stages and not stored_adverse - and current_report.get("validation_outcome") not in ("pass", "fail", "incomplete") + and "validation_outcome" not in report ) if not stale_schema: if isinstance(sidecar, dict) and sidecar.get("last_ingested_run_id") == run_id: return {**out, "action": "cached"} - report_ts = _parse_ts((current_report or {}).get("ts")) + report_ts = _parse_ts(report.get("ts")) created = _parse_ts(out["created"]) if report_ts is not None and created is not None and report_ts >= created: return {**out, "action": "local-fresher"} @@ -192,10 +207,8 @@ def resolve_outcome(ingested: dict[str, Any] | None) -> str: network. Reports predating ``validation_outcome`` fall back to the legacy boolean and fail closed. """ - outcome = (ingested or {}).get("validation_outcome") - if outcome in ("pass", "fail", "incomplete"): - return str(outcome) - return "pass" if (ingested or {}).get("release_ready") is True else "fail" + from heart import validate + return validate.report_outcome(ingested) or "fail" def main(argv: list[str] | None = None) -> int: diff --git a/heart/dashboard.py b/heart/dashboard.py index cece2e8..d27544c 100644 --- a/heart/dashboard.py +++ b/heart/dashboard.py @@ -37,6 +37,7 @@ from dataclasses import dataclass, field from typing import Any, Iterable, Sequence +from heart import validate from heart.checks.test_run import counts_measured as tr_counts_measured from heart.heart_color import ( c_bold, c_dim, c_fail, c_info, c_meta, c_ok, c_warn, @@ -571,13 +572,11 @@ def build_board( profile = vr.get("profile") or "?" stages = vr.get("stages") or {} meta = f"v{ver} profile={profile} ({vr.get('ts', '?')})" - # Mirror the readiness gate: read `validation_outcome`, falling back to - # the legacy boolean. `incomplete` is an evidence gap (WARN), not a - # failure (FAIL) — a green integrate-only ingest lands there, and a FAIL - # row beside a stale header verdict reads as a broken release. - outcome = vr.get("validation_outcome") - if outcome not in ("pass", "fail", "incomplete"): - outcome = "fail" if ready is False else ("pass" if ready is True else None) + # Same normaliser the readiness gate uses, so this row can never + # contradict the header verdict. `incomplete` is an evidence gap (WARN), + # not a failure (FAIL) — a green integrate-only ingest lands there, and + # a FAIL row beside a stale header reads as a broken release. + outcome = validate.report_outcome(vr) if outcome == "fail": st, summary = FAIL, f"NOT release_ready — {meta}" elif outcome == "incomplete": diff --git a/heart/readiness.py b/heart/readiness.py index 053cfb0..0edaea5 100644 --- a/heart/readiness.py +++ b/heart/readiness.py @@ -87,7 +87,7 @@ import yaml -from heart import state +from heart import state, validate from heart.checks.ci_status import FAILURE_CONCLUSIONS, load_required_workflows from heart.checks.test_run import counts_measured as tr_counts_measured from heart.heart_color import ( @@ -469,18 +469,7 @@ def scope_local(msg: str, key: str) -> None: # so `false` there stays RED — fail closed. vr = snapshot.get("validation_report") if isinstance(vr, dict) and vr: - ready = vr.get("release_ready") - outcome = vr.get("validation_outcome") - if outcome not in ("pass", "fail", "incomplete"): - if "validation_outcome" in vr: - # Present but unrecognised: a malformed gate artifact is - # untrustworthy evidence, never a pass. - outcome = "fail" - else: - outcome = "fail" if ready is False else ("pass" if ready is True else None) - elif outcome == "pass" and ready is False: - # The two fields contradict each other; believe the pessimistic one. - outcome = "fail" + outcome = validate.report_outcome(vr) if outcome == "incomplete": # Nothing is wrong; the rehearsal evidence is simply absent. The # wording must contain "release validation" — the Health Agent diff --git a/heart/validate.py b/heart/validate.py index ee81b99..1bd442f 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -403,26 +403,28 @@ def validation_outcome(self) -> str: """ if self._force_fail or self._has_adverse_evidence(): return "fail" - # A malformed discriminator, or one contradicting the boolean beside it, - # is untrustworthy evidence — not an evidence gap. + # A malformed discriminator is untrustworthy evidence, not a gap. if self._outcome_invalid: return "fail" - if ( - self._explicit_outcome == "pass" - and self._explicit_ready is False - ): + # An explicit `fail` from a merged base outranks everything below: a + # base saying "this failed" must never be softened by what the stages + # look like now. + if self._explicit_outcome == "fail": return "fail" + if self._explicit_ready is False and self._explicit_outcome in (None, "pass"): + # Either a legacy report whose `false` we cannot explain, or a base + # whose two fields contradict each other. Both fail closed. + return "fail" + # Everything else is decided by the evidence actually folded in, NOT by + # the base's explicit `pass`/`incomplete`. A base's stale `incomplete` + # must not survive a rehearsal supplied in the same ingest, and its + # `pass` must not stand in for rehearsal evidence that isn't here. + # # A stage that RAN and did not pass is not evidence of passing. `skip` # covers both a deliberately skipped stage and any status token - # `_norm_status` did not recognise, so neither can be read as a pass. + # `_norm_status` did not recognise, so neither can read as a pass. if any(s.get("status") != "pass" for s in self.stages.values()): return "incomplete" - if self._explicit_outcome is not None: - return self._explicit_outcome - if self._explicit_ready is not None: - # A legacy base report with no discriminator: false means "not - # ready" and we cannot tell why, so treat it as a failure. - return "pass" if self._explicit_ready else "fail" rehearse = self.stages.get("rehearse") if rehearse and rehearse.get("status") == "pass": return "pass" @@ -446,6 +448,37 @@ def release_ready(self) -> bool: return bool(rehearse and rehearse.get("status") == "pass") +def report_outcome(report: Any) -> str | None: + """Severity of a *persisted* report: ``pass``/``fail``/``incomplete``/None. + + The single normaliser every consumer must use — the readiness gate, the + dashboard, the ``validate`` CLI summary and the tick's status line. Each of + them previously re-derived this inline, which is how they came to disagree: + readiness rejected a malformed discriminator while the dashboard beside it + still rendered a green ``release_ready`` row for the same report. + + Fails closed. ``None`` means "no report / nothing stated" — the caller + decides what an absent verdict means in its own context. + """ + if not isinstance(report, dict) or not report: + return None + outcome = report.get("validation_outcome") + ready = report.get("release_ready") + if outcome in ("pass", "fail", "incomplete"): + # The two fields contradict each other; believe the pessimistic one. + if outcome == "pass" and ready is False: + return "fail" + return str(outcome) + if "validation_outcome" in report: + # Present but unrecognised: malformed, never a pass. + return "fail" + if ready is True: + return "pass" + if ready is False: + return "fail" + return None + + def _fold( sources: Sequence[str | Path], *, @@ -738,10 +771,7 @@ def _print_summary(report: dict[str, Any]) -> None: # disagree (a stage saying pass while carrying failing counts is # `release_ready: true` but `validation_outcome: "fail"`), and printing the # boolean alone rendered a green tick over a failing report. - outcome = report.get("validation_outcome") - ready = report.get("release_ready") - if outcome not in ("pass", "fail", "incomplete"): - outcome = "pass" if ready is True else ("fail" if ready is False else None) + outcome = report_outcome(report) if outcome == "pass": glyph, label = glyph_ok(), c_ok("release_ready") elif outcome == "fail": diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index b76769b..4dae37d 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -358,3 +358,23 @@ def test_machine_surface_carries_stale_reasons(): payload = dashboard.to_dict(board) assert "stale_reasons" in payload assert payload["stale_reasons"] == verdict["stale_reasons"] + + +def test_validation_row_mirrors_the_readiness_normaliser(): + """The row must not contradict the header verdict. + + Both now go through `validate.report_outcome`; when they each re-derived it + inline, readiness graded these RED while the row rendered a green + `release_ready`. + """ + for vr in ( + {"release_ready": False, "validation_outcome": "pass", # contradictory + "testpypi_version": "1", "profile": "release", + "stages": {"integrate": {"status": "pass"}}, "ts": TS}, + {"release_ready": True, "validation_outcome": "PASS", # malformed + "testpypi_version": "1", "profile": "release", + "stages": {"integrate": {"status": "pass"}}, "ts": TS}, + ): + board = dashboard.build_board(make_snapshot(validation_report=vr), + make_verdict("red", 45), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.FAIL diff --git a/tests/test_release_run.py b/tests/test_release_run.py index 4914798..bac7f45 100644 --- a/tests/test_release_run.py +++ b/tests/test_release_run.py @@ -185,12 +185,12 @@ def test_migration_never_re_folds_a_report_carrying_adverse_evidence(): guard alone would let the migration overwrite it with a green integrate-only artifact — converting a real failure into an evidence gap. """ + # ONE adverse signal only — a combined fixture would still pass with any + # single check removed. legacy_red = { "ts": "2026-08-14T20:00:00+00:00", "release_ready": False, "stages": {"unit": {"status": "fail"}}, - "totals": {"passed": 0, "failed": 3, "skipped": 0, "timeout": 0}, - "failures": [{"project": "x", "script": "y.py"}], } d = rr.decide(legacy_red, {"last_ingested_run_id": 7}, _run(run_id=7)) assert d["action"] == "cached" @@ -217,14 +217,126 @@ def test_migration_blocked_by_a_failures_list_alone(): def test_migration_survives_a_malformed_stages_or_totals_field(): - """Defensive: a corrupt report must not crash the tick.""" + """Defensive: a corrupt report must not crash the tick. + + Asserts the exact action — accepting either would pass with the guard gone. + No rehearsal, nothing adverse readable, no discriminator → migrate. + """ d = rr.decide( {"ts": "2026-08-14T20:00:00+00:00", "stages": "not-a-dict", "totals": None}, {"last_ingested_run_id": 7}, _run(run_id=7), ) - assert d["action"] in ("ingest", "cached") + assert d["action"] == "ingest" + + +def test_decide_survives_a_report_that_is_not_an_object(): + """`validate.load()` returns whatever JSON was on disk, not always a dict.""" + for junk in ("a string", ["a", "list"], 42): + assert rr.decide(junk, None, _run(run_id=7))["action"] in ( + "ingest", "cached", "local-fresher", + ) def test_resolve_outcome_reads_incomplete_over_the_legacy_boolean(): assert rr.resolve_outcome({"release_ready": False, "validation_outcome": "incomplete"}) == "incomplete" + + +def test_migration_blocked_by_timeouts_alone(): + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 9, "failed": 0, "skipped": 0, "timeout": 1}}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_migration_blocked_by_per_project_counts_alone(): + """`per_project` is merged independently of `totals`, so it can be the only + adverse signal — and the guard must use the same definition of "adverse" + that `validate._has_adverse_evidence` does.""" + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 9, "failed": 0, "skipped": 0, "timeout": 0}, + "per_project": {"autolens_workspace": + {"passed": 6, "failed": 1, "skipped": 0, "timeout": 0}}}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_migration_not_triggered_by_a_present_but_malformed_discriminator(): + """Readiness grades a malformed discriminator RED on purpose. + + Treating it as "predates the schema" would let the migration overwrite the + very report that RED rests on. + """ + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", "validation_outcome": "FAILED", + "stages": {"integrate": {"status": "pass"}}}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached" + + +def test_tick_forces_fail_when_the_run_conclusion_is_not_success(monkeypatch, tmp_path): + """Pins the tick -> validate.run hop, not just `ingest(force_fail=True)`. + + Reverting either the tick's argument or `run`'s threading of it must fail + here; asserting on `ingest` alone would not catch that. + """ + import json as _json + from heart import validate + + (tmp_path / "stage_report.json").write_text(_json.dumps({ + "stage": "integrate", "status": "pass", "profile": "release", + "summary": {"passed": 50, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + })) + + seen = {} + real_run = validate.run + + def spy(sources, **kw): + seen.update(kw) + return real_run(sources, **kw) + + # `main()` imports these inside the function body, so patch the modules. + monkeypatch.setattr(validate, "run", spy) + monkeypatch.setattr(validate, "load", lambda: None) + monkeypatch.setattr(rr, "latest_run", lambda: _run(run_id=99, conclusion="failure")) + monkeypatch.setattr(rr, "download_stage_report", + lambda run_id, dest: tmp_path / "stage_report.json") + monkeypatch.setattr(rr, "_read_json", lambda p: None) + + rr.main([]) + assert seen.get("force_fail") is True + + +def test_tick_does_not_force_fail_on_a_successful_run(monkeypatch, tmp_path): + import json as _json + from heart import validate + + (tmp_path / "stage_report.json").write_text(_json.dumps({ + "stage": "integrate", "status": "pass", "profile": "release", + "summary": {"passed": 50, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], + })) + seen = {} + real_run = validate.run + + def spy(sources, **kw): + seen.update(kw) + return real_run(sources, **kw) + + monkeypatch.setattr(validate, "run", spy) + monkeypatch.setattr(validate, "load", lambda: None) + monkeypatch.setattr(rr, "latest_run", lambda: _run(run_id=98, conclusion="success")) + monkeypatch.setattr(rr, "download_stage_report", + lambda run_id, dest: tmp_path / "stage_report.json") + monkeypatch.setattr(rr, "_read_json", lambda p: None) + + rr.main([]) + assert seen.get("force_fail") is False diff --git a/tests/test_validate.py b/tests/test_validate.py index c66d733..61e62f6 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -60,6 +60,9 @@ def test_ingest_rehearsal_only(tmp_path): assert report["stages"]["rehearse"]["run_id"] == "645" # rehearsal artifact presence = build succeeded → release_ready True (pass axis) assert report["release_ready"] is True + # The M2 intent: a rehearsal-only report is a genuine `pass`, gated on + # fidelity by readiness rather than downgraded to an evidence gap here. + assert report["validation_outcome"] == "pass" # no integration stage yet → no release profile (the gate keeps this YELLOW) assert report["profile"] is None # the build sha is recorded under PyAutoHands, not a library head @@ -798,3 +801,94 @@ def test_force_fail_beats_even_a_complete_passing_report(tmp_path): _write(tmp_path / "integrate.json", dict(INTEGRATE)) assert validate.ingest([tmp_path])["validation_outcome"] == "pass" assert validate.ingest([tmp_path], force_fail=True)["validation_outcome"] == "fail" + + +# --- precedence inside validation_outcome ---------------------------------- + + +def test_explicit_fail_outranks_a_non_passing_stage(tmp_path): + """A base saying "this failed" must not be softened to an evidence gap. + + Ordering the not-passed-stage check before the explicit outcome laundered + RED into STALE. + """ + _write(tmp_path / "validation_report.json", { + "schema_version": 1, "release_ready": False, "validation_outcome": "fail", + "stages": {"integrate": {"status": "skip"}}, + "totals": {"passed": 0, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": "2026-06-01T00:00:00+00:00", + }) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_explicit_pass_cannot_substitute_for_missing_rehearsal(tmp_path): + """`pass` requires rehearsal evidence to actually be present.""" + _write(tmp_path / "validation_report.json", { + "schema_version": 1, "release_ready": True, "validation_outcome": "pass", + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": "2026-06-01T00:00:00+00:00", + }) + assert validate.ingest([tmp_path])["validation_outcome"] == "incomplete" + + +def test_stale_incomplete_base_is_upgraded_by_a_fresh_rehearsal(tmp_path): + """The false-STALE case: evidence supplied in the same ingest wins. + + Base reports are folded after fresh artifacts, so a base's explicit + `incomplete` must not override a rehearsal that has just arrived. + """ + _write(tmp_path / "validation_report.json", { + "schema_version": 1, "release_ready": False, "validation_outcome": "incomplete", + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": "2026-06-01T00:00:00+00:00", + }) + _write(tmp_path / "rehearsal.json", REHEARSAL) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +# --- report_outcome: the one normaliser every consumer shares -------------- + + +@pytest.mark.parametrize("report,expected", [ + ({"validation_outcome": "pass", "release_ready": True}, "pass"), + ({"validation_outcome": "fail", "release_ready": False}, "fail"), + ({"validation_outcome": "incomplete", "release_ready": False}, "incomplete"), + # contradiction → believe the pessimistic field + ({"validation_outcome": "pass", "release_ready": False}, "fail"), + # present but unrecognised → malformed, never a pass + ({"validation_outcome": "PASS", "release_ready": True}, "fail"), + ({"validation_outcome": None, "release_ready": True}, "fail"), + # absent field → legacy compatibility + ({"release_ready": True}, "pass"), + ({"release_ready": False}, "fail"), + ({"release_ready": None}, None), + ({}, None), + (None, None), + ("not-a-dict", None), +]) +def test_report_outcome(report, expected): + assert validate.report_outcome(report) == expected + + +def test_print_summary_reports_the_tri_state_not_the_legacy_boolean(capsys): + """The CLI printed a green tick over a failing report. + + `release_ready` and `validation_outcome` legitimately disagree when a stage + says pass while carrying failing counts. + """ + validate._print_summary({ + "release_ready": True, "validation_outcome": "fail", + "totals": {"passed": 10, "failed": 2, "skipped": 0, "timeout": 0}, + "stages": {"integrate": {"status": "pass"}}, + }) + out = capsys.readouterr().out + assert "NOT release_ready" in out + + validate._print_summary({ + "release_ready": False, "validation_outcome": "incomplete", + "totals": {"passed": 10, "failed": 0, "skipped": 0, "timeout": 0}, + "stages": {"integrate": {"status": "pass"}}, + }) + assert "no rehearsal evidence" in capsys.readouterr().out From d37ef3832931958c582ab1edaa6a6a3f69a06e25 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 13:29:24 -0400 Subject: [PATCH 4/6] fix: derive release_ready, make adverse verdicts sticky (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Third Codex review round. Three findings, each reproduced then fixed and mutation-tested. The first was a regression from the previous commit. - **A passing ingest could be emitted as a false RED.** Letting a fresh rehearsal upgrade a stale `incomplete` base fixed `validation_outcome`, but `release_ready()` still returned the base's stale explicit `false`. The producer emitted `release_ready: false` beside `validation_outcome: "pass"`, which every consumer then normalised back to `fail` via the contradiction rule — manufacturing a RED out of a green ingest. `release_ready()` is now DERIVED (`validation_outcome() == "pass"`), so the producer cannot emit a self-contradictory report at all and `report_outcome`'s reconciliation only has to cope with hand-edited or foreign ones. - **An explicit `fail` was not sticky.** `--ingest` folds a whole directory in filename order, and `add_report` assigned rather than accumulated, so a later `z_pass.json` silently cleared an earlier `a_failed.json`. Adverse verdicts (`validation_outcome: fail`, `release_ready: false`) are now sticky; nothing about file ordering on disk can clear a recorded failure. - **The migration guard did not normalise stage statuses.** It matched the literal `"fail"` while the accumulator folds `"failure"`/`"timed_out"` to it, so a stored report with a synonym looked benign and could be overwritten by a green artifact. It now calls `_norm_status`, the same way the ingest does. The test that missed the first finding asserted only the raw discriminator, not what a consumer sees; it now asserts `report_outcome` and the boolean too. Added an invariant test that no emitted report is ever self-contradictory, across five shapes. Re-checked that deriving the boolean does not block a real release: the manual release-drive ingest, an idempotent re-ingest, and the M2 rehearsal-only report all still emit `release_ready: true` / `pass`. Mutation-verified: reverting each of the four new guards fails a test. Suite 425 passed. Live end-to-end unchanged: YELLOW 70, `red_reasons: []`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- heart/checks/release_run.py | 11 ++++++- heart/validate.py | 39 +++++++++++++++---------- tests/test_release_run.py | 16 ++++++++++ tests/test_validate.py | 58 ++++++++++++++++++++++++++++++++++++- 4 files changed, 106 insertions(+), 18 deletions(-) diff --git a/heart/checks/release_run.py b/heart/checks/release_run.py index eb1f7f1..9a57b94 100644 --- a/heart/checks/release_run.py +++ b/heart/checks/release_run.py @@ -166,6 +166,8 @@ def decide( # Note the test is neither `cached` nor `local-fresher`: every ingest happens # after the run it ingests, so "the report is fresher than the run" says # nothing about where the report came from. + from heart import validate + report = current_report if isinstance(current_report, dict) else {} stages = report.get("stages") stages = stages if isinstance(stages, dict) else {} @@ -179,7 +181,14 @@ def _adverse_counts(counts: Any) -> bool: per_project = report.get("per_project") per_project = per_project if isinstance(per_project, dict) else {} stored_adverse = ( - any(isinstance(s, dict) and s.get("status") == "fail" for s in stages.values()) + # Normalise exactly as the ingest does. A stored report can carry a + # synonym ("failure", "timed_out"), which the accumulator folds to + # `fail`; matching only the literal token here let such a report look + # benign and be overwritten by a green artifact. + any( + isinstance(s, dict) and validate._norm_status(s.get("status")) == "fail" + for s in stages.values() + ) or _adverse_counts(report.get("totals")) or any(_adverse_counts(c) for c in per_project.values()) or bool(report.get("failures")) diff --git a/heart/validate.py b/heart/validate.py index 1bd442f..203f940 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -351,12 +351,19 @@ def add_report(self, data: dict[str, Any]) -> None: self.failures.append(f) for k, v in (data.get("run_urls") or {}).items(): self.run_urls.setdefault(str(k), str(v)) + # Adverse verdicts are STICKY across bases. `--ingest` accepts a whole + # directory and folds every report in it in filename order, so without + # this a later `z_pass.json` silently overwrites an earlier + # `a_failed.json` and the failure disappears. Nothing about the ordering + # of files on disk should be able to clear a recorded failure. if isinstance(data.get("release_ready"), bool): - self._explicit_ready = data["release_ready"] + if self._explicit_ready is not False: + self._explicit_ready = data["release_ready"] if "validation_outcome" in data: outcome = data.get("validation_outcome") if outcome in ("pass", "fail", "incomplete"): - self._explicit_outcome = str(outcome) + if self._explicit_outcome != "fail": + self._explicit_outcome = str(outcome) else: # Present but not a value we recognise: the report is malformed, # and a malformed gate artifact must never read as a pass. @@ -431,21 +438,21 @@ def validation_outcome(self) -> str: return "incomplete" def release_ready(self) -> bool: - """True iff no ran stage failed AND the rehearse stage passed. - - The rehearse stage is mandatory: a report with nothing built is not - release-ready. An explicit ``release_ready`` from a merged base report is - honoured only when no stage contradicts it with a failure. - - Unchanged and kept for compatibility. It cannot distinguish "failed" - from "incomplete" — read ``validation_outcome()`` for that. + """The legacy boolean, now DERIVED from :meth:`validation_outcome`. + + It used to be computed independently, and the two could then disagree + inside a single emitted report — an incomplete base upgraded by a fresh + rehearsal produced ``release_ready: false`` beside + ``validation_outcome: "pass"``, which every consumer then normalised + back to ``fail``, manufacturing a RED out of a passing ingest. One + source of truth removes that class of contradiction at the producer, + so the reconciliation in :func:`report_outcome` only ever has to cope + with hand-edited or foreign reports. + + It still cannot distinguish "failed" from "incomplete" — that is what + ``validation_outcome`` is for. """ - if any(s.get("status") == "fail" for s in self.stages.values()): - return False - if self._explicit_ready is not None: - return bool(self._explicit_ready) - rehearse = self.stages.get("rehearse") - return bool(rehearse and rehearse.get("status") == "pass") + return self.validation_outcome() == "pass" def report_outcome(report: Any) -> str | None: diff --git a/tests/test_release_run.py b/tests/test_release_run.py index bac7f45..f4fa580 100644 --- a/tests/test_release_run.py +++ b/tests/test_release_run.py @@ -340,3 +340,19 @@ def spy(sources, **kw): rr.main([]) assert seen.get("force_fail") is False + + +def test_migration_blocked_by_a_stage_status_synonym(): + """The guard must normalise statuses the way the ingest does. + + A stored `"failure"` folds to `fail` in the accumulator, so matching only + the literal token here made such a report look benign and let a green + artifact overwrite it. + """ + for token in ("failure", "timed_out", "error"): + d = rr.decide( + {"ts": "2026-08-14T20:00:00+00:00", "release_ready": False, + "stages": {"integrate": {"status": token}}}, + {"last_ingested_run_id": 7}, _run(run_id=7), + ) + assert d["action"] == "cached", token diff --git a/tests/test_validate.py b/tests/test_validate.py index 61e62f6..d231f98 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -845,7 +845,14 @@ def test_stale_incomplete_base_is_upgraded_by_a_fresh_rehearsal(tmp_path): "failures": [], "ts": "2026-06-01T00:00:00+00:00", }) _write(tmp_path / "rehearsal.json", REHEARSAL) - assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "pass" + # ...and what a CONSUMER sees, which is the part that actually gated. The + # emitted report used to carry `release_ready: false` beside that `pass`, + # and every consumer normalised the contradiction back to `fail` — a RED + # manufactured out of a passing ingest. + assert report["release_ready"] is True + assert validate.report_outcome(report) == "pass" # --- report_outcome: the one normaliser every consumer shares -------------- @@ -892,3 +899,52 @@ def test_print_summary_reports_the_tri_state_not_the_legacy_boolean(capsys): "stages": {"integrate": {"status": "pass"}}, }) assert "no rehearsal evidence" in capsys.readouterr().out + + +def test_emitted_reports_are_never_self_contradictory(tmp_path): + """`release_ready` is derived, so the producer cannot emit `false` + `pass`. + + Anything self-contradictory is normalised to `fail` by every consumer, so a + producer able to emit it manufactures REDs. + """ + cases = [ + {"rehearsal.json": REHEARSAL}, # M2 + {"rehearsal.json": REHEARSAL, "integrate.json": dict(INTEGRATE)}, # full pass + {"integrate.json": dict(INTEGRATE)}, # incomplete + {"integrate.json": dict(INTEGRATE, status="fail")}, # fail + {"integrate.json": dict( # adverse counts + INTEGRATE, summary={"passed": 1, "failed": 1, "skipped": 0, "timeout": 0})}, + ] + for i, files in enumerate(cases): + d = tmp_path / f"case{i}" + d.mkdir() + for name, payload in files.items(): + _write(d / name, payload) + report = validate.ingest([d]) + assert validate.report_outcome(report) == report["validation_outcome"], report + assert report["release_ready"] is (report["validation_outcome"] == "pass") + + +def test_explicit_fail_is_sticky_across_multiple_bases(tmp_path): + """`--ingest` folds a whole directory; file order must not clear a failure.""" + common = { + "schema_version": 1, "release_ready": True, + "stages": {"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": "2026-06-01T00:00:00+00:00", + } + _write(tmp_path / "a_failed.json", dict(common, validation_outcome="fail")) + _write(tmp_path / "z_pass.json", dict(common, validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_explicit_not_ready_is_sticky_across_multiple_bases(tmp_path): + common = { + "schema_version": 1, + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": "2026-06-01T00:00:00+00:00", + } + _write(tmp_path / "a_notready.json", dict(common, release_ready=False)) + _write(tmp_path / "z_ready.json", dict(common, release_ready=True)) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" From c7ba80358bc367dc2e6f9af80cbd369866af8e98 Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 13:47:04 -0400 Subject: [PATCH 5/6] fix: order merged bases by recency; restore legacy re-ingest (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fourth Codex review round. Three findings, each reproduced then fixed and mutation-tested. The first was a regression from the previous commit. - **Blanket `release_ready: false` stickiness manufactured REDs.** Making every `false` sticky treated `false` beside `validation_outcome: "incomplete"` — which means "evidence missing", not "something failed" — as a recorded failure. A stale attempt left in a re-used artifacts directory then poisoned every later one: an older `false/incomplete` plus a newer `true/pass` emitted `false/fail`, and swapping the two files changed the answer. Merged bases are now ordered properly. A base is a SEED, not evidence: when the ingest also folded first-hand stage artifacts, those decide and the base's verdict is ignored — the same rule its counts already followed. Otherwise the NEWEST base wins by `ts`, which makes the result independent of filename order. Only on an equal or unparseable `ts` may a base escalate to adverse and never soften, so file order still cannot clear a recorded failure. "Adverse" now means an explicit `fail`, or a `false` with no discriminator to explain it — never a benign `false`/`incomplete` pair. - **Idempotent re-ingest of a legacy report was broken.** A report predating `validation_outcome` states only `release_ready: true`; folding it back demoted it to `incomplete`, contradicting the schema's idempotence promise and the stated boolean compatibility. Such a report is honoured again. A report that DOES carry the discriminator gets no such benefit — an explicit `pass` still never substitutes for absent rehearsal evidence. - **Malformed `stages` crashed the consumers.** `(x or {}).items()` catches an empty list but not a populated one, so a non-empty non-dict raised AttributeError in readiness, the dashboard and the CLI summary — breaking readiness's "never raises on partial/malformed data" contract. All three now isinstance-check. Tests: added the permutation coverage the review named — newest-base-wins in both filename orders, stale-failure-vs-newer-pass in both directions, equal timestamps with the adverse base folded first AND second (only the second exercises the escalation path), benign-gap non-stickiness, the accumulating artifacts directory, legacy idempotence, and malformed `stages` at every consumer. Mutation-verified: reverting each of the seven guards fails a test. Suite 441 passed. Live end-to-end unchanged: YELLOW 70, `red_reasons: []`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- heart/dashboard.py | 3 +- heart/readiness.py | 3 +- heart/validate.py | 98 +++++++++++++++++++++++------- tests/test_dashboard.py | 9 +++ tests/test_readiness.py | 15 +++++ tests/test_validate.py | 131 ++++++++++++++++++++++++++++++++++++++++ 6 files changed, 235 insertions(+), 24 deletions(-) diff --git a/heart/dashboard.py b/heart/dashboard.py index d27544c..5dfa98a 100644 --- a/heart/dashboard.py +++ b/heart/dashboard.py @@ -570,7 +570,8 @@ def build_board( ready = vr.get("release_ready") ver = vr.get("testpypi_version") or "?" profile = vr.get("profile") or "?" - stages = vr.get("stages") or {} + stages = vr.get("stages") + stages = stages if isinstance(stages, dict) else {} meta = f"v{ver} profile={profile} ({vr.get('ts', '?')})" # Same normaliser the readiness gate uses, so this row can never # contradict the header verdict. `incomplete` is an evidence gap (WARN), diff --git a/heart/readiness.py b/heart/readiness.py index 0edaea5..751b2f6 100644 --- a/heart/readiness.py +++ b/heart/readiness.py @@ -477,8 +477,9 @@ def scope_local(msg: str, key: str) -> None: stale.append("release validation incomplete: no rehearsal for current source") hit("validation_absent") elif outcome == "fail": + vr_stages = vr.get("stages") failed_stages = [ - n for n, s in (vr.get("stages") or {}).items() + n for n, s in (vr_stages if isinstance(vr_stages, dict) else {}).items() if isinstance(s, dict) and s.get("status") == "fail" ] red.append( diff --git a/heart/validate.py b/heart/validate.py index 203f940..255cdd6 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -142,6 +142,15 @@ def _read_json(path: Path) -> Any: return None +def _parse_iso(value: Any) -> datetime.datetime | None: + """Parse an ISO timestamp, or None. Used to order merged base reports.""" + try: + t = datetime.datetime.fromisoformat(str(value)) + except (TypeError, ValueError): + return None + return t.replace(tzinfo=datetime.timezone.utc) if t.tzinfo is None else t + + def _norm_status(value: Any) -> str: """Map a stage/status token onto pass|fail|skip (unknown → ``skip``).""" s = str(value or "").strip().lower() @@ -226,6 +235,9 @@ def __init__(self) -> None: self._explicit_outcome: str | None = None self._outcome_invalid = False self._force_fail = False + self._base_seen = False + self._base_ts: datetime.datetime | None = None + self._base_adverse = False # True once a real stage artifact (add_stage) has contributed counts. # add_report() consults this so merging an old validation_report.json # as a "base" never double-counts totals/per_project/failures that a @@ -351,23 +363,54 @@ def add_report(self, data: dict[str, Any]) -> None: self.failures.append(f) for k, v in (data.get("run_urls") or {}).items(): self.run_urls.setdefault(str(k), str(v)) - # Adverse verdicts are STICKY across bases. `--ingest` accepts a whole - # directory and folds every report in it in filename order, so without - # this a later `z_pass.json` silently overwrites an earlier - # `a_failed.json` and the failure disappears. Nothing about the ordering - # of files on disk should be able to clear a recorded failure. - if isinstance(data.get("release_ready"), bool): - if self._explicit_ready is not False: - self._explicit_ready = data["release_ready"] - if "validation_outcome" in data: - outcome = data.get("validation_outcome") - if outcome in ("pass", "fail", "incomplete"): - if self._explicit_outcome != "fail": - self._explicit_outcome = str(outcome) - else: - # Present but not a value we recognise: the report is malformed, - # and a malformed gate artifact must never read as a pass. - self._outcome_invalid = True + # --- the base's own verdict -------------------------------------- + # + # A base report is a SEED, not evidence. Three rules, in order: + # + # 1. If this ingest also folded first-hand stage artifacts, THEY are the + # evidence and the base's verdict is ignored outright — the same rule + # its counts already follow just above. Without this, a stale failed + # report left in a re-used artifacts directory permanently poisons + # every later attempt that writes into the same directory. + # 2. Otherwise the NEWEST base wins, by `ts`. Ingest walks a directory, + # so which base is "last" is an accident of filename order; recency + # is the only defensible ordering, and it makes the result + # order-independent. + # 3. On an equal or unparseable `ts` we cannot tell which came first, so + # a base may only ESCALATE to adverse, never soften. File order on + # disk must not be able to clear a recorded failure. + outcome = data.get("validation_outcome") if "validation_outcome" in data else None + valid_outcome = outcome if outcome in ("pass", "fail", "incomplete") else None + if outcome is not None and valid_outcome is None: + # Present but not a value we recognise: the report is malformed, and + # a malformed gate artifact must never read as a pass. Sticky. + self._outcome_invalid = True + ready = data["release_ready"] if isinstance(data.get("release_ready"), bool) else None + # `false` alone is adverse only when we cannot see why; paired with an + # explicit `incomplete` it just means "evidence missing", which is benign + # and must not be sticky. + adverse = valid_outcome == "fail" or (ready is False and outcome is None) + + if self._stage_counts_seen: + return + + ts = _parse_iso(data.get("ts")) + strictly_newer = ts is not None and (self._base_ts is None or ts > self._base_ts) + if self._base_seen and not strictly_newer: + if adverse: + self._base_adverse = True + if valid_outcome == "fail": + self._explicit_outcome = "fail" + if ready is False: + self._explicit_ready = False + return + + self._base_seen = True + if ts is not None: + self._base_ts = ts + self._explicit_outcome = valid_outcome + self._explicit_ready = ready + self._base_adverse = adverse @staticmethod def _counts_adverse(counts: Any) -> bool: @@ -416,11 +459,10 @@ def validation_outcome(self) -> str: # An explicit `fail` from a merged base outranks everything below: a # base saying "this failed" must never be softened by what the stages # look like now. - if self._explicit_outcome == "fail": + if self._base_adverse or self._explicit_outcome == "fail": return "fail" - if self._explicit_ready is False and self._explicit_outcome in (None, "pass"): - # Either a legacy report whose `false` we cannot explain, or a base - # whose two fields contradict each other. Both fail closed. + if self._explicit_outcome == "pass" and self._explicit_ready is False: + # A base whose two fields contradict each other. Fails closed. return "fail" # Everything else is decided by the evidence actually folded in, NOT by # the base's explicit `pass`/`incomplete`. A base's stale `incomplete` @@ -435,6 +477,14 @@ def validation_outcome(self) -> str: rehearse = self.stages.get("rehearse") if rehearse and rehearse.get("status") == "pass": return "pass" + if self._explicit_outcome is None and self._explicit_ready is True: + # Legacy compatibility: a report predating `validation_outcome` + # states only `release_ready: true`, and the schema's idempotent + # re-ingest promise means folding it back must not silently demote + # it. A report that DOES carry the discriminator gets no such + # benefit — an explicit `pass` never substitutes for the rehearsal + # evidence it claims. + return "pass" return "incomplete" def release_ready(self) -> bool: @@ -788,7 +838,11 @@ def _print_summary(report: dict[str, Any]) -> None: else: glyph, label = glyph_warn(), c_warn("release_ready unknown") t = report.get("totals", {}) or {} - stages = ", ".join(f"{n}:{s.get('status', '?')}" for n, s in (report.get("stages") or {}).items()) + _stages = report.get("stages") + stages = ", ".join( + f"{n}:{s.get('status', '?') if isinstance(s, dict) else '?'}" + for n, s in (_stages if isinstance(_stages, dict) else {}).items() + ) version = report.get("testpypi_version") or "?" prof = report.get("profile") or "?" print(f"{glyph} {c_info('validate')} {label} {c_meta(f'v{version} profile={prof}')}") diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 4dae37d..187722b 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -378,3 +378,12 @@ def test_validation_row_mirrors_the_readiness_normaliser(): board = dashboard.build_board(make_snapshot(validation_report=vr), make_verdict("red", 45), now=FRESH_NOW) assert _section(board, "release_validation").state == dashboard.FAIL + + +def test_malformed_stages_does_not_break_the_board(): + vr = {"release_ready": False, "validation_outcome": "fail", + "testpypi_version": "1", "profile": "release", + "stages": [{"stage": "integrate"}], "ts": TS} + board = dashboard.build_board(make_snapshot(validation_report=vr), + make_verdict("red", 45), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.FAIL diff --git a/tests/test_readiness.py b/tests/test_readiness.py index 180cc08..98a843b 100644 --- a/tests/test_readiness.py +++ b/tests/test_readiness.py @@ -970,3 +970,18 @@ def test_native_pass_report_is_green_and_keeps_the_fidelity_checks(): wrong_profile["profile"] = "smoke" v3 = compute(make_snapshot(validation_report=wrong_profile)) assert any("is not 'release'" in r for r in v3["stale_reasons"]) + + +@pytest.mark.parametrize("stages", [[{"stage": "integrate"}], "a-string", 42, None]) +def test_malformed_stages_never_raises(stages): + """`compute` promises never to raise on partial/malformed data. + + `(x or {}).items()` catches an EMPTY list but not a populated one, so the + guard has to be an isinstance check. + """ + report = _green_validation_report() + report["release_ready"] = False + report["validation_outcome"] = "fail" + report["stages"] = stages + v = compute(make_snapshot(validation_report=report)) + assert v["verdict"] == "red" diff --git a/tests/test_validate.py b/tests/test_validate.py index d231f98..1b76fce 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -948,3 +948,134 @@ def test_explicit_not_ready_is_sticky_across_multiple_bases(tmp_path): _write(tmp_path / "a_notready.json", dict(common, release_ready=False)) _write(tmp_path / "z_ready.json", dict(common, release_ready=True)) assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +# --- ordering across merged base reports ----------------------------------- +# +# `--ingest` walks a directory, so which base is folded "last" is an accident of +# filename order. Recency decides; an equal/unparseable ts falls back to +# "adverse wins" so file order can never clear a recorded failure. + +_OLD_TS = "2026-06-01T00:00:00+00:00" +_NEW_TS = "2026-08-01T00:00:00+00:00" + + +def _base(ts, **kw): + return dict({ + "schema_version": 1, + "stages": {"rehearse": {"status": "pass"}, "integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": ts, + }, **kw) + + +@pytest.mark.parametrize("first_name,second_name", [ + ("a_old.json", "z_new.json"), # older sorts first + ("z_old.json", "a_new.json"), # older sorts LAST — result must not change +]) +def test_newest_base_wins_regardless_of_filename_order(tmp_path, first_name, second_name): + _write(tmp_path / first_name, _base(_OLD_TS, release_ready=False, + validation_outcome="incomplete")) + _write(tmp_path / second_name, _base(_NEW_TS, release_ready=True, + validation_outcome="pass")) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "pass" + assert report["release_ready"] is True + + +def test_a_stale_failure_does_not_poison_a_newer_successful_attempt(tmp_path): + """A retried release drive writing into the same artifacts directory.""" + _write(tmp_path / "a_old.json", _base(_OLD_TS, release_ready=False, + validation_outcome="fail")) + _write(tmp_path / "z_new.json", _base(_NEW_TS, release_ready=True, + validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +def test_a_newer_failure_is_not_cleared_by_an_older_pass(tmp_path): + _write(tmp_path / "a_new.json", _base(_NEW_TS, release_ready=False, + validation_outcome="fail")) + _write(tmp_path / "z_old.json", _base(_OLD_TS, release_ready=True, + validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +def test_fresh_stage_artifacts_outrank_a_stale_failed_base(tmp_path): + """The accumulating-artifacts-directory case. + + A base is a seed, not evidence. When the ingest also folded first-hand stage + artifacts they decide — otherwise one old failed report in a re-used + directory blocks every later attempt written beside it, forever. + """ + _write(tmp_path / "validation_report.json", + _base(_OLD_TS, release_ready=False, validation_outcome="fail")) + _write(tmp_path / "rehearsal.json", REHEARSAL) + _write(tmp_path / "stage_report.json", dict(INTEGRATE)) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +def test_benign_not_ready_is_not_sticky(tmp_path): + """`release_ready: false` beside `incomplete` means "evidence missing". + + That is not adverse, so it must not behave like a recorded failure. + """ + _write(tmp_path / "a_gap.json", _base(_OLD_TS, release_ready=False, + validation_outcome="incomplete")) + _write(tmp_path / "z_ok.json", _base(_NEW_TS, release_ready=True, + validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +def test_legacy_report_re_ingest_is_idempotent(tmp_path): + """The schema promises idempotent full-report re-ingest. + + A legacy report states only `release_ready: true`; folding it back must not + silently demote it to an evidence gap. A report that DOES carry the + discriminator gets no such benefit (see + test_explicit_pass_cannot_substitute_for_missing_rehearsal). + """ + legacy = { + "schema_version": 1, "release_ready": True, + "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 5, "failed": 0, "skipped": 0, "timeout": 0}, + "failures": [], "ts": _OLD_TS, + } + assert validate.report_outcome(legacy) == "pass" + _write(tmp_path / "validation_report.json", legacy) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] == "pass" + assert report["release_ready"] is True + assert validate.report_outcome(report) == "pass" + + +@pytest.mark.parametrize("fail_name,pass_name", [ + ("a_failed.json", "z_pass.json"), # adverse base folded FIRST + ("z_failed.json", "a_pass.json"), # adverse base folded SECOND +]) +def test_equal_timestamps_let_a_base_escalate_but_never_soften( + tmp_path, fail_name, pass_name +): + """With equal timestamps we cannot tell which attempt came first. + + So a base may only escalate to adverse. Both permutations are needed: with + the failure folded first the initial assignment already records it, and only + the second permutation exercises the escalation path. + """ + _write(tmp_path / fail_name, _base(_OLD_TS, release_ready=True, + validation_outcome="fail")) + _write(tmp_path / pass_name, _base(_OLD_TS, release_ready=True, + validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail" + + +@pytest.mark.parametrize("gap_name,pass_name", [ + ("a_gap.json", "z_pass.json"), + ("z_gap.json", "a_pass.json"), +]) +def test_equal_timestamps_do_not_let_a_benign_gap_block(tmp_path, gap_name, pass_name): + """The escalation path must fire only for genuinely adverse bases.""" + _write(tmp_path / gap_name, _base(_OLD_TS, release_ready=False, + validation_outcome="incomplete")) + _write(tmp_path / pass_name, _base(_OLD_TS, release_ready=True, + validation_outcome="pass")) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" From 46dd28e42928d72d25a3d6f7b5a3fe98c1c2a03d Mon Sep 17 00:00:00 2001 From: Jammy2211 Date: Fri, 14 Aug 2026 14:00:05 -0400 Subject: [PATCH 6/6] fix: evidence outranks declarations; subordinate bases fully (#144) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fifth Codex review round. Five findings, each reproduced then fixed and mutation-tested. - **A declaration could beat the evidence beside it (fail-open, pre-existing).** `report_outcome` inspected only the two verdict fields, so a legacy report stating `release_ready: true` next to `stages.integrate: fail`, failing counts, per-project failures or a non-empty failures list normalised to `pass` and reached GREEN at every consumer. It now reconciles the report's contents first. This predates the PR — readiness read the same boolean directly on main — but the single normaliser is the right place to close it. - **A superseded base could lend its rehearsal to fresh artifacts.** When fresh stage artifacts were present the base's counts and verdict were skipped, but its *stages* still merged, so a stale `rehearse: pass` combined with a fresh integrate-only artifact and read as complete evidence — laundering a force-failed run into `pass`. A base is now subordinate in full: only the scalar fields seed through. - **Base ordering was asymmetric.** Every base that was "not strictly newer" could escalate to adverse, including one we could order and knew to be older. An old failure therefore pinned RED when it happened to be folded second but not when folded first. A strictly older base is now superseded outright; escalation is only for timestamps we genuinely cannot order. - **`validation_outcome: null` was classified two different ways.** The producer tested the value for None while `report_outcome` tested key presence, so the same report re-ingested as `pass` but normalised as `fail`. Both now test key presence. - **Malformed stage entries still crashed the dashboard.** The container was guarded; each entry needed it too. Re-verified that the flows which must keep working are unaffected: the manual release-drive ingest, legacy re-ingest idempotence, and the M2 rehearsal-only report all still produce `pass` / `release_ready: true`. Mutation-verified: reverting each of the five guards fails between 1 and 3 tests. Suite 452 passed. Live end-to-end unchanged: YELLOW 70, `red_reasons: []`. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV --- heart/dashboard.py | 4 ++- heart/validate.py | 63 +++++++++++++++++++++++++++-------- tests/test_dashboard.py | 10 ++++++ tests/test_validate.py | 74 +++++++++++++++++++++++++++++++++++++++++ 4 files changed, 136 insertions(+), 15 deletions(-) diff --git a/heart/dashboard.py b/heart/dashboard.py index 5dfa98a..bebafef 100644 --- a/heart/dashboard.py +++ b/heart/dashboard.py @@ -586,7 +586,9 @@ def build_board( st, summary = OK, f"release_ready — {meta}" else: st, summary = WARN, f"release_ready unknown — {meta}" - details = [f"stages: " + ", ".join(f"{n}:{s.get('status', '?')}" for n, s in stages.items())] \ + details = [f"stages: " + ", ".join( + f"{n}:{s.get('status', '?') if isinstance(s, dict) else '?'}" + for n, s in stages.items())] \ if stages else [] sections.append(Section("release_validation", "Release validation", st, summary, details)) diff --git a/heart/validate.py b/heart/validate.py index 255cdd6..5e92689 100644 --- a/heart/validate.py +++ b/heart/validate.py @@ -142,6 +142,13 @@ def _read_json(path: Path) -> Any: return None +def _counts_adverse(counts: Any) -> bool: + """True if a counts block records failures or timeouts.""" + return bool( + isinstance(counts, dict) and (counts.get("failed", 0) or counts.get("timeout", 0)) + ) + + def _parse_iso(value: Any) -> datetime.datetime | None: """Parse an ISO timestamp, or None. Used to order merged base reports.""" try: @@ -337,15 +344,21 @@ def add_report(self, data: dict[str, Any]) -> None: ``--ingest`` at a directory containing both a prior ``validation_report.json`` AND the raw ``integrate.json`` that produced it — which would contradict the "idempotent re-ingest" claim below. - ``stages`` themselves stay merged unconditionally (deduped by name), - as do the scalar fields (version/profile/commit_shas/run_urls). + When a fresh stage artifact HAS contributed counts, the base is + subordinate in full — its stages and its verdict are skipped along with + its counts, and only the scalar fields (version/profile/commit_shas/ + run_urls) seed through. Merging its stages anyway let a stale + ``rehearse: pass`` from a superseded attempt combine with a fresh + integrate-only artifact and read as complete evidence. """ if data.get("testpypi_version") and not self.testpypi_version: self.testpypi_version = str(data["testpypi_version"]) if data.get("profile") and not self.profile: self.profile = str(data["profile"]) self.add_commit_shas(data.get("commit_shas")) - for name, entry in (data.get("stages") or {}).items(): + base_stages = data.get("stages") + base_stages = base_stages if isinstance(base_stages, dict) else {} + for name, entry in ({} if self._stage_counts_seen else base_stages).items(): if isinstance(entry, dict) and name not in self.stages: merged = dict(entry) # Normalise exactly as add_stage does. Without this a merged @@ -379,23 +392,32 @@ def add_report(self, data: dict[str, Any]) -> None: # 3. On an equal or unparseable `ts` we cannot tell which came first, so # a base may only ESCALATE to adverse, never soften. File order on # disk must not be able to clear a recorded failure. - outcome = data.get("validation_outcome") if "validation_outcome" in data else None + has_outcome_key = "validation_outcome" in data + outcome = data.get("validation_outcome") if has_outcome_key else None valid_outcome = outcome if outcome in ("pass", "fail", "incomplete") else None - if outcome is not None and valid_outcome is None: - # Present but not a value we recognise: the report is malformed, and - # a malformed gate artifact must never read as a pass. Sticky. + if has_outcome_key and valid_outcome is None: + # Present but not a value we recognise — `null` included, which is + # why this tests key PRESENCE rather than the value being non-None. + # A malformed gate artifact must never read as a pass. Sticky. self._outcome_invalid = True ready = data["release_ready"] if isinstance(data.get("release_ready"), bool) else None # `false` alone is adverse only when we cannot see why; paired with an # explicit `incomplete` it just means "evidence missing", which is benign # and must not be sticky. - adverse = valid_outcome == "fail" or (ready is False and outcome is None) + adverse = valid_outcome == "fail" or (ready is False and not has_outcome_key) if self._stage_counts_seen: return ts = _parse_iso(data.get("ts")) strictly_newer = ts is not None and (self._base_ts is None or ts > self._base_ts) + strictly_older = ( + ts is not None and self._base_ts is not None and ts < self._base_ts + ) + if self._base_seen and strictly_older: + # Superseded outright. Escalation is for the case where we CANNOT + # order the two, not for one we can order and know to be older. + return if self._base_seen and not strictly_newer: if adverse: self._base_adverse = True @@ -412,12 +434,7 @@ def add_report(self, data: dict[str, Any]) -> None: self._explicit_ready = ready self._base_adverse = adverse - @staticmethod - def _counts_adverse(counts: Any) -> bool: - return bool( - isinstance(counts, dict) - and (counts.get("failed", 0) or counts.get("timeout", 0)) - ) + _counts_adverse = staticmethod(_counts_adverse) def _has_adverse_evidence(self) -> bool: """True if anything ingested is actually *bad* (not merely missing). @@ -521,6 +538,24 @@ def report_outcome(report: Any) -> str | None: return None outcome = report.get("validation_outcome") ready = report.get("release_ready") + # A declaration never outranks the evidence beside it. Legacy reports carry + # only `release_ready`, so a stale `true` sitting next to a failed stage + # used to reach GREEN on the strength of the boolean alone. + stages = report.get("stages") + per_project = report.get("per_project") + if ( + any( + isinstance(v, dict) and _norm_status(v.get("status")) == "fail" + for v in (stages if isinstance(stages, dict) else {}).values() + ) + or _counts_adverse(report.get("totals")) + or any( + _counts_adverse(c) + for c in (per_project if isinstance(per_project, dict) else {}).values() + ) + or bool(report.get("failures")) + ): + return "fail" if outcome in ("pass", "fail", "incomplete"): # The two fields contradict each other; believe the pessimistic one. if outcome == "pass" and ready is False: diff --git a/tests/test_dashboard.py b/tests/test_dashboard.py index 187722b..51e2cb6 100644 --- a/tests/test_dashboard.py +++ b/tests/test_dashboard.py @@ -387,3 +387,13 @@ def test_malformed_stages_does_not_break_the_board(): board = dashboard.build_board(make_snapshot(validation_report=vr), make_verdict("red", 45), now=FRESH_NOW) assert _section(board, "release_validation").state == dashboard.FAIL + + +def test_malformed_stage_entry_does_not_break_the_board(): + """The container was guarded; each ENTRY needs it too.""" + vr = {"release_ready": False, "validation_outcome": "fail", + "testpypi_version": "1", "profile": "release", + "stages": {"integrate": []}, "ts": TS} + board = dashboard.build_board(make_snapshot(validation_report=vr), + make_verdict("red", 45), now=FRESH_NOW) + assert _section(board, "release_validation").state == dashboard.FAIL diff --git a/tests/test_validate.py b/tests/test_validate.py index 1b76fce..df6aac7 100644 --- a/tests/test_validate.py +++ b/tests/test_validate.py @@ -1079,3 +1079,77 @@ def test_equal_timestamps_do_not_let_a_benign_gap_block(tmp_path, gap_name, pass _write(tmp_path / pass_name, _base(_OLD_TS, release_ready=True, validation_outcome="pass")) assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +# --- a declaration never outranks the evidence beside it ------------------- + + +@pytest.mark.parametrize("report", [ + # legacy: only the boolean, contradicted by a failed stage + {"release_ready": True, "stages": {"integrate": {"status": "fail"}}}, + # legacy: contradicted by failing counts + {"release_ready": True, "stages": {"integrate": {"status": "pass"}}, + "totals": {"passed": 1, "failed": 1, "skipped": 0, "timeout": 0}}, + # legacy: contradicted by per-project counts + {"release_ready": True, "stages": {"integrate": {"status": "pass"}}, + "per_project": {"ws": {"passed": 1, "failed": 1, "skipped": 0, "timeout": 0}}}, + # legacy: contradicted by a failures list + {"release_ready": True, "stages": {"integrate": {"status": "pass"}}, + "failures": [{"project": "ws", "script": "x.py"}]}, + # native: an explicit pass contradicted by a failed stage + {"release_ready": True, "validation_outcome": "pass", + "stages": {"integrate": {"status": "fail"}}}, + # a status synonym still counts + {"release_ready": True, "stages": {"integrate": {"status": "failure"}}}, +]) +def test_report_outcome_never_lets_a_declaration_beat_the_evidence(report): + """A stale `release_ready: true` beside a failed stage reached GREEN. + + The normaliser is what every consumer trusts, so it has to reconcile the + report's contents, not just its two verdict fields. + """ + assert validate.report_outcome(report) == "fail" + + +def test_a_superseded_base_cannot_lend_its_rehearsal_to_fresh_artifacts(tmp_path): + """The base is subordinate in FULL when fresh artifacts are present. + + Merging its stages anyway let a stale `rehearse: pass` from a superseded + attempt combine with a fresh integrate-only artifact and read as complete + evidence — laundering a force-failed run into a pass. + """ + _write(tmp_path / "validation_report.json", _base( + _OLD_TS, release_ready=False, validation_outcome="fail")) + _write(tmp_path / "stage_report.json", dict(INTEGRATE)) + report = validate.ingest([tmp_path]) + assert report["validation_outcome"] != "pass" + assert "rehearse" not in report["stages"] + + +@pytest.mark.parametrize("first,second", [ + ("a_old.json", "z_new.json"), + ("a_new.json", "z_old.json"), +]) +def test_base_ordering_is_symmetric_for_adverse_reports(tmp_path, first, second): + """A strictly older adverse base is superseded, whichever order it is read. + + Allowing every "not strictly newer" base to escalate meant an older failure + still pinned RED when it happened to be folded second. + """ + older = _base(_OLD_TS, release_ready=False, validation_outcome="fail") + newer = _base(_NEW_TS, release_ready=True, validation_outcome="pass") + _write(tmp_path / first, older if "old" in first else newer) + _write(tmp_path / second, older if "old" in second else newer) + assert validate.ingest([tmp_path])["validation_outcome"] == "pass" + + +def test_null_discriminator_is_malformed_not_absent(tmp_path): + """`validation_outcome: null` is present-but-unrecognised. + + Testing the value for None rather than the KEY made the producer disagree + with `report_outcome`, which already called it malformed. + """ + nul = _base(_OLD_TS, release_ready=True, validation_outcome=None) + assert validate.report_outcome(nul) == "fail" + _write(tmp_path / "validation_report.json", nul) + assert validate.ingest([tmp_path])["validation_outcome"] == "fail"