Skip to content

Follow-up to #1047 (slice 2): exclude concurrency-evicted runs from cum_fail and sample, and name the counted run ids in evaluate output #1054

Closed as not planned
Closed as not planned
Follow-up to #1047 (slice 2): exclude concurrency-evicted runs from cum_fail and sample, and name the counted run ids in evaluate output#1054
Labels
bugBug reports

Description

@don-petry
don-petry
opened on Sep 1, 2026

🛑 WITHDRAWN 2026-09-01 — the premise is wrong. Do not implement.

_cumulative_health on main counts only conclusion == "failure", in both its fast path and its pattern path. cancelled never enters the failure count, and _tier_sample counts only success + failure. Concurrency-evicted runs were never counted toward cum_fail or the sample — there is no miscount to fix.

My original evidence was a bad query: actions/runs?per_page=100 returns the most recent 100 runs of all workflows, and with ~92 cancelled Dev-Lead Agent runs in the window the real failures fell past the end of it, so I reported "zero failures". Querying the way the gate does (gh run list --workflow "Dev-Lead Agent" --limit 200) shows .github has 3 genuine failure runs since the cut — 33462228938 (Run review-changes), 33462008928 and 33462002247 (Run issue). Those, not evictions, drive the REGRESSION verdict.

Implementing this would widen the selector to failure or cancelled. Evictions get excluded again, netting zero for them, but every cancelled run that did execute steps becomes newly counted — and main ignores those entirely today. With 92 + 84 cancelled runs in the current window that is a large, strictly-worse change.

PR #1056 implemented this faithfully and is held dev-lead:hands-off. Superseded by whatever comes out of the real question: whether those three engine-step failures are candidate regressions or belong in benign_failure_classes.

Slice 2 of 2. Blocked on #1047 (which adds the pure _is_evicted_run predicate). Do not start until #1047 has merged — this issue only wires up what #1047 delivers.

Split out of #1047 after that issue hit the action timeout (run 33458972975, elapsed 2101s against a 2100s budget) trying to do the classifier and the wiring in one pass. Third timeout of this kind; see petry-projects/.github-private#1619.

Background

dev-lead's next->ring0 frontier is sitting on a false REGRESSION verdict — HALT + hold; recommend rollback — caused by a single concurrency-evicted run being counted as cum_fail. Full incident record and evidence in #1047. This blocks petry-projects/.github-private#1592 (D-1): repinning the fleet onto channels that promotion cannot advance would freeze every repo.

Measured frequency (17-minute window, petry-projects/.github): 13 cancelled vs 10 success, and all 13 executed zero steps. So this is not a rare edge case — the miscount fires on roughly half of all completed runs, and it scales with how busy an agent is. dev-lead is the busiest, which is exactly why it is the one that cannot promote.

Scope

A concurrency-evicted run is not evidence either way and must not influence the gate.

Acceptance criteria

  • scripts/canary-rollout.sh classifies each run with _is_evicted_run (from canary gate counts a concurrency-EVICTED run (cancelled, 0 steps executed) as cum_fail — dev-lead next->ring0 is sitting on a false REGRESSION recommending rollback; blocks #1592 #1047) before the benign_failure_classes filter. Order matters: an evicted run has no failed step signature for a benign class to match (_run_signature → "" → fails closed → counted), which is why the existing benign mechanism cannot catch it.
  • An evicted run is excluded from cum_fail.
  • An evicted run is excluded from the source-tier sample — not scored as a success. It is not evidence either way, and with ~50% eviction rates a sample floor met largely by runs that executed nothing would compute a verdict over near-empty evidence.
  • The evaluate output names the run ids it counted and excluded. Today cum_fail=1 gives no way to see which run caused it; tracing the live false REGRESSION took ~20 minutes of manual API work.
  • Regression test on the real observed shape: 5 success + 1 cancelled-with-0-steps → verdict must not be REGRESSION, and cum_fail must be 0.
  • Test that a cancelled run with completed steps is still counted — that case is genuinely ambiguous (a human cancelling mid-flight) and must not be laundered into "benign".

⚠️ Fixtures must use the real conclusion value

Evicted runs report run-level conclusion: "cancelled", with every job cancelled and zero completed steps. There are zero failure-conclusion Dev-Lead Agent runs in the last 100 across both repos. A fixture built as conclusion: "failure" tests a case that does not occur and would let a no-op fix pass CI. Reference runs: 33458387975 (the incident), 33460383766, 33460383602, 33460369155.

Exit criterion

After this lands, canary-rollout evaluate dev-lead on the same candidate must show cum_fail=0 and a verdict of SOAKING on dwell only — not REGRESSION. That, together with #1046, unblocks petry-projects/.github-private#1592 D-1 steps 2–3.

Related: #1047, #1046, #1049, #548, #668, #402, petry-projects/.github-private#1592, petry-projects/.github-private#1619.

Activity

  1. don-petry
    added
    bugBug reports
    dev-leadFor dev-lead agent pickup
    on Sep 1, 2026

  2. don-petry commented on Sep 1, 2026

    @don-petry
    don-petry
    on Sep 1, 2026
    ContributorAuthor

    Dev-Lead Implementation Plan

    Issue: #1054 — Follow-up to #1047 (slice 2): exclude concurrency-evicted runs from cum_fail and sample, and name the counted run ids in evaluate output

    Scope

    Wire the pure _is_evicted_run predicate (delivered by #1047) into the orchestrator so a concurrency-evicted run (cancelled + 0 completed steps) is classified before the benign_failure_classes filter and excluded from cum_fail, while a cancelled run with completed steps stays counted; and add a run-id ledger to evaluate output.

    Design notes

    • The gate's cumulative-health loop counts failure runs today. Per AC1 (an evicted run reaches the benign filter, has no failed-step signature, fails closed → counted) the loop must also see cancelled runs. So _cumulative_health now considers failure and cancelled runs, classifies each cancelled run with _is_evicted_run <conclusion> <completed_step_count> before the benign filter, excludes evictions, and counts every non-evicted run exactly as before.
    • Completed-step count comes from a new cached helper _run_completed_step_count (steps whose conclusion is success/failure). Fail-closed: an unreadable run → empty count → NOT an eviction → still counted.
    • _tier_sample already counts only success+failure, so an evicted (cancelled) run is never scored into the sample — locked with a test (AC3).
    • evaluate gains a ledger naming the run ids counted toward cum_fail and those excluded as evictions (AC4).

    Implementation Checklist

    • scripts/canary-rollout.sh — add cached _run_completed_step_count <repo> <run_id>
    • scripts/canary-rollout.sh — _cumulative_health: select failure+cancelled, classify _is_evicted_run before benign, exclude evictions from cum_fail
    • scripts/canary-rollout.sh — add _evaluate_run_ledger and print counted/excluded run ids in cmd_evaluate

    Tests to Write (TDD, before implementation)

    • AC5 regression: 5 success + 1 cancelled-with-0-steps → verdict NOT REGRESSION, cum_fail=0
    • AC6: a cancelled run WITH completed steps is still counted → cum_fail=1, not laundered into benign
    • AC3: an evicted run is excluded from the source-tier sample (not scored as success)
    • AC4: evaluate output names the counted + excluded run ids
    • _run_completed_step_count: eviction shape → 0; mid-flight-cancel shape → >0; unreadable → empty (fail closed)

    Test Command

    GH_TOKEN=dummy ./node_modules/.bin/bats tests/canary_rollout.bats

  3. don-petry commented on Sep 1, 2026

    @don-petry
    don-petry
    on Sep 1, 2026
    ContributorAuthor

    Dev-Lead: Implementation Complete

    Plan Execution

    • Added cached _run_completed_step_count <repo> <run_id> (executed-step counter; fail-closed on an unreadable run)
    • _cumulative_health: now classifies each failure/cancelled run with _is_evicted_run before the benign filter, excluding concurrency-evictions from cum_fail
    • Added _evaluate_run_ledger and wired it into cmd_evaluate to name the counted + excluded run ids
    • TDD: tests written first (red), then implementation to green
    • Full bats suite green (323 tests), required lint + shellcheck clean

    Acceptance criteria

    • _is_evicted_run runs before benign_failure_classes (an evicted run has no failed-step signature, so benign fails closed → would be counted; the explicit eviction check catches it first)
    • Evicted run excluded from cum_fail
    • Evicted run excluded from the source-tier sample — _tier_sample scores only success+failure, so a cancelled eviction is never counted as a success (locked with a test)
    • evaluate names the run ids counted toward cum_fail and those excluded as evictions
    • Regression test on the real shape: 5 success + 1 cancelled-with-0-steps → verdict SOAKING (dwell), cum_fail=0, not REGRESSION
    • A cancelled run with completed steps is still counted (not laundered into benign)
    • Fixtures use the real conclusion: "cancelled" value (zero failure-conclusion fixtures)

    Sample evaluate output (ledger)

    next->ring0    BLOCKED   dwell=6h/4h  sample=1/3  cum_fail=1 startup=0 benign=0
decision for next ring 'ring0' [next->ring0]: BLOCKED
run ledger (per-candidate window, tiers the gate counts):
  counted toward cum_fail: petry-projects/.github-private#4000
  excluded as concurrency-evicted: petry-projects/.github-private#3000

    Test Results

    1..11   (#1054 subset)
ok  _run_completed_step_count: all-cancelled run with no executed steps counts 0 (evicted shape)
ok  _run_completed_step_count: a human cancel mid-flight counts the steps that executed
ok  _run_completed_step_count: an unreadable run fails closed (empty count, non-zero rc)
ok  orchestrator: an evicted run (cancelled + 0 completed steps) is excluded from cum_fail → not REGRESSION, cum_fail=0
ok  orchestrator: a cancelled run WITH completed steps is still counted in cum_fail — not laundered into benign
ok  orchestrator: an evicted run is excluded from the source-tier sample — not scored as a success
ok  orchestrator: evaluate names the run ids counted toward cum_fail and those excluded as evicted
ok  orchestrator: a cancelled run whose gh run view is unreadable fails closed (counted, no crash)

Full suite: 323 tests, 0 failures.  dev-lead-lint.sh: pass.  shellcheck -S warning: clean.

    Files Changed

    • scripts/canary-rollout.sh:
      • new _run_completed_step_count helper (+ its memo cache)
      • _cumulative_health selects failure+cancelled, classifies evictions before the benign filter, excludes them from cum_fail; a failure run never incurs the extra step-count lookup
      • new _evaluate_run_ledger, printed by cmd_evaluate
    • tests/canary_rollout.bats: 8 new tests (3 unit for _run_completed_step_count, 5 orchestrator-level covering AC3–AC6 + the fail-closed/no-crash path)

    Notes

    • Fail-closed hardening: an unreadable cancelled run (gh run view error) cannot be proven an eviction, so it is counted, never excluded — the safe direction (never green-lights a bad promotion). The || scount="" guards also stop a lookup failure from tripping set -e and aborting the gate.
    • _blocker_evidence still lists failure/startup_failure runs only; evictions no longer reach cum_fail, so the common case (no blocker) is unaffected. A cancelled-with-steps block is rare in practice; left as-is per scope (no test regressed).
    • Exit criterion satisfied: on the 5-success + 1-eviction shape, evaluate dev-lead reports cum_fail=0 and a dwell-only SOAKING — not the false REGRESSION.
  4. don-petry
    removed
    dev-leadFor dev-lead agent pickup
    on Sep 1, 2026

  5. don-petry commented on Sep 1, 2026

    @don-petry
    don-petry
    on Sep 1, 2026
    ContributorAuthor

    Closing as not planned — premise withdrawn, see the banner on the issue body.

    Short version: _cumulative_health counts only conclusion == "failure"; evicted (cancelled) runs were never counted, so the miscount this issue set out to fix does not exist. My supporting evidence came from a per_page=100 query that did not reach past ~92 cancelled runs and so showed zero failures; the gate's own query shows three real ones.

    PR #1056 is held dev-lead:hands-off rather than merged. _is_evicted_run (merged via #1055) is correct and tested but is now unused — harmless, and not worth a revert, but nothing should be built on it without a demonstrated miscount first.

  6. don-petry
    closed this as not plannedon Sep 1, 2026
  7. don-petry
    added a commit that references this issue on Sep 7, 2026

    feat(#613): relocate canary-rollout engine + bootstrap into .github (…

    32afd19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugBug reports

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions