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)