From 7489cf5233b2f324b987b54eb918fd985ca15613 Mon Sep 17 00:00:00 2001 From: Mengye Ren Date: Sat, 3 Oct 2026 14:39:33 -0400 Subject: [PATCH] Contract: a conditional sibling-regression allowance per benchmark An optional regression block lets a benchmark tolerate a free regression, cap it hard, and unlock the range in between only when the climbed benchmark improves by a required gain. One floor set both a benchmark's win bar and the regression it allowed on others, which could not express a memory benchmark that should climb at 5% yet allow a large speed win to cost some memory. Benchmarks without the block gate exactly as before; verdicts name the rule that decided. --- CHANGELOG.md | 8 ++ docs/contract.md | 45 +++++++ src/outerloop/contract.py | 55 +++++++++ src/outerloop/orchestrator.py | 120 +++++++++++++++---- tests/fixtures/suite_measurement_legacy.json | 1 + tests/test_contract.py | 70 +++++++++++ tests/test_measure_and_decide.py | 60 ++++++++++ tests/test_orchestrator.py | 93 +++++++++++++- 8 files changed, 430 insertions(+), 22 deletions(-) create mode 100644 tests/fixtures/suite_measurement_legacy.json diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ae64b5c..02be4acb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ Versions follow [SemVer](https://semver.org). ## [Unreleased] +- Add optional per-benchmark `regression` allowances with a free tolerance, + exclusive hard cap, and optional measured climbed-gain requirement, in + relative or absolute units. Suite rows and reports identify the applied rule. + Upgrading: no action for existing contracts; their gate behavior and measurement + signatures are unchanged. Legacy suite rows without `rule` default to + `legacy-floor`; the new row field is additive. Contracts using the new block + require this kernel version and must remove it before rolling back. + ### Upgrading Operator actions (everything else needs no action; details in each entry): diff --git a/docs/contract.md b/docs/contract.md index 796f273d..8d5a52c0 100644 --- a/docs/contract.md +++ b/docs/contract.md @@ -21,6 +21,7 @@ The knobs that shape a climb, all optional: | Knob | What it decides | | --- | --- | | `seed_env`, `min_delta` / `min_delta_rel` | Paired seeding for resampled evals, and the significance floor a delta must clear — calibrate it from seed variance, the gate enforces it | +| `regression.free_rel`, `max_rel`, `requires_gain_rel` (or absolute `free`, `max`, `requires_gain`) | Per-benchmark sibling regression allowance, independent of its win floor | | `eval_minutes`, `gpus` | Evals that need their own job (and GPUs) are dispatched to the cluster rather than run in the author's job | | `baseline: paired \| cached` | Re-measure the base tree beside every candidate, or measure it once per base and run only candidates | | `depth_k`, `sleep_k` | How many experiments an author may launch and how many times it may sleep for results | @@ -128,3 +129,47 @@ from either the fleet author or a rebound author. Supported judge backends are `kind:backend:model[endpoint=judge-profile]` with their own judge credential. Omitted models retain today's author-model inheritance. This existing setting needs no additional contract pin; self-report verification bypasses the panel. + +### Conditional sibling regressions + +When shared code changes, a sibling can opt into a separate regression policy: + +```yaml +- name: rollout-mem + command: ./bench --memory --json + metric: rollout_mem_bytes_f16 + direction: min + min_delta_rel: 0.05 + regression: + free_rel: 0.005 + max_rel: 0.5 + requires_gain_rel: 0.05 +``` + +A win on this benchmark still requires at least 5% saved. As a sibling, +regressions up to and including 0.5% are free; larger regressions below 50% +require at least 5% measured gain on the climbed benchmark. A regression of +50% or more is refused regardless of gain. Reports and suite verdict rows +record the applied rule, such as `free-allowance`, `gain-unlocked`, +`insufficient-gain`, or `hard-cap`. + +Use one unit family per block. Relative values are fractions of the absolute +baseline; `free_rel` defaults to this sibling's `min_delta_rel`, or zero. +The absolute twins `free`, `max`, and `requires_gain` use sibling metric units +for losses and climbed metric units for gains; `free` defaults to `min_delta`, +or zero. An empty block selects relative units when `min_delta_rel` is set, +otherwise absolute units. The default free allowance must also be at most max. + +All values must be finite and nonnegative. `requires_gain_rel` needs `max_rel` +(and `requires_gain` needs `max`). Without a cap, the free allowance is the +entire tolerance. With a cap but no gain requirement, every regression below +the cap is allowed. The cap is exclusive and takes precedence when free equals +max. Non-regressing siblings pass, including when max is zero. Non-finite +measurements fail closed. A zero sibling baseline scales relative allowances +to zero; a zero climbed baseline cannot unlock a relative gain requirement. +Negative baselines use their absolute magnitude as the scale. + +Omitting `regression` preserves the existing gate exactly: a regression is +refused only when it exceeds the larger of the sibling's absolute and scaled +relative significance floors. This block changes only sibling checks, not +whether this benchmark qualifies as the climbed benchmark's improvement. diff --git a/src/outerloop/contract.py b/src/outerloop/contract.py index ab1aa9d0..ff490d4b 100644 --- a/src/outerloop/contract.py +++ b/src/outerloop/contract.py @@ -84,6 +84,42 @@ class _StrictModel(BaseModel): model_config = ConfigDict(extra="forbid") +class Regression(_StrictModel): + """Sibling regression allowance, in one unit family per block. + + free_rel: fraction of the sibling's absolute baseline always allowed; + defaults to its min_delta_rel (or zero). max_rel: optional exclusive + hard cap. requires_gain_rel: climbed benchmark's minimum relative gain + to unlock the interval above free_rel; requires max_rel. The absolute + twins free/max/requires_gain use sibling/climbed metric units respectively. + Without a cap, free is the entire allowance. All values must be finite + and nonnegative; fractions may exceed one for unbounded metrics. + """ + + free_rel: float | None = Field(default=None, ge=0, allow_inf_nan=False) + max_rel: float | None = Field(default=None, ge=0, allow_inf_nan=False) + requires_gain_rel: float | None = Field(default=None, ge=0, allow_inf_nan=False) + free: float | None = Field(default=None, ge=0, allow_inf_nan=False) + max: float | None = Field(default=None, ge=0, allow_inf_nan=False) + requires_gain: float | None = Field(default=None, ge=0, allow_inf_nan=False) + + @model_validator(mode="after") + def _bounds(self) -> Regression: + relative = any(v is not None for v in (self.free_rel, self.max_rel, self.requires_gain_rel)) + absolute = any(v is not None for v in (self.free, self.max, self.requires_gain)) + if relative and absolute: + raise ValueError("regression must use either relative or absolute units, not both") + for free, cap, gain in ( + (self.free_rel, self.max_rel, self.requires_gain_rel), + (self.free, self.max, self.requires_gain), + ): + if gain is not None and cap is None: + raise ValueError("regression requires_gain requires max in the same units") + if free is not None and cap is not None and free > cap: + raise ValueError("regression free must be <= max") + return self + + class Benchmark(_StrictModel): # Slug shape only: the name reaches branch names, ledger keys, and log # labels — contract text must not shape refs or paths beyond a slug. @@ -168,6 +204,8 @@ def measurement_signature(self) -> tuple: so a future field joins the signature by default and the base-sync skip fails toward re-measuring.""" data = self.model_dump() + if self.regression is None: + data.pop("regression") # Preserve existing gate ledger signatures across the schema addition. if self.verification == "gate": data.pop("verification") @@ -209,6 +247,23 @@ def _gpu_benchmarks_dispatch(self) -> Benchmark: ) return self + @model_validator(mode="after") + def _regression_defaults(self) -> Benchmark: + r = self.regression + if r is not None and r.free is None and r.free_rel is None: + absolute = r.max is not None or r.requires_gain is not None + relative = r.max_rel is not None or r.requires_gain_rel is not None + if relative or (not absolute and self.min_delta_rel is not None): + defaults = {"free_rel": self.min_delta_rel or 0.0} + else: + defaults = {"free": self.min_delta or 0.0} + self.regression = Regression.model_validate(r.model_dump() | defaults) + return self + + # Optional sibling-only policy; never changes this benchmark's win floor. + # Omission preserves the legacy gate and measurement signature. + regression: Regression | None = None + # Cross-seed noise floor. A comparison against the RECORDED best was # measured under a different seed, so a delta inside the floor is noise, # not progress; same-seed paired comparisons are exempt by construction. diff --git a/src/outerloop/orchestrator.py b/src/outerloop/orchestrator.py index ccd5e02b..f2373ced 100644 --- a/src/outerloop/orchestrator.py +++ b/src/outerloop/orchestrator.py @@ -36,6 +36,7 @@ from outerloop.contract import ( Benchmark, Contract, + Regression, _fold, load_contract, normalize_path, @@ -477,6 +478,7 @@ class SuiteMeasurement: candidate: float regressed: bool display_digits: int | None = None + rule: str = "legacy-floor" @dataclass(frozen=True) @@ -544,7 +546,9 @@ def report(self, config: RunConfig, redact_secrets: tuple[str, ...] = ()) -> str lines.append(f"Candidate{label}: {self.candidate}") for row in self.suite: verdict = "REGRESSED" if row.regressed else "ok" - lines.append(f"Suite {row.name}: {row.baseline} -> {row.candidate} ({verdict})") + lines.append( + f"Suite {row.name}: {row.baseline} -> {row.candidate} ({verdict}; {row.rule})" + ) if self.panel_rounds: if self.panel_blocking_open: state = "blocking findings OPEN at the cap" @@ -736,19 +740,80 @@ def suite_regressed( direction: str, min_delta: float | None = None, min_delta_rel: float | None = None, + *, + regression: Regression | None = None, + climbed_gain_rel: float | Fraction | None = None, + climbed_gain: float | Fraction | None = None, ) -> bool: - """Did a sibling benchmark move the WRONG way beyond its own floor? - - Both sides are same-seed paired, so with no floor declared any wrong-way - move counts (paired noise is ~0 by construction); a declared floor gives - a stochastic eval its honest tolerance. Non-finite values fail closed — - an unmeasurable sibling must never read as "no regression".""" + """Whether a sibling violates its allowance (legacy floor when omitted).""" + return suite_regression_verdict( + baseline, + candidate, + direction, + min_delta, + min_delta_rel, + regression=regression, + climbed_gain_rel=climbed_gain_rel, + climbed_gain=climbed_gain, + )[0] + + +def suite_regression_verdict( + baseline: float, + candidate: float, + direction: str, + min_delta: float | None = None, + min_delta_rel: float | None = None, + *, + regression: Regression | None = None, + climbed_gain_rel: float | Fraction | None = None, + climbed_gain: float | Fraction | None = None, +) -> tuple[bool, str]: + """Decision and rule for reports. New policy boundaries use exact decimal + arithmetic, like reaches_floor. A zero baseline cannot unlock relative + gain; relative sibling thresholds scale to zero. Hard caps win ties.""" if not (math.isfinite(baseline) and math.isfinite(candidate)): - return True - drop = baseline - candidate if direction == "max" else candidate - baseline - if drop <= 0: - return False - return drop > benchmark_floor(baseline, min_delta, min_delta_rel) + return True, "non-finite" + if regression is None: + # Keep the original floating-point comparison for existing contracts. + drop = baseline - candidate if direction == "max" else candidate - baseline + refused = drop > 0 and drop > benchmark_floor(baseline, min_delta, min_delta_rel) + return refused, "legacy-floor" + r = regression + relative = any(v is not None for v in (r.free_rel, r.max_rel, r.requires_gain_rel)) + if not relative and all(v is None for v in (r.free, r.max, r.requires_gain)): + relative = min_delta_rel is not None + free = r.free_rel if relative else r.free + if free is None: + free = (min_delta_rel if relative else min_delta) or 0.0 + cap = r.max_rel if relative else r.max + required = r.requires_gain_rel if relative else r.requires_gain + gain = climbed_gain_rel if relative else climbed_gain + if not all( + isinstance(v, Fraction) or math.isfinite(v) + for v in (free, cap, required, gain) + if v is not None + ): + return True, "non-finite" + p, c = Fraction(repr(baseline)), Fraction(repr(candidate)) + loss = p - c if direction == "max" else c - p + if loss <= 0: + return False, "no-regression" + scale = abs(p) if relative else Fraction(1) + if cap is not None and loss >= Fraction(repr(cap)) * scale: + return True, "hard-cap" + if loss <= Fraction(repr(free)) * scale: + return False, "free-allowance" + if cap is None: + return True, "free-exceeded" + if required is None: + return False, "unconditional-allowance" + if gain is None: + return True, "insufficient-gain" + exact_gain = gain if isinstance(gain, Fraction) else Fraction(repr(gain)) + if exact_gain >= Fraction(repr(required)): + return False, "gain-unlocked" + return True, "insufficient-gain" def improved(baseline: float, candidate: float, direction: str, min_rel: float) -> bool: @@ -793,8 +858,8 @@ def make_task( f"`{bench.command}` on a private seed to verify any improvement " "claim, and the PR's CI runs the repository tests" + ( - "; changes touching shared paths are suite-gated, so no sibling " - "benchmark may regress beyond its floor" + "; changes touching shared paths are suite-gated, so every sibling " + "benchmark must satisfy its regression policy (its floor by default)" if suite_gated else "" ) @@ -1049,18 +1114,31 @@ def measure_and_decide( run_seed=seed, ) + # Exact decimal gain avoids rounding an inclusive unlock boundary down. + main_base, main_cand = Fraction(repr(baseline)), Fraction(repr(candidate)) + gain = main_cand - main_base if bench.direction == "max" else main_base - main_cand + gain_rel = gain / abs(main_base) if main_base else None suite_rows: list[SuiteMeasurement] = [] for b in siblings: sib_base = vals[f"sib-{b.name}-base"] sib_cand = vals[f"sib-{b.name}-cand"] + refused, rule = suite_regression_verdict( + sib_base, + sib_cand, + b.direction, + b.min_delta, + b.min_delta_rel, + regression=b.regression, + climbed_gain_rel=gain_rel, + climbed_gain=gain, + ) suite_rows.append( SuiteMeasurement( name=b.name, baseline=sib_base, candidate=sib_cand, - regressed=suite_regressed( - sib_base, sib_cand, b.direction, b.min_delta, b.min_delta_rel - ), + regressed=refused, + rule=rule, display_digits=b.display_digits, ) ) @@ -2526,13 +2604,13 @@ def pr_body( suite_lines = [ "", "Shared code was touched, so every sibling benchmark was re-measured " - "on both sides (paired seed): none regressed beyond its floor.", + "on both sides (paired seed): all passed their sibling regression policies.", "", - "| suite benchmark | baseline | candidate |", - "| --- | --- | --- |", + "| suite benchmark | baseline | candidate | rule |", + "| --- | --- | --- | --- |", ] + [ f"| {row.name} | {fmt_metric(row.baseline, row.display_digits)} " - f"| {fmt_metric(row.candidate, row.display_digits)} |" + f"| {fmt_metric(row.candidate, row.display_digits)} | {row.rule} |" for row in result.suite ] if result.panel_blocking_open: diff --git a/tests/fixtures/suite_measurement_legacy.json b/tests/fixtures/suite_measurement_legacy.json new file mode 100644 index 00000000..f7208d0c --- /dev/null +++ b/tests/fixtures/suite_measurement_legacy.json @@ -0,0 +1 @@ +{"name": "memory", "baseline": 100.0, "candidate": 100.0, "regressed": false, "display_digits": null} diff --git a/tests/test_contract.py b/tests/test_contract.py index b3fdfc27..bad34105 100644 --- a/tests/test_contract.py +++ b/tests/test_contract.py @@ -377,3 +377,73 @@ def test_review_topup_rejects_out_of_bounds(knobs): with pytest.raises(ValidationError): ReviewTopup.model_validate(knobs) + + +@pytest.mark.parametrize( + "policy", + [ + {}, + {"free_rel": 0.005, "max_rel": 0.5, "requires_gain_rel": 0.05}, + {"free": 1, "max": 10, "requires_gain": 2}, + {"max_rel": 0.5}, + {"free_rel": 0.5, "max_rel": 0.5}, + {"free_rel": 0, "max_rel": 2}, + ], +) +def test_regression_schema_accepts(policy): + from outerloop.contract import Benchmark + + b = Benchmark( + name="mem", + command="eval", + metric="bytes", + direction="min", + min_delta_rel=0.05, + regression=policy, + ) + assert b.regression is not None + if not policy or policy == {"max_rel": 0.5}: + assert b.regression.free_rel == 0.05 + + +@pytest.mark.parametrize( + "policy", + [ + {"typo": 0.1}, + {"free_rel": -0.1}, + {"max": -1}, + {"requires_gain_rel": -1, "max_rel": 1}, + {"free_rel": 0.6, "max_rel": 0.5}, + {"free": 2, "max": 1}, + {"requires_gain": 1}, + {"requires_gain_rel": 0.05}, + {"free": 1, "max_rel": 0.5}, + {"free_rel": float("nan")}, + {"max_rel": float("inf")}, + {"max_rel": 0.01}, # inherited free_rel is 0.05 + ], +) +def test_regression_schema_rejects(policy): + from outerloop.contract import Benchmark + + with pytest.raises(ValidationError): + Benchmark( + name="mem", + command="eval", + metric="bytes", + direction="min", + min_delta_rel=0.05, + regression=policy, + ) + + +def test_regression_preserves_legacy_measurement_signature(): + from outerloop.contract import Benchmark + + b = Benchmark(name="mem", command="eval", metric="bytes", direction="min") + # Signature before this field existed, including omission of default verification. + legacy = b.model_dump(exclude={"regression", "verification"}) + expected = tuple(sorted((k, repr(v)) for k, v in legacy.items() if k not in b._WORKFLOW_DIALS)) + assert b.measurement_signature() == expected + changed = Benchmark.model_validate(b.model_dump() | {"regression": {"free_rel": 0.005}}) + assert changed.measurement_signature() != expected diff --git a/tests/test_measure_and_decide.py b/tests/test_measure_and_decide.py index 061fdaa8..86d44d57 100644 --- a/tests/test_measure_and_decide.py +++ b/tests/test_measure_and_decide.py @@ -532,3 +532,63 @@ def fail(p, target): assert isinstance(second, MeasureOK) and second.baseline == 0.5 assert second.baseline_note assert len(measured) == 2 and path.read_bytes() == saved + + +@pytest.mark.parametrize( + "memory,passes,rule", [(133, True, "gain-unlocked"), (160, False, "hard-cap")] +) +@pytest.mark.parametrize("direction,main_candidate", [("max", 127), ("min", 73)]) +def test_conditional_memory_gate(memory, passes, rule, direction, main_candidate): + text = ( + CONTRACT.replace("direction: max", "direction: min") + .replace("metric: r2\n direction: min", f"metric: r2\n direction: {direction}") + .replace( + "min_delta: 0.02", + """min_delta_rel: 0.05 + regression: + free_rel: 0.005 + max_rel: 0.5 + requires_gain_rel: 0.05""", + ) + ) + out = _decide( + FakeMeasurer( + { + "baseline": 100, + "candidate": main_candidate, + "sib-sib-base": 100, + "sib-sib-cand": memory, + } + ), + measured_paths=("src/shared/util.py",), + text=text, + ) + assert isinstance(out, MeasureOK) is passes + assert out.suite[0].rule == rule + assert out.suite[0].regressed is not passes + if not passes: + assert out.outcome == "suite-regression" + + +@pytest.mark.parametrize( + "baseline,candidate,passes", + [(0.3, 0.314999, False), (0.3, 0.315, True), (0.3, 0.315001, True), (0, 0.03, False)], +) +def test_suite_unlock_uses_exact_measured_relative_gain(baseline, candidate, passes): + text = CONTRACT.replace( + "min_delta: 0.02", + """min_delta_rel: 0.05 + regression: + free_rel: 0.005 + max_rel: 0.5 + requires_gain_rel: 0.05""", + ) + out = _decide( + FakeMeasurer( + {"baseline": baseline, "candidate": candidate, "sib-sib-base": 100, "sib-sib-cand": 67} + ), + measured_paths=("src/shared/util.py",), + text=text, + ) + assert isinstance(out, MeasureOK) is passes + assert out.suite[0].rule == ("gain-unlocked" if passes else "insufficient-gain") diff --git a/tests/test_orchestrator.py b/tests/test_orchestrator.py index c547a450..068b5493 100644 --- a/tests/test_orchestrator.py +++ b/tests/test_orchestrator.py @@ -1503,7 +1503,8 @@ def test_pr_body_carries_the_suite_table(tmp_path: Path) -> None: ) body = pr_body(result, CONFIG, redact_secrets=()) assert "| sokoban | 0.8 | 0.8 |" in body - assert "none regressed beyond its floor" in body + assert "all passed their sibling regression policies" in body + assert "legacy-floor" in body def test_task_names_the_suite_gate_only_when_it_exists(tmp_path: Path) -> None: @@ -3198,3 +3199,93 @@ def resume(record): assert result.outcome == "no-improvement" assert len(refused) == 2 assert all(meter == (1, 1, 0.1) for meter in meters) + + +@pytest.mark.parametrize( + "candidate,gain,refused,rule", + [ + (99, 0, False, "no-regression"), + (100, 0, False, "no-regression"), + (100.5, 0, False, "free-allowance"), + (100.5001, 0.049999, True, "insufficient-gain"), + (133, 0.049999, True, "insufficient-gain"), + (133, 0.05, False, "gain-unlocked"), + (133, 0.050001, False, "gain-unlocked"), + (149.9999, 0.27, False, "gain-unlocked"), + (150, 0.27, True, "hard-cap"), + (160, 0.27, True, "hard-cap"), + (133, None, True, "insufficient-gain"), + (133, float("nan"), True, "non-finite"), + (133, float("inf"), True, "non-finite"), + (float("nan"), 0.27, True, "non-finite"), + (float("inf"), 0.27, True, "non-finite"), + ], +) +def test_conditional_suite_regression(candidate, gain, refused, rule): + from outerloop.contract import Regression + from outerloop.orchestrator import suite_regressed, suite_regression_verdict + + policy = Regression(free_rel=0.005, max_rel=0.5, requires_gain_rel=0.05) + args = dict(regression=policy, climbed_gain_rel=gain) + assert suite_regressed(100, candidate, "min", **args) is refused + assert suite_regression_verdict(100, candidate, "min", **args) == (refused, rule) + + +@pytest.mark.parametrize("direction,baseline,sign", [("min", 100, 1), ("max", -100, -1)]) +def test_regression_units_directions_and_defaults(direction, baseline, sign): + from outerloop.contract import Regression + from outerloop.orchestrator import suite_regressed + + def refused(loss, policy, **kwargs): + return suite_regressed( + baseline, + baseline + sign * loss, + direction, + min_delta=2, + min_delta_rel=0.05, + regression=policy, + **kwargs, + ) + + assert not refused(5, None) + assert refused(5.01, None) + assert not refused(5, Regression()) + assert refused(5.01, Regression()) + assert not refused(49, Regression(free_rel=0.005, max_rel=0.5)) + assert refused(50, Regression(free_rel=0.005, max_rel=0.5)) + assert not refused(2, Regression(max=10, requires_gain=3)) + assert refused(3, Regression(max=10, requires_gain=3), climbed_gain=2.99) + assert not refused(3, Regression(max=10, requires_gain=3), climbed_gain=3) + assert refused(10, Regression(max=10, requires_gain=3), climbed_gain=100) + assert refused(5, Regression(free_rel=0.05, max_rel=0.05)) + + +def test_regression_zero_and_nonfinite_baseline(): + from outerloop.contract import Regression + from outerloop.orchestrator import suite_regressed + + policy = Regression(free_rel=0.005, max_rel=0.5) + assert not suite_regressed(0, 0, "min", regression=policy) + assert suite_regressed(0, 0.001, "min", regression=policy) + for baseline in (float("nan"), float("inf"), -float("inf")): + assert suite_regressed(baseline, 1, "min", regression=policy) + assert suite_regressed(baseline, 1, "min") + + +def test_legacy_suite_row_and_rule_reports(): + import json + from dataclasses import replace + + from outerloop.orchestrator import AttemptResult, SuiteMeasurement + + legacy = json.loads( + (Path(__file__).parent / "fixtures/suite_measurement_legacy.json").read_text() + ) + row = SuiteMeasurement(**legacy) + assert row.rule == "legacy-floor" + for rule in ("legacy-floor", "gain-unlocked", "hard-cap"): + result = AttemptResult( + outcome="improved", baseline=100, candidate=73, suite=(replace(row, rule=rule),) + ) + assert rule in result.report(CONFIG) + assert rule in pr_body(result, CONFIG, redact_secrets=())