Skip to content

gate: confirm blocking regressions at fresh seeds before they block - #49

Merged
oshaughnessy-junior merged 4 commits into
rift_O4dfrom
rift_O4d_gate_confirm
Aug 7, 2026
Merged

gate: confirm blocking regressions at fresh seeds before they block#49
oshaughnessy-junior merged 4 commits into
rift_O4dfrom
rift_O4d_gate_confirm

Conversation

@oshaughnessy-junior

Copy link
Copy Markdown
Owner

Gate infrastructure, split out from #47 because it changes how everyone's merges are judged and should be reviewed on its own.

Problem

Every gate threshold (n_eff >= 100, JS, pull, width) is a hard cut on a stochastic quantity, so a cell sitting near a threshold flips on realization alone and can block a merge that changed nothing about it.

What this adds

confirm_regressions.py re-runs only the disputed cells, in both arms, at several fresh run seeds, and blocks only if the candidate is worse in a majority. Ties count as not-worse — the burden of proof is on the claim that the candidate broke something. run_shape_recovery.sh now documents that a FAIL from it is a hypothesis, not a verdict.

Validated on the case that motivated it

GMM mix_d6_n3_s303 was reported as a blocking REGRESSION(pass->starved) in two consecutive full gate runs (base 119, candidate 66). That looks reproducible rather than flaky, and I initially misread it as such. At 5 fresh seeds the two arms are bit-identical:

seed 988654: base=STARVED cand=STARVED (n_eff 93 vs 93)
seed 989654: base=STARVED cand=STARVED (n_eff 80 vs 80)
seed 990654: base=PASS    cand=PASS    (n_eff 119 vs 119)
seed 991654: base=STARVED cand=STARVED (n_eff 95 vs 95)
seed 992654: base=STARVED cand=STARVED (n_eff 96 vs 96)

So the branch never reached that row; the 119-vs-66 split was an artifact of where the job landed in the worker pool. Re-running the identical run would have reproduced the same false block — only fresh seeds separate "your change broke this" from "this cell responds to how the run was laid out".

A second finding, deliberately not acted on

That cell starves in 4 of 5 seeds. Its PASS at the default seed is the lucky draw, so as a strict blocking row it has been a coin flip on every branch. It should be re-budgeted or dropped from strict — but the strict list and budgets are shared with other people's work, so that is a decision for review rather than a change to slip into this PR.

Explicitly not done: seeding the samplers

Making the GMM fit deterministic would remove this noise and would be a mistake. Independent copies that localize differently are the working detector for support and mode-collapse failures (review-lore standing rule 4), and pinning every fit to one seed silences it while making N production copies no better than one. Reproducibility bought that way is a disguised loss of the detector.

Every gate verdict is a hard threshold on a stochastic quantity, so a cell near
a threshold flips on realization alone and can block a merge that changed
nothing.  Adds confirm_regressions.py: re-runs only the disputed cells, in BOTH
arms, at several fresh run seeds, and blocks only if the candidate is worse in a
majority (ties count as not-worse -- the burden of proof is on the claim that
the candidate broke something).

Validated on the live case that motivated it.  `GMM mix_d6_n3_s303` was reported
as a blocking REGRESSION in two consecutive full runs, base 119 vs candidate 66,
which looked reproducible rather than flaky.  At 5 fresh seeds the arms were
BIT-IDENTICAL -- 93/93, 80/80, 119/119, 95/95, 96/96 -- so the branch does not
reach that row at all, and 4 of the 5 seeds starve: the cell sits on the
n_eff=100 floor and its PASS at the default seed is the lucky draw.  The
apparent regression was an artifact of where the job landed in the worker pool.

Deliberately NOT fixed by seeding the samplers.  Independent copies that
localize differently are our working detector for support and mode-collapse
failures; pinning every fit to one seed would silence it and make N production
copies no better than one.  The right answer is to ask the question again at
fresh seeds, which is what this does.

Separately worth review: that cell starving 4 of 5 seeds means it is
mis-budgeted for the strict set.  Not changed here -- the strict list and budgets
are shared with other people's work.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
oshaughnessy-junior and others added 2 commits August 6, 2026 18:48
…led reruns, and is wired in

Three P1s from review of #49.

[P1] The confirm step recognised only PASS -> non-PASS.  The comparator also
blocks REGRESSION(metrics) -- JS, pull, width, correlation, evidence bias or
n_eff worsening beyond tolerance -- and those rows produced "no blocking
regressions to confirm" and exit 0.  Our own gate v11 had exactly such a row
(GMM d8_n1_s303, n_eff 448->210), which would have been waved through.

Fixed structurally rather than by adding a second branch: compare_shape_results
now exposes classify() / is_blocking() / blocking_keys() as the SINGLE definition
of a regression, and confirm_regressions imports them.  Two copies of that logic
will always drift apart; there is now one.  Verified the refactor reproduces the
v11 verdict exactly (2 blocking).  The confirm step now sees both rows where it
previously saw one.

[P1] Failed reruns were silently skipped, so worse == same == 0 read as "not
confirmed".  Now: a candidate that produces no record where the base did counts
AGAINST the candidate (crashing is worse than passing, not missing evidence); a
verdict requires --min-valid usable pairs, defaulting to all seeds; and too few
valid pairs is INCONCLUSIVE with a nonzero exit, never a silent clear.

[P1] Confirmation was documented but never invoked.  compare_shape_results gains
--confirm-base-checkout / --confirm-cand-checkout / --confirm-repeats and returns
the confirmed verdict as its exit code, so the comparison workflow enforces it.
Without those flags it still exits 1 on a blocking row, and now says explicitly
that the row was NOT confirmed rather than implying it was.

Adds test_confirm_regressions.py (5 checks, all on the dangerous direction -- the
ways a confirmation can wrongly CLEAR a real regression).

CI on this PR is red at "Set up job" on 4 jobs; the same runner-provisioning
failure hits #47, which shares no files with this change, and no failing job
reaches a step that executes repository code.  Infrastructural, not from here.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The previous commit landed without this hunk (a heredoc escaping slip), so the
runner still advertised the standalone command as the whole procedure.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@oshaughnessy-junior

Copy link
Copy Markdown
Owner Author

All three fixed. Thanks — the first one was live in our own data, not hypothetical.

[P1] Metrics-only regressions were invisible to the confirm step

Correct, and gate run v11 contained exactly such a row: GMM mix_d8_n1_s303 REGRESSION(metrics) [n_eff 448->210]. The confirm step would have reported "no blocking regressions to confirm" and exited 0 on it.

Fixed structurally rather than by adding a second branch. compare_shape_results.py now exposes classify() / is_blocking() / blocking_keys() as the single definition of a regression, and confirm_regressions.py imports them. Two copies of that logic will always drift; there is now one. The refactor reproduces the v11 verdict exactly (2 blocking), and the confirm step now sees both rows where it previously saw one:

blocking rows the confirm step now sees: [('GMM','mix_d6_n3_s303'), ('GMM','mix_d8_n1_s303')]

Per-seed comparison inside the confirmation uses the same classify(), so a metrics-only regression is judged there exactly as it is in the gate.

[P1] Failed reruns were skipped

Also correct, and the failure direction was the dangerous one. Now:

  • a candidate that produces no record where the base did counts against the candidate — crashing or emitting nothing is worse than passing, not absent evidence;
  • a verdict requires --min-valid usable pairs, defaulting to all seeds;
  • too few valid pairs is INCONCLUSIVE with exit 1, never a silent clear.

[P1] Confirmation was documented but not wired

Right — it was advisory. compare_shape_results.py now takes --confirm-base-checkout / --confirm-cand-checkout / --confirm-repeats and returns the confirmed verdict as its exit code, so the comparison workflow enforces it. Without those flags it still exits 1 on a blocking row and now says explicitly that the row was not confirmed, instead of implying it was. Runner docs updated to the wired form.

Verified:

unconfirmed blocking -> exit 1
clean gate           -> exit 0

Tests

test_confirm_regressions.py, 5 checks, all aimed at the direction that matters — the ways a confirmation can wrongly clear a real regression (a false block only costs a rerun):

PASS test_metrics_only_regression_is_recognised
PASS test_candidate_crash_counts_against_the_candidate
PASS test_insufficient_valid_pairs_is_inconclusive_not_a_pass
PASS test_genuine_noise_clears
PASS test_real_regression_is_confirmed

CI

Agreed it is infrastructural, and here is the evidence rather than just the impression: all 4 failing jobs fail at Set up job (runner provisioning), the identical pattern hits #47 which shares no files with this change, and no failing job reaches a step that executes repository code.

classify() returned ONLY-IN-BASE when the candidate omitted a row, which
is_blocking() did not treat as a regression -- so the row never reached
confirmation and the gate exited 0.  A candidate crashing before emitting its
first result could bypass the fail-closed rerun logic entirely.

Now REGRESSION(missing-in-candidate).  Confirmation re-tests such a row using
whichever record exists for the cell spec, so the candidate is actually re-run
rather than written off.  The reverse (a NEW row in the candidate) still does not
block.  Two tests added.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@oshaughnessy-junior

Copy link
Copy Markdown
Owner Author

Last P1 fixed in 089ed551.

classify() returned ONLY-IN-BASE when the candidate omitted a row, is_blocking() did not treat it as a regression, so it never reached confirmation and the gate exited 0 -- a candidate crashing before emitting its first result bypassed the fail-closed rerun logic entirely. It is now REGRESSION(missing-in-candidate).

One thing worth flagging in the fix: confirmation must still be able to re-test such a row. The candidate has no record, but the base record defines the cell (kind, dim, ncomp, seed), so the confirm step now takes the spec from whichever record exists and actually re-runs the candidate, rather than writing it off. Otherwise the row would block on missing evidence instead of on measurement.

Two tests added: the missing-record case blocks and is picked up as a row to confirm, and the reverse direction (a new row in the candidate) still does not block. Seven tests pass.

New evidence that this policy should extend to the flag-ON probe

While validating #47 I hit the same failure mode in probe_portfolio_optin_flags.py, which has no confirmation step. d4_n1_s303 reported base=PASS flag=STARVED and 1 opt-in regression. Re-running the probe against an earlier commit reproduced it identically, while an earlier full gate run at that same commit had reported PROBE_EXIT=0:

d4_n1_s303, weight_clip ON run A run B
n_eff 422 (PASS) 46 (STARVED)

Same code, opposite verdicts. The probe reuses the gate's thresholds and evaluate(), so it inherits the same near-threshold sensitivity, and it can currently fail a PR on a coin flip. Suggest extending confirm-on-fail to the probe as a follow-up to this PR.

@oshaughnessy-junior
oshaughnessy-junior merged commit ceb29ac into rift_O4d Aug 7, 2026
19 checks passed
@oshaughnessy-junior
oshaughnessy-junior deleted the rift_O4d_gate_confirm branch August 13, 2026 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant