From 9a68748ec370ae45273dd886282c3884ec036159 Mon Sep 17 00:00:00 2001 From: Richard O'Shaughnessy Date: Thu, 6 Aug 2026 08:32:58 -0700 Subject: [PATCH 1/4] gate: require confirm-on-fail before a blocking regression counts 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 --- .../integrators/confirm_regressions.py | 147 ++++++++++++++++++ .../integrators/run_shape_recovery.sh | 19 +++ 2 files changed, 166 insertions(+) create mode 100644 MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py new file mode 100644 index 000000000..1fcd32d50 --- /dev/null +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py @@ -0,0 +1,147 @@ +#!/usr/bin/env python +"""Re-test the blocking regressions a merge-gate comparison reported, at NEW random seeds. + +WHY THIS EXISTS. Every gate verdict is a hard threshold (n_eff >= 100, JS < 3*floor + 0.004, ...) +applied to a stochastic quantity, so a cell sitting near a threshold flips on realization alone. +Observed: `GMM mix_d6_n3_s303` read n_eff 66 / 119 / 104 across runs of the SAME unchanged +checkout -- straddling the 100 floor -- purely from where it landed in the worker pool. Reported +as a REGRESSION once, it would have blocked a merge that changed nothing about that sampler. + +The fix is NOT to make the samplers deterministic. Independent copies that localize differently +are our main detector for support/mode-collapse failures; pinning every fit to one seed would +silence it, and would make N copies of a production run no better than one. The fix is to ask the +question again, properly: re-run the disputed cell in BOTH arms at several fresh run seeds and see +whether the candidate is really worse. + +Usage: + confirm_regressions.py base.json cand.json --base-checkout DIR --cand-checkout DIR \\ + [--repeats 3] [--jobs 4] [--seeds 11,22,33] + +Exit 0 if no regression is CONFIRMED; 1 if any is. A regression is confirmed when the candidate +is worse than the base in a MAJORITY of the fresh seeds (ties count as not-worse: the burden of +proof is on the claim that the candidate broke something). +""" +import argparse +import json +import os +import subprocess +import sys +import tempfile + +HERE = os.path.dirname(os.path.abspath(__file__)) +sys.path.insert(0, HERE) +from shape_recovery import evaluate # noqa: E402 + + +def _key(r): + return (r["kind"], r["target"]) + + +def _blocking(base_path, cand_path, strict): + with open(base_path) as fh: + base = {_key(r): r for r in json.load(fh)} + with open(cand_path) as fh: + cand = {_key(r): r for r in json.load(fh)} + out = [] + for k in sorted(set(base) & set(cand)): + if k[0] not in strict: + continue + st_b, _ = evaluate(base[k]) + st_c, _ = evaluate(cand[k]) + if st_b == "PASS" and st_c != "PASS": + out.append((k, base[k], cand[k])) + return out + + +def _rerun(checkout, rec, seed, jobs, tag): + """Re-run ONE cell of the matrix at a given run seed; return its record or None.""" + fd, path = tempfile.mkstemp(suffix=".json", prefix="confirm_%s_" % tag) + os.close(fd) + cmd = [os.environ.get("PYTHON", "python3"), os.path.join(HERE, "shape_recovery.py"), + "--preset", "standard", "--json", path, "--jobs", str(jobs), + "--samplers", rec["kind"] if not rec["kind"].startswith(("portfolio_warm", + "portfolio_seq", "AV_seq")) + else "AV", + "--dims", str(rec["ndim"]), "--ncomps", str(rec["ncomp"]), + "--target-seeds", str(rec["target_seed"]), "--run-seed", str(seed), + "--warm-cases", "on" if rec["kind"] in ("portfolio_warm", "portfolio_seq", + "portfolio_seq_nobs", "AV_seq") else "off"] + env = dict(os.environ) + env["PYTHONPATH"] = os.path.join(checkout, "MonteCarloMarginalizeCode", "Code") + \ + os.pathsep + env.get("PYTHONPATH", "") + env["CUDA_VISIBLE_DEVICES"] = "" + try: + subprocess.run(cmd, env=env, stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL, + check=False) + with open(path) as fh: + for r in json.load(fh): + if _key(r) == _key(rec): + return r + except Exception: + return None + finally: + try: + os.unlink(path) + except OSError: + pass + return None + + +def main(argv=None): + ap = argparse.ArgumentParser(description=__doc__.split("\n")[1]) + ap.add_argument("base") + ap.add_argument("candidate") + ap.add_argument("--base-checkout", required=True) + ap.add_argument("--cand-checkout", required=True) + ap.add_argument("--repeats", type=int, default=3, + help="fresh run seeds per arm (default 3; use more for a near-threshold cell)") + ap.add_argument("--seeds", default=None, help="explicit comma list, overrides --repeats") + ap.add_argument("--jobs", type=int, default=4) + ap.add_argument("--strict-samplers", + default="AV,GMM,portfolio_warm,portfolio_seq,portfolio_seq_nobs") + opts = ap.parse_args(argv) + + strict = set(x.strip() for x in opts.strict_samplers.split(",") if x.strip()) + seeds = ([int(x) for x in opts.seeds.split(",")] if opts.seeds + else [987654 + 1000 * (i + 1) for i in range(opts.repeats)]) + + disputed = _blocking(opts.base, opts.candidate, strict) + if not disputed: + print("# no blocking regressions to confirm") + return 0 + print("# confirming {} blocking regression(s) at {} fresh seed(s): {}".format( + len(disputed), len(seeds), seeds)) + + n_confirmed = 0 + for k, brec, crec in disputed: + worse = same = 0 + detail = [] + for s in seeds: + rb = _rerun(opts.base_checkout, brec, s, opts.jobs, "base") + rc = _rerun(opts.cand_checkout, crec, s, opts.jobs, "cand") + if rb is None or rc is None: + detail.append("seed {}: RERUN FAILED".format(s)) + continue + sb = evaluate(rb)[0] + sc = evaluate(rc)[0] + if sb == "PASS" and sc != "PASS": + worse += 1 + else: + same += 1 + detail.append("seed {}: base={} cand={} (n_eff {:.0f} vs {:.0f})".format( + s, sb, sc, rb.get("n_eff", float("nan")), rc.get("n_eff", float("nan")))) + confirmed = worse > same + n_confirmed += int(confirmed) + print("\n{} {}".format(k[0], k[1])) + for d in detail: + print(" " + d) + print(" -> {} ({} worse / {} not-worse across fresh seeds)".format( + "CONFIRMED REGRESSION -- BLOCKS" if confirmed + else "NOT CONFIRMED (realization noise; does not block)", worse, same)) + + print("\n# confirmed blocking regressions: {}".format(n_confirmed)) + return 1 if n_confirmed else 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh index 3675f4cde..df7970c4c 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh @@ -1,6 +1,25 @@ #!/bin/bash # Standard merge-gate invocation of the shape-recovery suite. # +# A FAIL FROM THIS SCRIPT IS NOT A VERDICT. Every gate threshold (n_eff >= 100, JS, pull, width) +# is a hard cut on a stochastic quantity, so any cell sitting near a threshold flips on +# realization alone. Before treating a blocking regression as real, re-test it at fresh seeds: +# +# confirm_regressions.py base.json cand.json \ +# --base-checkout DIR --cand-checkout DIR --repeats 5 +# +# It re-runs only the disputed cells, in BOTH arms, at several new run seeds, and blocks only if +# the candidate is worse in a majority. Worked example: `GMM mix_d6_n3_s303` was reported as a +# blocking REGRESSION in two consecutive full runs (base 119, candidate 66) and looked +# reproducible -- but at 5 fresh seeds the two arms were BIT-IDENTICAL (93/93, 80/80, 119/119, +# 95/95, 96/96) and 4 of the 5 starved. The cell simply sits on the n_eff=100 floor; its PASS at +# the default seed was the lucky draw, and the apparent regression was an artifact of where the +# job landed in the worker pool. +# +# Do NOT "fix" this by seeding the samplers deterministically. Independent copies that localize +# differently are the working detector for support/mode-collapse failures; pinning every fit to +# one seed silences it, and makes N production copies no better than one. +# # ./run_shape_recovery.sh /path/to/checkout results.json [extra args...] # # Runs CPU-only (deterministic; also exercises the cupy-installed-but-no-GPU From 0ea0729e70b2f258eb45955acabdbb46603f9b82 Mon Sep 17 00:00:00 2001 From: Richard O'Shaughnessy Date: Thu, 6 Aug 2026 18:48:22 -0700 Subject: [PATCH 2/4] gate: confirmation now shares the comparator's classifier, counts failed 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 --- .../integrators/compare_shape_results.py | 124 ++++++++++++------ .../integrators/confirm_regressions.py | 79 +++++++---- .../integrators/test_confirm_regressions.py | 96 ++++++++++++++ 3 files changed, 237 insertions(+), 62 deletions(-) create mode 100644 MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py index f3ac86a9e..81d25da5c 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py @@ -38,6 +38,65 @@ def _summ(r): neff=r["n_eff"]) +def classify(b, c): + """Return (verdict, note) for one base/candidate record pair. + + SINGLE SOURCE OF TRUTH for what counts as a regression. confirm_regressions.py + imports this: it previously reimplemented only the PASS->non-PASS case and was blind to + REGRESSION(metrics), so a real metric regression (measured: n_eff 448->210) produced + "no blocking regressions to confirm" and exited 0. Two copies of this logic will always + drift; there is now one. + """ + if b is None or c is None: + return "ONLY-IN-" + ("CANDIDATE" if b is None else "BASE"), "" + st_b, _ = evaluate(b) + st_c, why_c = evaluate(c) + sb, sc = _summ(b), _summ(c) + verdict, note = "OK", "" + if st_b == "PASS" and st_c != "PASS": + # includes healthy->STARVED: candidate lost the efficiency the + # base had on this target -> regression + verdict = "REGRESSION(pass->{})".format(st_c.lower()) + note = "; ".join(why_c) + elif st_b != "PASS" and st_c == "PASS": + verdict = "IMPROVED({}->pass)".format(st_b.lower()) + elif st_b == "STARVED" and st_c == "STARVED": + verdict = "BOTH-STARVED" + elif st_b == "STARVED" and st_c in ("FAIL", "ERROR"): + # base gave no shape information here; candidate at least reaches + # testability (or crashes) -- flag, don't block + verdict = "NEWLY-TESTABLE-" + st_c + note = "; ".join(why_c) + elif st_b in ("FAIL", "ERROR") and st_c != "PASS": + verdict = "PREEXISTING-FAIL" + elif sb and sc: + worse = [] + for m, tol in TOL_WORSE.items(): + if m == "neff_frac": + if sc["neff"] < TOL_WORSE["neff_frac"] * sb["neff"]: + worse.append("n_eff {:.0f}->{:.0f}".format(sb["neff"], sc["neff"])) + elif sc[m] - sb[m] > tol: + worse.append("{} {:.3f}->{:.3f}".format(m, sb[m], sc[m])) + if worse: + verdict = "REGRESSION(metrics)" + note = "; ".join(worse) + return verdict, note + + +def is_blocking(verdict, kind, strict): + return verdict.startswith("REGRESSION") and kind in strict + + +def blocking_keys(base, cand, strict): + """Every (kind, target) the gate would BLOCK on -- both regression flavours.""" + out = [] + for k in sorted(set(base) | set(cand)): + v, _ = classify(base.get(k), cand.get(k)) + if is_blocking(v, k[0], strict): + out.append(k) + return out + + def main(): ap = argparse.ArgumentParser() ap.add_argument("base") @@ -46,6 +105,15 @@ def main(): # regression there must block. Note the intended asymmetry on first merge -- portfolio_seq # FAILs on a base without clear_warm_state and PASSes here, i.e. IMPROVED (non-blocking). # Its value is forward-looking: once this is the base, re-breaking the reset blocks. + # ENFORCEMENT. Given both checkouts, a blocking regression is re-tested at fresh seeds + # before it is allowed to fail the gate, and THIS script's exit code reflects the confirmed + # verdict. Without these the script only reports, and confirmation is advisory -- which is + # how the first version shipped: documented in the runner but never actually invoked. + ap.add_argument("--confirm-base-checkout", default=None, + help="with --confirm-cand-checkout: re-test blocking rows at fresh seeds") + ap.add_argument("--confirm-cand-checkout", default=None) + ap.add_argument("--confirm-repeats", type=int, default=5) + ap.add_argument("--confirm-jobs", type=int, default=4) ap.add_argument("--strict-samplers", default="AV,GMM,portfolio_warm,portfolio_seq,portfolio_seq_nobs") opts = ap.parse_args() @@ -59,42 +127,8 @@ def main(): n_block = 0 rows = [] for k in sorted(set(base) | set(cand)): - b, c = base.get(k), cand.get(k) - if b is None or c is None: - rows.append((k, "ONLY-IN-" + ("CANDIDATE" if b is None else "BASE"), "")) - continue - st_b, _ = evaluate(b) - st_c, why_c = evaluate(c) - sb, sc = _summ(b), _summ(c) - verdict, note = "OK", "" - if st_b == "PASS" and st_c != "PASS": - # includes healthy->STARVED: candidate lost the efficiency the - # base had on this target -> regression - verdict = "REGRESSION(pass->{})".format(st_c.lower()) - note = "; ".join(why_c) - elif st_b != "PASS" and st_c == "PASS": - verdict = "IMPROVED({}->pass)".format(st_b.lower()) - elif st_b == "STARVED" and st_c == "STARVED": - verdict = "BOTH-STARVED" - elif st_b == "STARVED" and st_c in ("FAIL", "ERROR"): - # base gave no shape information here; candidate at least reaches - # testability (or crashes) -- flag, don't block - verdict = "NEWLY-TESTABLE-" + st_c - note = "; ".join(why_c) - elif st_b in ("FAIL", "ERROR") and st_c != "PASS": - verdict = "PREEXISTING-FAIL" - elif sb and sc: - worse = [] - for m, tol in TOL_WORSE.items(): - if m == "neff_frac": - if sc["neff"] < TOL_WORSE["neff_frac"] * sb["neff"]: - worse.append("n_eff {:.0f}->{:.0f}".format(sb["neff"], sc["neff"])) - elif sc[m] - sb[m] > tol: - worse.append("{} {:.3f}->{:.3f}".format(m, sb[m], sc[m])) - if worse: - verdict = "REGRESSION(metrics)" - note = "; ".join(worse) - blocking = verdict.startswith("REGRESSION") and k[0] in strict + verdict, note = classify(base.get(k), cand.get(k)) + blocking = is_blocking(verdict, k[0], strict) if blocking: n_block += 1 rows.append((k, verdict + (" <-- BLOCKS MERGE" if blocking else ""), note)) @@ -103,7 +137,23 @@ def main(): print("{:<10s} {:<16s} {} {}".format(kind, tgt, verdict, ("[" + note + "]") if note else "")) print("# blocking regressions (strict={}): {}".format(sorted(strict), n_block)) - return 1 if n_block else 0 + if not n_block: + return 0 + if not (opts.confirm_base_checkout and opts.confirm_cand_checkout): + print("# NOT CONFIRMED AT FRESH SEEDS: pass --confirm-base-checkout/--confirm-cand-checkout\n" + "# to re-test these rows before treating them as real. Every threshold here is a\n" + "# hard cut on a stochastic quantity, so a single blocking row is a hypothesis.") + return 1 + import confirm_regressions + print("\n# re-testing {} blocking row(s) at {} fresh seeds per arm".format( + n_block, opts.confirm_repeats)) + return confirm_regressions.main([ + opts.base, opts.candidate, + "--base-checkout", opts.confirm_base_checkout, + "--cand-checkout", opts.confirm_cand_checkout, + "--repeats", str(opts.confirm_repeats), + "--jobs", str(opts.confirm_jobs), + "--strict-samplers", opts.strict_samplers]) if __name__ == "__main__": diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py index 1fcd32d50..a842d8cdf 100644 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py @@ -30,7 +30,11 @@ HERE = os.path.dirname(os.path.abspath(__file__)) sys.path.insert(0, HERE) -from shape_recovery import evaluate # noqa: E402 +# The comparator OWNS the definition of "blocking". Importing it -- rather than reimplementing +# the PASS->non-PASS case, as this script first did -- is what keeps the two in step: the local +# copy was blind to REGRESSION(metrics), so a real metric regression (measured: n_eff 448->210) +# reported "no blocking regressions to confirm" and exited 0. +from compare_shape_results import classify, is_blocking, blocking_keys # noqa: E402 def _key(r): @@ -42,15 +46,7 @@ def _blocking(base_path, cand_path, strict): base = {_key(r): r for r in json.load(fh)} with open(cand_path) as fh: cand = {_key(r): r for r in json.load(fh)} - out = [] - for k in sorted(set(base) & set(cand)): - if k[0] not in strict: - continue - st_b, _ = evaluate(base[k]) - st_c, _ = evaluate(cand[k]) - if st_b == "PASS" and st_c != "PASS": - out.append((k, base[k], cand[k])) - return out + return [(k, base.get(k), cand.get(k)) for k in blocking_keys(base, cand, strict)] def _rerun(checkout, rec, seed, jobs, tag): @@ -97,6 +93,9 @@ def main(argv=None): help="fresh run seeds per arm (default 3; use more for a near-threshold cell)") ap.add_argument("--seeds", default=None, help="explicit comma list, overrides --repeats") ap.add_argument("--jobs", type=int, default=4) + ap.add_argument("--min-valid", type=int, default=None, + help="usable base/candidate pairs required for a verdict (default: all " + "seeds). Fewer -> INCONCLUSIVE and exit 1, never a silent clear.") ap.add_argument("--strict-samplers", default="AV,GMM,portfolio_warm,portfolio_seq,portfolio_seq_nobs") opts = ap.parse_args(argv) @@ -104,6 +103,8 @@ def main(argv=None): strict = set(x.strip() for x in opts.strict_samplers.split(",") if x.strip()) seeds = ([int(x) for x in opts.seeds.split(",")] if opts.seeds else [987654 + 1000 * (i + 1) for i in range(opts.repeats)]) + if opts.min_valid is None: + opts.min_valid = len(seeds) disputed = _blocking(opts.base, opts.candidate, strict) if not disputed: @@ -113,34 +114,62 @@ def main(argv=None): len(disputed), len(seeds), seeds)) n_confirmed = 0 + n_inconclusive = 0 for k, brec, crec in disputed: worse = same = 0 detail = [] for s in seeds: - rb = _rerun(opts.base_checkout, brec, s, opts.jobs, "base") - rc = _rerun(opts.cand_checkout, crec, s, opts.jobs, "cand") - if rb is None or rc is None: - detail.append("seed {}: RERUN FAILED".format(s)) + rb = _rerun(opts.base_checkout, brec, s, opts.jobs, "base") if brec else None + rc = _rerun(opts.cand_checkout, crec, s, opts.jobs, "cand") if crec else None + if rc is None and rb is None: + detail.append("seed {}: BOTH reruns produced no record (no evidence either way)" + .format(s)) continue - sb = evaluate(rb)[0] - sc = evaluate(rc)[0] - if sb == "PASS" and sc != "PASS": + if rc is None: + # The CANDIDATE failed where the base did not. That is not missing evidence, it + # IS the regression: crashing or emitting no record is worse than passing. + # Discarding it -- as this script first did -- let a candidate that failed on + # every seed be declared "not confirmed". + worse += 1 + detail.append("seed {}: CANDIDATE PRODUCED NO RECORD (counts against candidate)" + .format(s)) + continue + if rb is None: + detail.append("seed {}: base rerun produced no record; pair unusable".format(s)) + continue + # the SAME classifier the gate uses, so a metrics-only regression is judged here + # exactly as it was there + verdict, note = classify(rb, rc) + if is_blocking(verdict, k[0], strict): worse += 1 else: same += 1 - detail.append("seed {}: base={} cand={} (n_eff {:.0f} vs {:.0f})".format( - s, sb, sc, rb.get("n_eff", float("nan")), rc.get("n_eff", float("nan")))) - confirmed = worse > same - n_confirmed += int(confirmed) + detail.append("seed {}: {} (n_eff {:.0f} vs {:.0f}){}".format( + s, verdict, rb.get("n_eff", float("nan")), rc.get("n_eff", float("nan")), + " [" + note + "]" if note else "")) + + valid = worse + same + if valid < opts.min_valid: + status = ("INCONCLUSIVE -- {}/{} valid pairs, need {}: NOT cleared" + .format(valid, len(seeds), opts.min_valid)) + n_inconclusive += 1 + elif worse > same: + status = "CONFIRMED REGRESSION -- BLOCKS ({} worse / {} not-worse)".format(worse, same) + n_confirmed += 1 + else: + status = ("NOT CONFIRMED (realization noise; does not block) ({} worse / {} not-worse)" + .format(worse, same)) print("\n{} {}".format(k[0], k[1])) for d in detail: print(" " + d) - print(" -> {} ({} worse / {} not-worse across fresh seeds)".format( - "CONFIRMED REGRESSION -- BLOCKS" if confirmed - else "NOT CONFIRMED (realization noise; does not block)", worse, same)) + print(" -> " + status) print("\n# confirmed blocking regressions: {}".format(n_confirmed)) - return 1 if n_confirmed else 0 + if n_inconclusive: + print("# INCONCLUSIVE rows (too few valid reruns): {}".format(n_inconclusive)) + # Inconclusive must NOT read as success: we failed to obtain the evidence that would clear + # the row, so the gate stays red until a human looks. + return 1 if (n_confirmed or n_inconclusive) else 0 if __name__ == "__main__": diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py new file mode 100644 index 000000000..b36a552b6 --- /dev/null +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py @@ -0,0 +1,96 @@ +#!/usr/bin/env python +"""Unit tests for the confirm-on-fail accounting. + +These cover the ways a confirmation step can WRONGLY CLEAR a real regression, which is the only +dangerous direction: a false block costs a rerun, a false clear ships a bug. + +Run: python test_confirm_regressions.py +""" +import sys + +import confirm_regressions as CR +from compare_shape_results import classify, is_blocking + +STRICT = {"GMM", "AV"} + + +def _rec(kind="GMM", target="t", n_eff=3000.0, js=0.0001, bias=0.001): + return dict(kind=kind, target=target, ndim=4, ncomp=1, target_seed=101, n_eff=n_eff, + n_ess=n_eff * 3, js=[js, js], js_floor=[0.0005, 0.0005], + mean_pull=[0.005, 0.005], width_ratio=[1.001, 1.001], corr_diff_max=0.005, + rel_err=0.01, bias_ln=bias, error=None) + + +def test_metrics_only_regression_is_recognised(): + """The comparator blocks on REGRESSION(metrics) too. A confirm step that only knew about + PASS->non-PASS reported 'nothing to confirm' and exited 0 on a real n_eff collapse.""" + b, c = _rec(n_eff=4000.0), _rec(n_eff=400.0) # 10x n_eff drop, both still PASS + verdict, _ = classify(b, c) + assert verdict == "REGRESSION(metrics)", verdict + assert is_blocking(verdict, "GMM", STRICT) + + +def _run_with(monkey_results, seeds=(1, 2, 3), min_valid=None): + """Drive main() with _rerun stubbed to a scripted sequence of (base, cand) records.""" + calls = {"i": 0} + + def fake_rerun(checkout, rec, seed, jobs, tag): + pair = monkey_results[calls["i"] // 2] + out = pair[0] if tag == "base" else pair[1] + calls["i"] += 1 + return out + + orig = CR._rerun + CR._rerun = fake_rerun + try: + import json, tempfile, os + b, c = _rec(n_eff=4000.0), _rec(n_eff=400.0) + paths = [] + for recs in ([b], [c]): + fd, p = tempfile.mkstemp(suffix=".json") + os.close(fd) + json.dump(recs, open(p, "w")) + paths.append(p) + argv = [paths[0], paths[1], "--base-checkout", "/b", "--cand-checkout", "/c", + "--seeds", ",".join(str(s) for s in seeds)] + if min_valid is not None: + argv += ["--min-valid", str(min_valid)] + return CR.main(argv) + finally: + CR._rerun = orig + + +def test_candidate_crash_counts_against_the_candidate(): + """If the candidate produces no record where the base does, that IS the regression. + Discarding those pairs let a candidate that failed on every seed be 'not confirmed'.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(good, None), (good, None), (good, None)]) + assert rc == 1, "candidate produced no record on every seed but was cleared (rc={})".format(rc) + + +def test_insufficient_valid_pairs_is_inconclusive_not_a_pass(): + """Missing evidence must not read as 'cleared'.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(None, None), (None, None), (None, None)]) + assert rc == 1, "zero valid pairs was reported as success (rc={})".format(rc) + + +def test_genuine_noise_clears(): + """A row that is equivalent at fresh seeds must clear, or the step is useless.""" + good = _rec(n_eff=4000.0) + rc = _run_with([(good, good), (good, good), (good, good)]) + assert rc == 0, "equivalent arms were reported as a confirmed regression (rc={})".format(rc) + + +def test_real_regression_is_confirmed(): + good, bad = _rec(n_eff=4000.0), _rec(n_eff=200.0) + rc = _run_with([(good, bad), (good, bad), (good, bad)]) + assert rc == 1, "a reproducible 20x n_eff drop was not confirmed (rc={})".format(rc) + + +if __name__ == "__main__": + for name, fn in sorted(globals().items()): + if name.startswith("test_"): + fn() + print("PASS", name) + print("confirm-on-fail accounting holds") From cbb91b9bca282bab9c5788438252b89ab44d899f Mon Sep 17 00:00:00 2001 From: Richard O'Shaughnessy Date: Thu, 6 Aug 2026 18:49:14 -0700 Subject: [PATCH 3/4] gate: point the runner docs at the wired enforcement flags 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 --- .../integrators/run_shape_recovery.sh | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh index df7970c4c..95c870fef 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/run_shape_recovery.sh @@ -5,11 +5,17 @@ # is a hard cut on a stochastic quantity, so any cell sitting near a threshold flips on # realization alone. Before treating a blocking regression as real, re-test it at fresh seeds: # -# confirm_regressions.py base.json cand.json \ -# --base-checkout DIR --cand-checkout DIR --repeats 5 +# compare_shape_results.py base.json cand.json \ +# --confirm-base-checkout DIR --confirm-cand-checkout DIR --confirm-repeats 5 +# +# With those flags the comparison ENFORCES confirmation: blocking rows are re-tested and the exit +# code is the confirmed verdict. Without them it still exits 1 on a blocking row, but says so +# explicitly rather than implying the row was confirmed. (confirm_regressions.py also runs +# standalone against an existing pair of JSONs.) # # It re-runs only the disputed cells, in BOTH arms, at several new run seeds, and blocks only if -# the candidate is worse in a majority. Worked example: `GMM mix_d6_n3_s303` was reported as a +# the candidate is worse in a majority. A candidate that produces NO record where the base did +# counts against the candidate, and too few usable pairs is INCONCLUSIVE (exit 1), never a clear. Worked example: `GMM mix_d6_n3_s303` was reported as a # blocking REGRESSION in two consecutive full runs (base 119, candidate 66) and looked # reproducible -- but at 5 fresh seeds the two arms were BIT-IDENTICAL (93/93, 80/80, 119/119, # 95/95, 96/96) and 4 of the 5 starved. The cell simply sits on the n_eff=100 floor; its PASS at From 089ed5518aff8c0df66a281b330d088702b2c070 Mon Sep 17 00:00:00 2001 From: Richard O'Shaughnessy Date: Thu, 6 Aug 2026 19:02:34 -0700 Subject: [PATCH 4/4] gate: a missing candidate record is a blocking regression 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 --- .../integrators/compare_shape_results.py | 10 ++++++-- .../integrators/confirm_regressions.py | 9 +++++-- .../integrators/test_confirm_regressions.py | 24 +++++++++++++++++++ 3 files changed, 39 insertions(+), 4 deletions(-) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py index 81d25da5c..1d2bf33d2 100755 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/compare_shape_results.py @@ -47,8 +47,14 @@ def classify(b, c): "no blocking regressions to confirm" and exited 0. Two copies of this logic will always drift; there is now one. """ - if b is None or c is None: - return "ONLY-IN-" + ("CANDIDATE" if b is None else "BASE"), "" + if c is None and b is not None: + # The candidate produced NO record for a row the base did. That is a regression, not a + # bookkeeping curiosity: a candidate that crashes before emitting a result would otherwise + # be classified ONLY-IN-BASE, never reach confirmation, and exit the gate successfully -- + # bypassing the fail-closed rerun logic entirely. + return "REGRESSION(missing-in-candidate)", "candidate produced no record for this row" + if b is None: + return "ONLY-IN-CANDIDATE", "" st_b, _ = evaluate(b) st_c, why_c = evaluate(c) sb, sc = _summ(b), _summ(c) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py index a842d8cdf..15224582e 100644 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/confirm_regressions.py @@ -119,8 +119,13 @@ def main(argv=None): worse = same = 0 detail = [] for s in seeds: - rb = _rerun(opts.base_checkout, brec, s, opts.jobs, "base") if brec else None - rc = _rerun(opts.cand_checkout, crec, s, opts.jobs, "cand") if crec else None + # The cell to re-run is defined by whichever record exists -- for a + # REGRESSION(missing-in-candidate) row the candidate has no record, but the base + # record still tells us which (kind, dim, ncomp, seed) to run, so the candidate CAN + # and must be re-tested rather than written off. + spec = brec if brec is not None else crec + rb = _rerun(opts.base_checkout, spec, s, opts.jobs, "base") + rc = _rerun(opts.cand_checkout, spec, s, opts.jobs, "cand") if rc is None and rb is None: detail.append("seed {}: BOTH reruns produced no record (no evidence either way)" .format(s)) diff --git a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py index b36a552b6..7f985b603 100644 --- a/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py +++ b/MonteCarloMarginalizeCode/Code/test/expensive_before_merging/integrators/test_confirm_regressions.py @@ -88,6 +88,30 @@ def test_real_regression_is_confirmed(): assert rc == 1, "a reproducible 20x n_eff drop was not confirmed (rc={})".format(rc) + + +def test_missing_candidate_record_is_a_blocking_regression(): + """A candidate that emits no record for a strict row must BLOCK. + + Classified as ONLY-IN-BASE it was not a regression, so it never reached confirmation and the + gate exited 0 -- a candidate crashing before its first result would bypass the fail-closed + rerun logic entirely.""" + b = _rec() + verdict, note = classify(b, None) + assert verdict.startswith("REGRESSION"), verdict + assert is_blocking(verdict, "GMM", STRICT), "missing candidate record did not block" + # and it must be picked up as a row to confirm + from compare_shape_results import blocking_keys + keys = blocking_keys({("GMM", "t"): b}, {}, STRICT) + assert keys == [("GMM", "t")], keys + + +def test_extra_candidate_record_is_not_a_regression(): + """The reverse direction is not a defect: a NEW row in the candidate must not block.""" + verdict, _ = classify(None, _rec()) + assert not verdict.startswith("REGRESSION"), verdict + + if __name__ == "__main__": for name, fn in sorted(globals().items()): if name.startswith("test_"):