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/protocol.py b/vero/src/vero/harbor/protocol.py index 1f41ed7..20c0500 100644 --- a/vero/src/vero/harbor/protocol.py +++ b/vero/src/vero/harbor/protocol.py @@ -51,6 +51,10 @@ class StatusSummary: submit_enabled: bool # per (split, dataset_id): tier + whether the agent may evaluate it + remaining budget splits: list[dict] = field(default_factory=list) + # the seeded baseline sha and whether its one budget-free reference eval + # is still available (None/False when the task has no recorded baseline) + base_commit: str | None = None + free_baseline_available: bool = False def to_dict(self) -> dict: return asdict(self) @@ -98,6 +102,8 @@ def build_status( submit_enabled: bool, budget: dict[tuple[str, str], SplitBudget], split_accesses: list[SplitAccess], + base_commit: str | None = None, + free_baseline_available: bool = False, ) -> StatusSummary: """Build the agent-facing status from the budget ledger + split tiers. @@ -117,4 +123,9 @@ def build_status( "remaining_run_budget": b.remaining_run_budget, } ) - return StatusSummary(submit_enabled=submit_enabled, splits=splits) + return StatusSummary( + submit_enabled=submit_enabled, + splits=splits, + base_commit=base_commit, + free_baseline_available=free_baseline_available, + ) diff --git a/vero/src/vero/harbor/serve.py b/vero/src/vero/harbor/serve.py index 85f6c28..962805a 100644 --- a/vero/src/vero/harbor/serve.py +++ b/vero/src/vero/harbor/serve.py @@ -71,6 +71,12 @@ 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 + # 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 @@ -221,6 +227,7 @@ async def build_components(config: ServeConfig) -> tuple[EvaluationSidecar, Veri agent_volume=Path(config.agent_volume), admin_volume=Path(config.admin_volume), submit_enabled=config.submit_enabled, + base_commit=config.base_commit, ) verifier = Verifier( engine=engine, @@ -232,6 +239,8 @@ 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, + auto_best_baseline_floor=config.auto_best_baseline_floor, ) token = generate_token() diff --git a/vero/src/vero/harbor/server.py b/vero/src/vero/harbor/server.py index f75cf6e..8724960 100644 --- a/vero/src/vero/harbor/server.py +++ b/vero/src/vero/harbor/server.py @@ -55,6 +55,7 @@ def __init__( agent_volume: Path, admin_volume: Path, submit_enabled: bool = False, + base_commit: str | None = None, ): self.engine = engine self.split_accesses = split_accesses @@ -62,6 +63,8 @@ def __init__( self.agent_volume = Path(agent_volume) self.admin_volume = Path(admin_volume) self.submit_enabled = submit_enabled + self.base_commit = base_commit + self._free_baseline_used = False # ------------------------------------------------------------------ # Handlers (the HTTP layer resolves `admin` from auth and calls these) @@ -69,7 +72,31 @@ def __init__( async def evaluate(self, req: EvalRequest, *, admin: bool = False) -> EvalSummary: sha = await self._transfer_commit(req.commit) - exp = await self.engine.evaluate(replace(req, commit=sha), admin=admin) + # The agent's FIRST eval of the seeded baseline is budget-free. The + # baseline is the reference every candidate is implicitly compared to, + # yet it can never win selection (auto_best excludes base_commit), so + # metering it forces a choice between optimizing blind and paying a + # budgeted eval for a commit that cannot be selected (observed live: + # an optimizer that skipped the reference could not tell a no-op edit + # from an improvement and quit with budget unspent). Capped at one: + # later baseline evals debit normally, so free compute is bounded. + free_baseline = ( + not admin + and self.base_commit is not None + and sha == self.base_commit + and not self._free_baseline_used + ) + exp = await self.engine.evaluate( + replace(req, commit=sha), admin=admin or free_baseline + ) + # Consume the free slot only after the eval actually succeeds. Setting it + # before the await would burn the one free baseline on a transient engine + # failure (timeout, infra), forcing the agent to pay for the retry, which is + # the exact failure mode this feature prevents. Safe in the single-threaded + # asyncio loop: no await runs between the check above and this write. + if free_baseline: + self._free_baseline_used = True + # Route with the agent's real tier even when the eval was unmetered. result_path = self._route_results(exp, admin=admin) budget_remaining = None if not admin: @@ -99,6 +126,10 @@ def status(self) -> StatusSummary: submit_enabled=self.submit_enabled, budget=self.engine.budget.status(), split_accesses=self.split_accesses, + base_commit=self.base_commit, + free_baseline_available=( + self.base_commit is not None and not self._free_baseline_used + ), ) # ------------------------------------------------------------------ diff --git a/vero/src/vero/harbor/verifier.py b/vero/src/vero/harbor/verifier.py index ddb2614..d0d996c 100644 --- a/vero/src/vero/harbor/verifier.py +++ b/vero/src/vero/harbor/verifier.py @@ -53,6 +53,8 @@ def __init__( selection_dataset_id: str | None = None, 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) @@ -67,9 +69,27 @@ 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 + # 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 +109,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 +125,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 +155,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": @@ -236,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_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_server.py b/vero/tests/test_harbor_server.py index 16986a4..44b0078 100644 --- a/vero/tests/test_harbor_server.py +++ b/vero/tests/test_harbor_server.py @@ -41,7 +41,7 @@ def _experiment(split: str, commit: str = "abcdef123456") -> Experiment: ) -def _sidecar(tmp_path, *, split, submit_enabled=False): +def _sidecar(tmp_path, *, split, submit_enabled=False, base_commit=None): engine = MagicMock() engine.evaluate = AsyncMock(return_value=_experiment(split)) engine.budget = BudgetLedger( @@ -54,6 +54,7 @@ def _sidecar(tmp_path, *, split, submit_enabled=False): agent_volume=tmp_path / "agent_vol", admin_volume=tmp_path / "admin_vol", submit_enabled=submit_enabled, + base_commit=base_commit, ) # Stub the git transfer (integration-tested separately); pin the sha. sidecar._transfer_commit = AsyncMock(return_value="abcdef123456") @@ -122,3 +123,80 @@ def test_status_reports_submit_and_splits(self, tmp_path): assert status.submit_enabled is True assert status.splits[0]["split"] == "train" assert status.splits[0]["remaining_run_budget"] == 5 + + +class TestFreeBaselineEval: + """The agent's first eval of the seeded baseline is budget-free: it is the + reference every candidate is compared to and can never win selection, so + metering it forced a choice between optimizing blind and wasting budget + (observed live: exp5's optimizer skipped the reference, could not tell a + no-op edit from an improvement, and quit with budget unspent).""" + + @pytest.mark.asyncio + async def test_first_baseline_eval_is_unmetered(self, tmp_path): + sidecar = _sidecar(tmp_path, split="validation", base_commit="abcdef123456") + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + # engine.evaluate was called with admin=True (bypasses the ledger) + assert sidecar.engine.evaluate.await_args.kwargs["admin"] is True + # but results were routed with the agent tier (summary written) + dest = tmp_path / "agent_vol" / "results" / "validation__abcdef123456" + assert (dest / "summary.json").exists() + + @pytest.mark.asyncio + async def test_second_baseline_eval_is_metered(self, tmp_path): + sidecar = _sidecar(tmp_path, split="validation", base_commit="abcdef123456") + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar.engine.evaluate.await_args.kwargs["admin"] is False + + @pytest.mark.asyncio + async def test_non_baseline_commit_always_metered(self, tmp_path): + sidecar = _sidecar(tmp_path, split="validation", base_commit="other000000") + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar.engine.evaluate.await_args.kwargs["admin"] is False + + @pytest.mark.asyncio + async def test_no_base_commit_never_free(self, tmp_path): + sidecar = _sidecar(tmp_path, split="validation") # base_commit=None + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar.engine.evaluate.await_args.kwargs["admin"] is False + + def test_status_surfaces_free_baseline(self, tmp_path): + sidecar = _sidecar(tmp_path, split="train", base_commit="abcdef123456") + s = sidecar.status() + assert s.base_commit == "abcdef123456" + assert s.free_baseline_available is True + + @pytest.mark.asyncio + async def test_status_flips_after_use(self, tmp_path): + sidecar = _sidecar(tmp_path, split="validation", base_commit="abcdef123456") + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar.status().free_baseline_available is False + + @pytest.mark.asyncio + async def test_admin_baseline_eval_does_not_consume_free_slot(self, tmp_path): + # An admin re-score of the base commit (finalize, score_baseline) must leave + # the agent's free slot intact: the guard is `not admin`, but pin it so a + # refactor that moves the flag outside the `if free_baseline` block regresses. + sidecar = _sidecar(tmp_path, split="validation", base_commit="abcdef123456") + await sidecar.evaluate( + EvalRequest(dataset_id="ds1", split="validation"), admin=True + ) + assert sidecar._free_baseline_used is False + assert sidecar.status().free_baseline_available is True + + @pytest.mark.asyncio + async def test_failed_baseline_eval_leaves_free_slot_available(self, tmp_path): + # A transient engine failure must NOT burn the one free baseline: the agent + # should be able to retry for free. The flag is consumed only after success. + sidecar = _sidecar(tmp_path, split="validation", base_commit="abcdef123456") + sidecar.engine.evaluate = AsyncMock(side_effect=RuntimeError("transient infra")) + with pytest.raises(RuntimeError): + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar._free_baseline_used is False + assert sidecar.status().free_baseline_available is True + # The retry is still granted for free (admin=True bypasses the ledger). + sidecar.engine.evaluate = AsyncMock(return_value=_experiment("validation")) + await sidecar.evaluate(EvalRequest(dataset_id="ds1", split="validation")) + assert sidecar.engine.evaluate.await_args.kwargs["admin"] is True + assert sidecar._free_baseline_used is True diff --git a/vero/tests/test_harbor_verifier.py b/vero/tests/test_harbor_verifier.py index 4672e1a..735763a 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" @@ -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. @@ -173,7 +339,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 +356,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() @@ -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,9 +401,10 @@ 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 = (await v.finalize())["rewards"] assert rewards == {"accuracy": 0.5} assert engine.evaluate_admin.await_args.kwargs["commit"] == "agent" @@ -260,8 +428,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 +449,50 @@ 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, + 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} # 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, @@ -299,8 +502,10 @@ 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} + 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 +521,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)