From 7e4a0478d63456677b84acdfc906be4c1e395266 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Fri, 3 Jul 2026 18:30:10 +0300 Subject: [PATCH 1/4] feat(harbor): the agent's first baseline eval is budget-free The seeded baseline is the reference every candidate is implicitly compared to, yet it can never win selection (auto_best excludes base_commit by design), so metering it forced the optimizer to choose between flying blind and paying a budgeted eval for an unselectable commit. Observed live twice: the incident behind #11/#12 (an optimizer burned its whole budget measuring the baseline), and exp5's optimizer which, warned off by #12, skipped the reference entirely, could not tell its no-op edit (train 0.375) from an improvement, and quit with 3 of 5 evals unspent. The first eval whose transferred commit resolves to base_commit now runs unmetered (engine admin path) while results still route through the agent's real split tier. Capped at one: later baseline evals debit normally, so free compute is bounded. /status exposes base_commit and whether the free reference eval is still available. Co-Authored-By: Claude Opus 4.8 (1M context) --- vero/src/vero/harbor/protocol.py | 13 +++++++- vero/src/vero/harbor/serve.py | 1 + vero/src/vero/harbor/server.py | 28 ++++++++++++++++- vero/tests/test_harbor_server.py | 52 +++++++++++++++++++++++++++++++- 4 files changed, 91 insertions(+), 3 deletions(-) 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..c2baef7 100644 --- a/vero/src/vero/harbor/serve.py +++ b/vero/src/vero/harbor/serve.py @@ -221,6 +221,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, diff --git a/vero/src/vero/harbor/server.py b/vero/src/vero/harbor/server.py index f75cf6e..cd88eb7 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,26 @@ 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 + ) + if free_baseline: + self._free_baseline_used = True + exp = await self.engine.evaluate( + replace(req, commit=sha), admin=admin or free_baseline + ) + # 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 +121,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/tests/test_harbor_server.py b/vero/tests/test_harbor_server.py index 16986a4..6b0c9b7 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,52 @@ 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 From 0874f11d32690ad4e88a93fe5e2e1f6de2a7da67 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 4 Jul 2026 08:59:03 +0300 Subject: [PATCH 2/4] fix(harbor): consume the free baseline slot only after a successful eval Setting _free_baseline_used before the await burned the agent's one free baseline on a transient engine failure, forcing the retry to be metered: the exact pay-for-it failure the feature prevents. Move the flag write to after a successful evaluate (safe in the single-threaded asyncio loop, no await between the check and the write) so a failed baseline eval can be retried for free. Adds two tests: an admin re-score of the base commit must not consume the free slot, and a failed baseline eval must leave the slot available for a free retry. Addresses Greptile P1 on this PR. Co-Authored-By: Claude Opus 4.8 (1M context) --- vero/src/vero/harbor/server.py | 9 +++++++-- vero/tests/test_harbor_server.py | 28 ++++++++++++++++++++++++++++ 2 files changed, 35 insertions(+), 2 deletions(-) diff --git a/vero/src/vero/harbor/server.py b/vero/src/vero/harbor/server.py index cd88eb7..8724960 100644 --- a/vero/src/vero/harbor/server.py +++ b/vero/src/vero/harbor/server.py @@ -86,11 +86,16 @@ async def evaluate(self, req: EvalRequest, *, admin: bool = False) -> EvalSummar and sha == self.base_commit and not self._free_baseline_used ) - if free_baseline: - self._free_baseline_used = True 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 diff --git a/vero/tests/test_harbor_server.py b/vero/tests/test_harbor_server.py index 6b0c9b7..44b0078 100644 --- a/vero/tests/test_harbor_server.py +++ b/vero/tests/test_harbor_server.py @@ -172,3 +172,31 @@ 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 From 46987cc62e9e92cb3bfa862637b78ced8e82b809 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 4 Jul 2026 18:15:11 +0300 Subject: [PATCH 3/4] 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 4/4] 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"]