From 46987cc62e9e92cb3bfa862637b78ced8e82b809 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 4 Jul 2026 18:15:11 +0300 Subject: [PATCH 1/2] fix(harbor): make baseline scoring at finalize durable and retried A live trial silently skipped baseline scoring: the nested baseline eval failed transiently, the failure was swallowed by a warn-and-continue except, and the only record (a log line) died with the sidecar container at teardown. The admin volume that baseline.json was written to does not survive teardown either, so there was no durable evidence of whether the baseline was scored, skipped, or crashed. Two changes: - Retry the baseline eval (default 2 attempts) so a single transient nested-run failure does not drop the regression check. - finalize() now returns {"rewards": ..., "baseline": ...}; the baseline outcome (scores, a skip reason, or an error) is surfaced in the finalize response. The CLI writes only rewards to reward.json (the outer harness consumes its keys, unchanged) and echoes the full payload to stdout, which is captured into the trial's stdout on the host, the one channel that survives teardown. The CLI tolerates the old bare-rewards shape for a mixed-version sidecar. Baseline scoring still never fails the trial. Co-Authored-By: Claude Opus 4.8 (1M context) --- vero/src/vero/harbor/cli.py | 12 ++- vero/src/vero/harbor/serve.py | 4 + vero/src/vero/harbor/verifier.py | 124 ++++++++++++++++++++--------- vero/tests/test_harbor_app.py | 7 +- vero/tests/test_harbor_cli.py | 23 ++++++ vero/tests/test_harbor_serve.py | 4 +- vero/tests/test_harbor_verifier.py | 67 ++++++++++++---- 7 files changed, 181 insertions(+), 60 deletions(-) diff --git a/vero/src/vero/harbor/cli.py b/vero/src/vero/harbor/cli.py index e68ce4c..11e2f52 100644 --- a/vero/src/vero/harbor/cli.py +++ b/vero/src/vero/harbor/cli.py @@ -120,8 +120,14 @@ def finalize_cmd(token_file, output): from vero.harbor.auth import read_admin_token token = read_admin_token(token_file) - reward = _request("POST", "/finalize", headers={"Authorization": f"Bearer {token}"}) + resp = _request("POST", "/finalize", headers={"Authorization": f"Bearer {token}"}) + # finalize returns {"rewards": {...}, "baseline": {...}}. Only the rewards are + # the reward.json payload the outer harness consumes; the baseline outcome is + # echoed to stdout (the trial's stdout survives teardown, the admin volume does + # not) so a baseline skip or failure is durably recorded. Tolerate the older + # bare-rewards shape so a mixed-version sidecar still writes a valid reward.json. + rewards = resp["rewards"] if isinstance(resp, dict) and "rewards" in resp else resp out = Path(output) out.parent.mkdir(parents=True, exist_ok=True) - out.write_text(json.dumps(reward)) - click.echo(json.dumps(reward, indent=2)) + out.write_text(json.dumps(rewards)) + click.echo(json.dumps(resp, indent=2)) diff --git a/vero/src/vero/harbor/serve.py b/vero/src/vero/harbor/serve.py index c2baef7..51e919f 100644 --- a/vero/src/vero/harbor/serve.py +++ b/vero/src/vero/harbor/serve.py @@ -71,6 +71,9 @@ class ServeConfig(BaseModel): # write it to /baseline.json: makes regressions visible # (an optimized candidate can score WORSE than the untouched baseline). score_baseline: bool = False + # Total attempts for the finalize baseline eval (>=1): a transient nested-run + # failure once silently dropped the regression check. + baseline_score_attempts: int = 2 # volumes / token agent_volume: str @@ -233,6 +236,7 @@ async def build_components(config: ServeConfig) -> tuple[EvaluationSidecar, Veri selection_task=config.task, selection_dataset_id=config.dataset_id, score_baseline=config.score_baseline, + baseline_score_attempts=config.baseline_score_attempts, ) token = generate_token() diff --git a/vero/src/vero/harbor/verifier.py b/vero/src/vero/harbor/verifier.py index ddb2614..f885017 100644 --- a/vero/src/vero/harbor/verifier.py +++ b/vero/src/vero/harbor/verifier.py @@ -53,6 +53,7 @@ def __init__( selection_dataset_id: str | None = None, rescore_top_k: int = 3, score_baseline: bool = False, + baseline_score_attempts: int = 2, ): self.engine = engine self.admin_volume = Path(admin_volume) @@ -67,9 +68,21 @@ def __init__( self.selection_dataset_id = selection_dataset_id self.rescore_top_k = rescore_top_k self.score_baseline = score_baseline + # Baseline scoring is retried this many times total before its outcome is + # reported as an error; the nested eval can fail transiently (a nested + # harbor run crashing right after a large eval), and a single blip must + # not silently drop the regression check. + self._baseline_score_attempts = max(1, baseline_score_attempts) - async def finalize(self) -> dict[str, float]: - """Select the commit and score it on every target -> {reward_key: score}. + async def finalize(self) -> dict: + """Select the commit, score it on every target, and score the baseline. + + Returns a wrapper ``{"rewards": {reward_key: score}, "baseline": {...}}``. + ``rewards`` is the reward.json payload the outer harness consumes (the CLI + writes only that to reward.json); ``baseline`` is the outcome of baseline + scoring, surfaced here because it is otherwise invisible: the admin volume + it used to be written to does not survive teardown, and the finalize + response echoed to the trial's stdout is the only host-durable channel. A run in which the optimizer produced no scorable candidate (never submitted in ``submit`` mode; no non-baseline experiments on the @@ -89,7 +102,8 @@ async def finalize(self) -> dict[str, float]: len(self.targets), default_minimum_score, ) - return {t.reward_key: float(default_minimum_score) for t in self.targets} + rewards = {t.reward_key: float(default_minimum_score) for t in self.targets} + return {"rewards": rewards, "baseline": {"skipped": "no candidate commit"}} logger.info(f"Verifier selected commit {sha} (mode={self.reward_mode})") rewards: dict[str, float] = {} for target in self.targets: @@ -104,22 +118,29 @@ async def finalize(self) -> dict[str, float]: rewards[target.reward_key] = ( float(score) if score is not None else default_minimum_score ) - await self._maybe_score_baseline(rewards) - return rewards + baseline = await self._maybe_score_baseline(rewards) + return {"rewards": rewards, "baseline": baseline} - async def _maybe_score_baseline(self, rewards: dict[str, float]) -> None: - """Admin-score the unmodified baseline on every target and persist it. + async def _maybe_score_baseline(self, rewards: dict[str, float]) -> dict: + """Admin-score the unmodified baseline on every target and report it. An optimized candidate can score WORSE than the untouched baseline (observed live: a weak inner model went 0.3 -> 0.2 after optimization); without this, the regression is invisible because auto_best excludes the - baseline from selection and nothing else ever scores it. Written to - /baseline.json (NOT into reward.json, whose keys the outer - harness consumes) and logged next to the candidate's rewards. Failures - here never fail the trial. + baseline from selection and nothing else ever scores it. + + Returns a structured outcome (``{"scores": ...}`` / ``{"error": ...}`` / + ``{"skipped": ...}``) that ``finalize`` surfaces in its response, so a + skip or failure is durably recorded rather than lost. A live trial once + skipped this silently: the nested baseline eval failed transiently and + the only record (a log line) died with the container at teardown. So the + eval is retried once, and any failure is returned instead of swallowed. + Baseline scoring still never fails the trial (reward.json is unaffected). + A best-effort copy is also written to /baseline.json for + in-cluster debugging while the sidecar is alive. """ if not self.score_baseline: - return + return {"skipped": "score_baseline is disabled"} if not self.base_commit: # Misconfiguration must not be a silent no-op: the operator asked # for baseline scoring and would otherwise never learn it is off. @@ -127,33 +148,60 @@ async def _maybe_score_baseline(self, rewards: dict[str, float]) -> None: "score_baseline=True but base_commit is not set; skipping " "baseline scoring." ) - return - try: - baselines: dict[str, float] = {} - for target in self.targets: - exp = await self.engine.evaluate_admin( - task=target.task, - dataset_id=target.dataset_id, - split=target.split, - commit=self.base_commit, - sample_ids=target.sample_ids, - ) - score = exp.result.score() - baselines[target.reward_key] = ( - float(score) if score is not None else default_minimum_score - ) - self.admin_volume.mkdir(parents=True, exist_ok=True) - (self.admin_volume / "baseline.json").write_text( - json.dumps(baselines, indent=2) - ) - for key, value in rewards.items(): - base = baselines.get(key) - tag = " (REGRESSION vs baseline)" if base is not None and value < base else "" - logger.info( - "finalize: %s=%s baseline=%s%s", key, value, base, tag + return {"skipped": "base_commit is not set"} + + last_error: Exception | None = None + for attempt in range(1, self._baseline_score_attempts + 1): + try: + baselines: dict[str, float] = {} + for target in self.targets: + exp = await self.engine.evaluate_admin( + task=target.task, + dataset_id=target.dataset_id, + split=target.split, + commit=self.base_commit, + sample_ids=target.sample_ids, + ) + score = exp.result.score() + baselines[target.reward_key] = ( + float(score) if score is not None else default_minimum_score + ) + # Best-effort local copy (admin volume does not survive teardown; + # the return value is the durable record). + try: + self.admin_volume.mkdir(parents=True, exist_ok=True) + (self.admin_volume / "baseline.json").write_text( + json.dumps(baselines, indent=2) + ) + except OSError: + logger.warning("could not write baseline.json to the admin volume") + for key, value in rewards.items(): + base = baselines.get(key) + tag = ( + " (REGRESSION vs baseline)" + if base is not None and value < base + else "" + ) + logger.info("finalize: %s=%s baseline=%s%s", key, value, base, tag) + return {"scores": baselines, "attempts": attempt} + except Exception as exc: # noqa: BLE001 - never fail the trial on baseline scoring + last_error = exc + logger.warning( + "baseline scoring attempt %d/%d failed: %s", + attempt, + self._baseline_score_attempts, + exc, ) - except Exception: - logger.exception("baseline scoring failed; reward.json is unaffected") + logger.exception( + "baseline scoring failed after %d attempt(s); reward.json is unaffected", + self._baseline_score_attempts, + exc_info=last_error, + ) + return { + "error": str(last_error), + "error_type": type(last_error).__name__ if last_error else None, + "attempts": self._baseline_score_attempts, + } async def _select_commit(self) -> str: if self.reward_mode == "submit": diff --git a/vero/tests/test_harbor_app.py b/vero/tests/test_harbor_app.py index c1ae87f..59892b2 100644 --- a/vero/tests/test_harbor_app.py +++ b/vero/tests/test_harbor_app.py @@ -75,7 +75,10 @@ def test_budget_exceeded_maps_to_429(self): class TestAdminEndpoint: def test_finalize_requires_token(self): verifier = MagicMock() - verifier.finalize = AsyncMock(return_value={"reward": 1.0}) + # Mock mirrors the real finalize contract: {"rewards": ..., "baseline": ...} + # (the CLI extracts "rewards" for reward.json and echoes the rest to stdout). + payload = {"rewards": {"reward": 1.0}, "baseline": {"scores": {"reward": 0.8}}} + verifier.finalize = AsyncMock(return_value=payload) client = _client(verifier=verifier) assert client.post("/finalize").status_code == 403 # no token @@ -83,7 +86,7 @@ def test_finalize_requires_token(self): verifier.finalize.assert_not_awaited() r = client.post("/finalize", headers={"Authorization": f"Bearer {TOKEN}"}) - assert r.status_code == 200 and r.json() == {"reward": 1.0} + assert r.status_code == 200 and r.json() == payload verifier.finalize.assert_awaited_once() diff --git a/vero/tests/test_harbor_cli.py b/vero/tests/test_harbor_cli.py index afab16c..bc0b125 100644 --- a/vero/tests/test_harbor_cli.py +++ b/vero/tests/test_harbor_cli.py @@ -79,4 +79,27 @@ def test_finalize_uses_token_and_writes_reward(monkeypatch, tmp_path): assert result.exit_code == 0 assert cap["url"].endswith("/finalize") assert cap["headers"]["Authorization"] == "Bearer T0KEN" + # Back-compat: a bare-rewards response (older sidecar) still writes reward.json. assert json.loads(out.read_text()) == {"reward": 1.0} + + +def test_finalize_writes_only_rewards_and_echoes_baseline(monkeypatch, tmp_path): + # New wrapper shape: reward.json gets only the rewards (the outer harness + # consumes its keys), while the baseline outcome is echoed to stdout, the one + # channel that survives teardown, so a baseline skip/failure is durably recorded. + monkeypatch.setenv("VERO_EVAL_URL", "http://sidecar:8000") + token_file = tmp_path / "tok" + token_file.write_text("T0KEN") + out = tmp_path / "reward.json" + cap: dict = {} + resp = {"rewards": {"accuracy": 0.4}, "baseline": {"scores": {"accuracy": 0.43}}} + _patch_httpx(monkeypatch, _Resp(200, resp), cap) + + result = CliRunner().invoke( + harbor, ["finalize", "--token-file", str(token_file), "--output", str(out)] + ) + assert result.exit_code == 0 + # reward.json is only the rewards, not the baseline wrapper + assert json.loads(out.read_text()) == {"accuracy": 0.4} + # the baseline outcome is visible on stdout (captured into test-stdout.txt on the host) + assert "baseline" in result.output and "0.43" in result.output diff --git a/vero/tests/test_harbor_serve.py b/vero/tests/test_harbor_serve.py index df023e8..655c389 100644 --- a/vero/tests/test_harbor_serve.py +++ b/vero/tests/test_harbor_serve.py @@ -139,7 +139,7 @@ async def test_serve_assembles_and_evaluates_and_finalizes(fixture): assert exp.result.sample_results[0].score == 1.0 # verifier selects the (only) candidate on "test" and scores it on the test target - rewards = await verifier.finalize() + rewards = (await verifier.finalize())["rewards"] assert rewards["reward"] == 1.0 @@ -197,7 +197,7 @@ async def test_finalize_does_not_run_agent_supplied_scorer(fixture): ) assert exp.result.sample_results[0].score == 0.0 # Finalize must reflect the TRUSTED score, not the agent's 1.0 scorer. - rewards = await verifier.finalize() + rewards = (await verifier.finalize())["rewards"] assert rewards["reward"] == 0.0 diff --git a/vero/tests/test_harbor_verifier.py b/vero/tests/test_harbor_verifier.py index 4672e1a..2b2c221 100644 --- a/vero/tests/test_harbor_verifier.py +++ b/vero/tests/test_harbor_verifier.py @@ -28,7 +28,7 @@ async def test_finalize_submit_scores_nominated_commit(self, tmp_path): reward_mode="submit", targets=[VerificationTarget(task="t", dataset_id="ds1", split="test", reward_key="reward")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"reward": 0.8} assert engine.evaluate_admin.await_args.kwargs["commit"] == "deadbeef" assert engine.evaluate_admin.await_args.kwargs["split"] == "test" @@ -48,7 +48,7 @@ async def test_finalize_submit_no_submission_floors_rewards(self, tmp_path): VerificationTarget(task="t2", dataset_id="ds2", split="test", reward_key="held_out"), ], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"reward": 0.0, "held_out": 0.0} engine.evaluate_admin.assert_not_awaited() @@ -67,7 +67,7 @@ async def test_finalize_emits_multiple_reward_keys(self, tmp_path): VerificationTarget(task="t2", dataset_id="ds2", split="test", reward_key="held_out"), ], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"in_domain": 0.9, "held_out": 0.4} assert engine.evaluate_admin.await_count == 2 @@ -103,7 +103,7 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): selection_task="math", targets=[VerificationTarget(task="math", dataset_id="ds1", split="test", reward_key="reward")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] # the final (target) eval is on the WINNER 'lo', chosen by admin re-score assert engine.evaluate_admin.await_args.kwargs["commit"] == "lo" assert engine.evaluate_admin.await_args.kwargs["split"] == "test" @@ -173,7 +173,7 @@ async def test_auto_best_baseline_only_floors_rewards(self, tmp_path): base_commit="base", targets=[VerificationTarget(task=None, dataset_id="ds1", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.0} # no candidate -> nothing re-scored, no target eval spent engine.evaluate_admin.assert_not_awaited() @@ -190,7 +190,7 @@ async def test_auto_best_no_experiments_floors_rewards(self, tmp_path): selection_split="train", targets=[VerificationTarget(task=None, dataset_id="ds1", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.0} engine.evaluate_admin.assert_not_awaited() @@ -236,7 +236,7 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): base_commit="base", targets=[VerificationTarget(task=None, dataset_id="ds1", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.5} assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" @@ -260,8 +260,11 @@ async def test_baseline_scored_and_persisted(self, tmp_path): score_baseline=True, targets=[VerificationTarget(task=None, dataset_id="ds", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() - assert rewards == {"accuracy": 0.2} # reward.json content unchanged + result = await v.finalize() + assert result["rewards"] == {"accuracy": 0.2} # reward.json content unchanged + # the baseline outcome is surfaced in the finalize response (durable channel: + # echoed to the trial stdout, which survives teardown; the admin volume does not) + assert result["baseline"]["scores"] == {"accuracy": 0.3} data = json.loads((tmp_path / "baseline.json").read_text()) assert data == {"accuracy": 0.3} # second admin eval was the baseline commit @@ -278,18 +281,23 @@ async def test_default_off_no_extra_evals(self, tmp_path): base_commit="base", targets=[VerificationTarget(task=None, dataset_id="ds", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.9} assert engine.evaluate_admin.await_count == 1 assert not (tmp_path / "baseline.json").exists() @pytest.mark.asyncio - async def test_baseline_failure_never_fails_trial(self, tmp_path): + async def test_baseline_failure_retries_then_reports_error_without_failing_trial(self, tmp_path): + # The baseline eval fails on every attempt (2 by default). The trial reward + # must survive, AND the failure must be surfaced in the finalize response + # (not silently swallowed): a live trial once lost its baseline check because + # the only record was a log line that died with the container at teardown. (tmp_path / "submission.json").write_text(json.dumps({"commit": "cand"})) engine = MagicMock() engine.evaluate_admin = AsyncMock( side_effect=[MagicMock(result=MagicMock(score=MagicMock(return_value=0.7))), - RuntimeError("modal down")] + RuntimeError("modal down"), # baseline attempt 1 + RuntimeError("modal down")] # baseline attempt 2 (retry) ) v = Verifier( engine=engine, @@ -299,8 +307,37 @@ async def test_baseline_failure_never_fails_trial(self, tmp_path): score_baseline=True, targets=[VerificationTarget(task=None, dataset_id="ds", split="validation", reward_key="accuracy")], ) - rewards = await v.finalize() - assert rewards == {"accuracy": 0.7} # trial reward survives baseline failure + result = await v.finalize() + assert result["rewards"] == {"accuracy": 0.7} # trial reward survives baseline failure + assert result["baseline"]["error_type"] == "RuntimeError" + assert result["baseline"]["attempts"] == 2 # tried twice before reporting + # 1 target eval + 2 baseline attempts + assert engine.evaluate_admin.await_count == 3 + assert not (tmp_path / "baseline.json").exists() # nothing persisted on failure + + @pytest.mark.asyncio + async def test_baseline_transient_failure_recovers_on_retry(self, tmp_path): + # A single transient blip on the baseline eval must not drop the check: the + # retry succeeds and the baseline score is reported normally. + (tmp_path / "submission.json").write_text(json.dumps({"commit": "cand"})) + engine = MagicMock() + engine.evaluate_admin = AsyncMock( + side_effect=[MagicMock(result=MagicMock(score=MagicMock(return_value=0.7))), # target + RuntimeError("transient"), # baseline attempt 1 + MagicMock(result=MagicMock(score=MagicMock(return_value=0.5)))] # baseline attempt 2 ok + ) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="submit", + base_commit="base", + score_baseline=True, + targets=[VerificationTarget(task=None, dataset_id="ds", split="validation", reward_key="accuracy")], + ) + result = await v.finalize() + assert result["rewards"] == {"accuracy": 0.7} + assert result["baseline"]["scores"] == {"accuracy": 0.5} + assert result["baseline"]["attempts"] == 2 @pytest.mark.asyncio async def test_missing_base_commit_warns(self, tmp_path, caplog): @@ -316,7 +353,7 @@ async def test_missing_base_commit_warns(self, tmp_path, caplog): targets=[VerificationTarget(task=None, dataset_id="ds", split="validation", reward_key="accuracy")], ) with caplog.at_level("WARNING", logger="vero.harbor.verifier"): - rewards = await v.finalize() + rewards = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.9} assert not (tmp_path / "baseline.json").exists() assert any("base_commit is not set" in m for m in caplog.messages) From 9cce15bebfa86f40f2dcd54fdc847646b42541d5 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 4 Jul 2026 18:22:57 +0300 Subject: [PATCH 2/2] fix(harbor): auto_best reverts to the baseline when no candidate beats it auto_best excludes base_commit from the candidate pool, so when every candidate regressed it still selected the least-bad one and shipped a regression (observed live: an opus optimizer on a weak haiku inner model produced only below-baseline candidates and finalize shipped one 0.10 below the baseline, even though the free baseline reference was available). Visibility alone did not prevent the harm; nothing acted on it. Add a selection floor: after the admin re-score picks the best candidate, admin- score the untouched base_commit on the selection split and revert to it when the best candidate does not strictly beat it (a statistical tie reverts too: if the optimizer cannot show an improvement, shipping the seed is the safe outcome). On by default, gated on a base_commit being set; costs one extra admin eval. Co-Authored-By: Claude Opus 4.8 (1M context) --- vero/src/vero/harbor/serve.py | 4 + vero/src/vero/harbor/verifier.py | 41 ++++++- vero/tests/test_harbor_verifier.py | 172 ++++++++++++++++++++++++++++- 3 files changed, 214 insertions(+), 3 deletions(-) diff --git a/vero/src/vero/harbor/serve.py b/vero/src/vero/harbor/serve.py index 51e919f..962805a 100644 --- a/vero/src/vero/harbor/serve.py +++ b/vero/src/vero/harbor/serve.py @@ -74,6 +74,9 @@ class ServeConfig(BaseModel): # Total attempts for the finalize baseline eval (>=1): a transient nested-run # failure once silently dropped the regression check. baseline_score_attempts: int = 2 + # auto_best never ships a candidate that fails to beat the untouched baseline + # on the selection split; it reverts to base_commit instead (needs base_commit). + auto_best_baseline_floor: bool = True # volumes / token agent_volume: str @@ -237,6 +240,7 @@ async def build_components(config: ServeConfig) -> tuple[EvaluationSidecar, Veri selection_dataset_id=config.dataset_id, score_baseline=config.score_baseline, baseline_score_attempts=config.baseline_score_attempts, + auto_best_baseline_floor=config.auto_best_baseline_floor, ) token = generate_token() diff --git a/vero/src/vero/harbor/verifier.py b/vero/src/vero/harbor/verifier.py index f885017..d0d996c 100644 --- a/vero/src/vero/harbor/verifier.py +++ b/vero/src/vero/harbor/verifier.py @@ -54,6 +54,7 @@ def __init__( rescore_top_k: int = 3, score_baseline: bool = False, baseline_score_attempts: int = 2, + auto_best_baseline_floor: bool = True, ): self.engine = engine self.admin_volume = Path(admin_volume) @@ -68,6 +69,12 @@ def __init__( self.selection_dataset_id = selection_dataset_id self.rescore_top_k = rescore_top_k self.score_baseline = score_baseline + # auto_best selection floor: never ship a candidate that fails to beat the + # untouched baseline on the selection split. Without it, auto_best (which + # excludes base_commit from the candidate pool) selects the least-bad + # candidate even when every candidate regressed, shipping a regression + # (observed live: a weak inner model, every candidate below baseline). + self.auto_best_baseline_floor = auto_best_baseline_floor # Baseline scoring is retried this many times total before its outcome is # reported as an error; the nested eval can fail transiently (a nested # harbor run crashing right after a large eval), and a single blip must @@ -284,4 +291,36 @@ async def _best_from_db(self) -> str: ) # Highest admin score wins; ties break to the earliest shortlist position. rescored.sort(key=lambda t: (-t[0], t[1])) - return rescored[0][2] + best_score, _, best_commit = rescored[0] + + # Selection floor: never ship a candidate that fails to beat the untouched + # baseline on the selection split. auto_best excludes base_commit from the + # candidate pool, so without this it selects the least-bad candidate even + # when every candidate regressed. Revert to the seed instead. Strict '>' so + # a statistical tie also reverts: if the optimizer cannot show an + # improvement, shipping the seed is the safe outcome. Needs a base_commit to + # compare against; costs one extra admin eval on the selection split. + if self.auto_best_baseline_floor and self.base_commit is not None: + base_dataset_id = self.selection_dataset_id + if base_dataset_id is None: + base_dataset_id = shortlist.iloc[0].get("dataset_subset_dataset_id") + base_exp = await self.engine.evaluate_admin( + task=self.selection_task, + dataset_id=base_dataset_id, + split=self.selection_split, + commit=self.base_commit, + ) + base_s = base_exp.result.score() + base_score = float(base_s) if base_s is not None else default_minimum_score + if best_score <= base_score: + logger.info( + "auto_best floor: best candidate %s (admin_score=%s) does not beat " + "baseline %s (admin_score=%s); reverting to base_commit.", + best_commit, best_score, self.base_commit, base_score, + ) + return self.base_commit + logger.info( + "auto_best floor: best candidate %s (%s) beats baseline (%s); keeping it.", + best_commit, best_score, base_score, + ) + return best_commit diff --git a/vero/tests/test_harbor_verifier.py b/vero/tests/test_harbor_verifier.py index 2b2c221..735763a 100644 --- a/vero/tests/test_harbor_verifier.py +++ b/vero/tests/test_harbor_verifier.py @@ -111,7 +111,9 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): assert rewards["reward"] == 0.95 @pytest.mark.asyncio - async def test_auto_best_excludes_baseline_after_rescore(self, tmp_path): + async def test_auto_best_excludes_baseline_from_ranking(self, tmp_path): + # base_commit is excluded from the candidate ranking pool. Floor off here so + # the test isolates ranking-exclusion (the floor is covered separately below). engine = MagicMock() engine.db.get_experiments_df.return_value = pd.DataFrame( { @@ -134,6 +136,7 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): selection_split="validation", base_commit="base", selection_task="math", + auto_best_baseline_floor=False, targets=[VerificationTarget(task="math", dataset_id="ds1", split="test", reward_key="reward")], ) await v.finalize() @@ -143,6 +146,169 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" +class TestAutoBestBaselineFloor: + """auto_best never ships a candidate that fails to beat the baseline. + + auto_best excludes base_commit from the candidate pool, so without a floor it + selects the least-bad candidate even when every candidate regressed (observed + live: a weak inner model, every candidate below baseline, shipped a -0.10 + regression despite the free baseline being available). The floor reverts to the + seed instead. + """ + + def _df(self): + return pd.DataFrame( + { + "dataset_subset_split": ["train", "train"], + "dataset_subset_dataset_id": ["ds1", "ds1"], + "candidate_commit": ["base", "agent"], + "mean_score": [0.3, 0.9], # agent inflated its own recorded score + "candidate_created_at": [1, 2], + } + ) + + @pytest.mark.asyncio + async def test_reverts_to_base_when_no_candidate_beats_baseline(self, tmp_path): + engine = MagicMock() + engine.db.get_experiments_df.return_value = self._df() + + # agent admin-scores 0.2 on the selection split; base admin-scores 0.3; + # the reverted base scores 0.35 on the target split (distinct values so the + # assertions can tell the target eval apart from the floor comparison). + async def _admin(*, task, dataset_id, split, commit, sample_ids=None): + if commit == "base": + score = 0.35 if split == "validation" else 0.3 + else: + score = 0.2 + return MagicMock(result=MagicMock(score=MagicMock(return_value=score))) + + engine.evaluate_admin = AsyncMock(side_effect=_admin) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="auto_best", + selection_split="train", + base_commit="base", + selection_task="math", + targets=[VerificationTarget(task="math", dataset_id="ds1", split="validation", reward_key="reward")], + ) + result = await v.finalize() + # winner reverted to base -> the emitted reward is the SEED's target-split + # score, not the regressed candidate's + assert result["rewards"] == {"reward": 0.35} + rescored = [c.kwargs["commit"] for c in engine.evaluate_admin.await_args_list] + assert "base" in rescored # base was admin-scored for the floor comparison + # the final call is the target eval of the reverted commit (validation split), + # not the floor comparison (train split) + assert engine.evaluate_admin.await_args.kwargs["commit"] == "base" + assert engine.evaluate_admin.await_args.kwargs["split"] == "validation" + + @pytest.mark.asyncio + async def test_exact_tie_reverts_to_base(self, tmp_path): + # The floor uses '<=': a statistical tie reverts. If the optimizer cannot + # show an improvement, shipping the seed is the safe outcome. Pins the + # boundary so a refactor to '<' regresses loudly. + engine = MagicMock() + engine.db.get_experiments_df.return_value = self._df() + + async def _admin(*, task, dataset_id, split, commit, sample_ids=None): + return MagicMock(result=MagicMock(score=MagicMock(return_value=0.3))) # all equal + + engine.evaluate_admin = AsyncMock(side_effect=_admin) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="auto_best", + selection_split="train", + base_commit="base", + selection_task="math", + targets=[VerificationTarget(task="math", dataset_id="ds1", split="validation", reward_key="reward")], + ) + await v.finalize() + assert engine.evaluate_admin.await_args.kwargs["commit"] == "base" + + @pytest.mark.asyncio + async def test_floor_noop_without_base_commit(self, tmp_path): + # floor on (default) but base_commit=None: the floor must silently no-op, + # never issuing an eval with commit=None, and the best candidate ships. + engine = MagicMock() + engine.db.get_experiments_df.return_value = pd.DataFrame( + { + "dataset_subset_split": ["train"], + "dataset_subset_dataset_id": ["ds1"], + "candidate_commit": ["agent"], + "mean_score": [0.9], + "candidate_created_at": [1], + } + ) + + async def _admin(*, task, dataset_id, split, commit, sample_ids=None): + return MagicMock(result=MagicMock(score=MagicMock(return_value=0.5))) + + engine.evaluate_admin = AsyncMock(side_effect=_admin) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="auto_best", + selection_split="train", + selection_task="math", + targets=[VerificationTarget(task="math", dataset_id="ds1", split="validation", reward_key="reward")], + ) + await v.finalize() + commits = [c.kwargs["commit"] for c in engine.evaluate_admin.await_args_list] + assert None not in commits + assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" + + @pytest.mark.asyncio + async def test_keeps_candidate_that_beats_baseline(self, tmp_path): + engine = MagicMock() + engine.db.get_experiments_df.return_value = self._df() + + async def _admin(*, task, dataset_id, split, commit, sample_ids=None): + score = 0.3 if commit == "base" else 0.6 # agent genuinely improves + return MagicMock(result=MagicMock(score=MagicMock(return_value=score))) + + engine.evaluate_admin = AsyncMock(side_effect=_admin) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="auto_best", + selection_split="train", + base_commit="base", + selection_task="math", + targets=[VerificationTarget(task="math", dataset_id="ds1", split="validation", reward_key="reward")], + ) + await v.finalize() + # 'agent' beats base -> it is selected and target-scored + assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" + + @pytest.mark.asyncio + async def test_floor_off_ships_least_bad_candidate(self, tmp_path): + # With the floor disabled, the old behavior stands: the best candidate is + # shipped even if it did not beat the baseline (base is never scored). + engine = MagicMock() + engine.db.get_experiments_df.return_value = self._df() + + async def _admin(*, task, dataset_id, split, commit, sample_ids=None): + return MagicMock(result=MagicMock(score=MagicMock(return_value=0.2))) + + engine.evaluate_admin = AsyncMock(side_effect=_admin) + v = Verifier( + engine=engine, + admin_volume=tmp_path, + reward_mode="auto_best", + selection_split="train", + base_commit="base", + selection_task="math", + auto_best_baseline_floor=False, + targets=[VerificationTarget(task="math", dataset_id="ds1", split="validation", reward_key="reward")], + ) + await v.finalize() + rescored = [c.kwargs["commit"] for c in engine.evaluate_admin.await_args_list] + assert "base" not in rescored + assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" + + class TestNoCandidateFallback: """finalize() floors rewards when the optimizer produced no candidate. @@ -212,7 +378,8 @@ async def test_auto_best_missing_db_still_raises(self, tmp_path): @pytest.mark.asyncio async def test_candidates_present_keeps_normal_selection(self, tmp_path): - # Regression guard: the fallback must not swallow the normal path. + # Regression guard: the fallback must not swallow the normal path. Floor off + # so this isolates candidate selection (the floor is covered separately). engine = MagicMock() engine.db.get_experiments_df.return_value = pd.DataFrame( { @@ -234,6 +401,7 @@ async def _admin(*, task, dataset_id, split, commit, sample_ids=None): reward_mode="auto_best", selection_split="train", base_commit="base", + auto_best_baseline_floor=False, targets=[VerificationTarget(task=None, dataset_id="ds1", split="validation", reward_key="accuracy")], ) rewards = (await v.finalize())["rewards"]