From 46987cc62e9e92cb3bfa862637b78ced8e82b809 Mon Sep 17 00:00:00 2001 From: Shehab Yasser Date: Sat, 4 Jul 2026 18:15:11 +0300 Subject: [PATCH] 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)