Skip to content

fix: grade an absent rehearsal as STALE, not RED (#144) - #145

Merged
Jammy2211 merged 6 commits into
mainfrom
feature/release-validation-tri-state
Aug 14, 2026
Merged

fix: grade an absent rehearsal as STALE, not RED (#144)#145
Jammy2211 merged 6 commits into
mainfrom
feature/release-validation-tri-state

Conversation

@Jammy2211

@Jammy2211 Jammy2211 commented Aug 14, 2026

Copy link
Copy Markdown
Contributor

Closes #144.

The symptom

Heart reported RED score 45, top blocker release validation FAILED, against run 31769743408 — a Release Integrate run whose conclusion was success with zero non-success jobs. The ingested report read release_ready: false with stages: {integrate: pass}, 661 passed / 0 failed, failures: []. Nothing had failed.

Root cause

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 absent an explicit merged value. release_ready was therefore false by construction on this path however green the run was, and readiness.py mapped every false onto the RED axis.

This contradicted the documented two-axis design in both files: absence is the STALE axis, failure is the RED axis.

The fix

One boolean cannot answer both "did anything fail?" and "is the evidence complete?". This adds validation_outcome: pass | fail | incomplete and reads that for severity, leaving release_ready 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". Also apply _norm_status in add_report, which previously copied stage statuses verbatim so a merged report's "failure" never normalised.
  • 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 rather than 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 persists 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.

Verification

End-to-end against the real state and the real artifact, with no CI re-dispatched:

Before After
verdict RED score 45 YELLOW score 70
red_reasons ["release validation FAILED"] []
stale_reasons [] ["release validation incomplete: no rehearsal for current source"]

The remaining yellows are the pre-existing workspace-validation and manifest-drift ones. Full Heart suite: 380 passed.

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. test_validation_failed_is_red is unmodified and serves as the control.

One pre-existing test, test_fresher_local_ingest_is_never_regressed, had its fixture given a validation_outcome — it now pins the steady-state invariant, with the one-time migration exception covered by a sibling test.

Review

The diagnosis and plan were cross-reviewed by Codex, which rejected an earlier readiness-only fix. Its load-bearing findings were each re-verified against the code before adoption — in particular that _norm_status runs only in add_stage and maps unknown → skip, and that release_ready() never reads totals/failures, so "no stage says fail" is not a sound proxy for "nothing failed". Combined with AUTONOMY.md leg 4 passing on STALE, the naive fix would have let autonomous ships clear a gate that should block them.

🤖 Generated with Claude Code

https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV

Jammy2211 and others added 6 commits August 14, 2026 12:37
Jammy2211 @claude Jammy2211
fix: grade an absent rehearsal as STALE, not RED (#144)
17598d1 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
Jammy2211 @claude Jammy2211
fix: close the fail-open holes a Codex review found (#144)
a0ebf26 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
Jammy2211 @claude Jammy2211
fix: one shared outcome normaliser + precedence and migration fixes (#…
f97834c 
…144)

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
Jammy2211 @claude Jammy2211
fix: derive release_ready, make adverse verdicts sticky (#144)
d37ef38 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
Jammy2211 @claude Jammy2211
fix: order merged bases by recency; restore legacy re-ingest (#144)
c7ba803 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
Jammy2211 @claude Jammy2211
fix: evidence outranks declarations; subordinate bases fully (#144)
46dd28e 
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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01ENfMeYpRfAifDQ63uheMbV
@Jammy2211
Jammy2211 merged commit a0113dc into main Aug 14, 2026
2 checks passed
@Jammy2211
Jammy2211 deleted the feature/release-validation-tri-state branch August 14, 2026 18:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

fix: grade an absent rehearsal as STALE, not RED, via validation_outcome

1 participant

@Jammy2211