diff --git a/vero/src/vero/harbor/cli.py b/vero/src/vero/harbor/cli.py index 8b700184..61790cbf 100644 --- a/vero/src/vero/harbor/cli.py +++ b/vero/src/vero/harbor/cli.py @@ -363,6 +363,140 @@ def build_command(config_path, output, params): click.echo(f"Compiled Harbor task: {compiled}") +#: The two request shapes an OpenAI-compatible upstream may accept. Agents in +#: this repo use both (gaia is on Responses, the rest are on Chat Completions), +#: and an upstream is free to implement only one, so a 404 from the first is +#: not evidence about the model until the second has also been tried. +_PROBE_ROUTES: tuple[tuple[str, str], ...] = ( + ("/responses", "input"), + ("/chat/completions", "messages"), +) + + +def _model_is_missing(body: str) -> bool: + """True when a 404 body blames the model rather than the route. + + A route-level 404 (the upstream does not implement this path) and a + model-level 404 (the deployment does not exist) share a status code and + mean opposite things, so the body is the only thing that separates them. + Matched on the providers' own error codes, and on the Azure and OpenAI + sentences, never on a bare "does not exist". + """ + lowered = body.lower() + if "deploymentnotfound" in lowered or "model_not_found" in lowered: + return True + return "model" in lowered and "does not exist" in lowered + + +def _probe_route( + base_url: str, api_key: str, model: str, route: str, input_key: str +) -> tuple[int | None, str]: + """Ask one route for one token from `model`. Returns (status, body).""" + payload: dict[str, object] = {"model": model} + if input_key == "input": + payload["input"] = "ok" + payload["max_output_tokens"] = 16 + else: + payload["messages"] = [{"role": "user", "content": "ok"}] + payload["max_tokens"] = 16 + request = urllib.request.Request( + f"{base_url.rstrip('/')}{route}", + data=json.dumps(payload).encode(), + headers={ + "Content-Type": "application/json", + "Authorization": f"Bearer {api_key}", + # Azure OpenAI authenticates on api-key; OpenAI ignores it. + "api-key": api_key, + }, + ) + try: + with urllib.request.urlopen(request, timeout=30) as response: + return response.status, "" + except urllib.error.HTTPError as error: + return error.code, error.read().decode("utf-8", "replace")[:400] + except Exception as error: # network/DNS/timeout: inconclusive, not fatal + return None, str(error) + + +def _probe_model(base_url: str, api_key: str, model: str) -> tuple[int | None, str]: + """Ask the upstream for one token from `model`. Returns (status, body). + + Tries each route until one is conclusive. A 404 is only returned when the + body names the model; a route-level 404 falls through to the next route, + and if every route 404s on the route rather than the model the result is + reported as inconclusive so the run proceeds. + """ + last: tuple[int | None, str] = (None, "no route was reachable") + for route, input_key in _PROBE_ROUTES: + status, body = _probe_route(base_url, api_key, model, route, input_key) + if status == 404 and not _model_is_missing(body): + # This upstream does not serve this route. Says nothing about the + # model; keep the result only as a fallback and try the next one. + last = (None, f"{route} is not served by this upstream") + continue + return status, body + return last + + +def _preflight_models(config) -> None: + """Refuse to launch when a configured model is not deployed upstream. + + A missing deployment is invisible until the run is over: the gateway + forwards the request, the upstream 404s, the agent makes no progress, and + every case is scored 0.0 as an honest-looking task failure. That costs a + full optimizer trial to discover. This costs one token per model. + + Only a definitive 404 blocks. Anything else (a timeout, a rate limit, a + 503) is inconclusive and must not stop a run that would have succeeded. + """ + gateway = getattr(config, "inference_gateway", None) + if gateway is None: + return + api_key = os.environ.get(gateway.upstream_api_key_env) + if not api_key: + return + base_url = gateway.default_upstream_base_url + if gateway.upstream_base_url_env: + base_url = os.environ.get(gateway.upstream_base_url_env) or base_url + + scopes: dict[str, str] = {} + for scope_name in ("producer", "evaluation", "finalization"): + scope = getattr(gateway, scope_name, None) + if scope is None: + continue + for name in scope.allowed_models: + scopes.setdefault(name, scope_name) + + missing: list[str] = [] + for name, scope_name in scopes.items(): + # A provider prefix is meaningful to a routing proxy and meaningless to + # a single-provider endpoint, so try the configured name first and only + # fall back to the bare one. Reporting a model missing on the strength + # of one spelling would block a run that would have worked. + candidates = [name] + if "/" in name: + candidates.append(name.split("/", 1)[1]) + for candidate in candidates: + status, body = _probe_model(base_url, api_key, candidate) + if status != 404: + break + if status == 404: + missing.append(f"{name} ({scope_name} scope): {body.strip()}") + elif status is not None and status >= 400: + click.echo( + f"Preflight: {name} returned HTTP {status}; continuing " + f"(only a 404 is treated as fatal)" + ) + if missing: + raise click.ClickException( + "these models are not deployed on " + f"{base_url} and every request to them would fail:\n - " + + "\n - ".join(missing) + + "\nNote a model can appear in GET /models (the catalogue) and " + "still have no deployment." + ) + + @harbor.command( "run", context_settings={"ignore_unknown_options": True}, @@ -409,6 +543,7 @@ def run_command(config_path, agent, model, environment, params, env_file, extra) if model is not None: resolved.setdefault("optimizer_model", model) config = load_harbor_build_config(config_path, params=resolved) + _preflight_models(config) with tempfile.TemporaryDirectory(prefix="vero-harbor-") as temporary: task = compile_harbor_task( config, diff --git a/vero/tests/test_v05_cli.py b/vero/tests/test_v05_cli.py index d18534c8..d58f45c8 100644 --- a/vero/tests/test_v05_cli.py +++ b/vero/tests/test_v05_cli.py @@ -6,6 +6,7 @@ import sys from pathlib import Path +import pytest from click.testing import CliRunner import vero @@ -309,3 +310,150 @@ def test_cli_rejects_options_that_do_not_apply(tmp_path: Path): assert "are only valid with --produce" in producer_timeout.output assert wandb.exit_code == 2 assert "require --wandb-project" in wandb.output + + +class _Gateway: + """Minimal stand-in for InferenceGatewaySpec's preflight-relevant surface.""" + + upstream_api_key_env = "OPENAI_API_KEY" + upstream_base_url_env = "OPENAI_BASE_URL" + default_upstream_base_url = "https://api.openai.com/v1" + finalization = None + + def __init__(self, producer, evaluation): + self.producer = type("S", (), {"allowed_models": producer})() + self.evaluation = type("S", (), {"allowed_models": evaluation})() + + +class _Build: + def __init__(self, gateway): + self.inference_gateway = gateway + + +def test_preflight_blocks_only_on_a_definitively_missing_deployment(monkeypatch): + """A missing deployment is otherwise invisible until a whole trial has burned. + + The upstream 404s, the agent makes no progress, and every case is scored 0.0 + as an honest-looking task failure. + """ + import click + + from vero.harbor import cli as harbor_cli + + monkeypatch.setenv("OPENAI_API_KEY", "key") + monkeypatch.setenv("OPENAI_BASE_URL", "https://example.invalid/openai/v1") + probed: list[str] = [] + + def _probe(base_url, api_key, model): + probed.append(model) + if model == "dead-model": + return 404, '{"error": {"code": "DeploymentNotFound"}}' + return 200, "" + + monkeypatch.setattr(harbor_cli, "_probe_model", _probe) + + with pytest.raises(click.ClickException) as raised: + harbor_cli._preflight_models( + _Build(_Gateway(["gpt-5.3-codex"], ["dead-model"])) + ) + assert "dead-model (evaluation scope)" in str(raised.value) + assert "DeploymentNotFound" in str(raised.value) + # A provider prefix routes on a proxy and is meaningless to a single + # provider endpoint, so the configured spelling is tried first. + probed.clear() + harbor_cli._preflight_models(_Build(_Gateway(["openai/gpt-4o"], ["gpt-4o"]))) + assert probed == ["openai/gpt-4o", "gpt-4o"] + + +def test_preflight_falls_back_to_the_bare_name_before_calling_a_model_missing( + monkeypatch, +): + """One spelling 404ing is not evidence the model is absent. + + Azure serves `gpt-5.3-codex` and 404s `openai/gpt-5.3-codex`; a routing + proxy does the reverse. Blocking on the first spelling would refuse a run + that would have worked. + """ + from vero.harbor import cli as harbor_cli + + monkeypatch.setenv("OPENAI_API_KEY", "key") + monkeypatch.setenv("OPENAI_BASE_URL", "https://example.invalid/openai/v1") + probed: list[str] = [] + + def _probe(base_url, api_key, model): + probed.append(model) + if "/" in model: + return 404, '{"error": {"code": "DeploymentNotFound"}}' + return 200, "" + + monkeypatch.setattr(harbor_cli, "_probe_model", _probe) + harbor_cli._preflight_models(_Build(_Gateway(["openai/gpt-5.3-codex"], []))) + assert probed == ["openai/gpt-5.3-codex", "gpt-5.3-codex"] + + +def test_probe_model_separates_a_missing_route_from_a_missing_model(monkeypatch): + """An upstream that serves only Chat Completions must not read as missing. + + Both failures are HTTP 404 and they mean opposite things, so the body is + the only discriminator. gaia's agent uses the Responses API and the rest + use Chat Completions, so either route may legitimately be absent. + """ + from vero.harbor import cli as harbor_cli + + seen: list[str] = [] + + def _route(base_url, api_key, model, route, input_key): + seen.append(route) + if route == "/responses": + return 404, '{"detail": "Not Found"}' # the route, not the model + return 200, "" + + monkeypatch.setattr(harbor_cli, "_probe_route", _route) + assert harbor_cli._probe_model("https://x/v1", "k", "m") == (200, "") + assert seen == ["/responses", "/chat/completions"] + + # A model-level 404 on the first route is conclusive: do not keep probing. + seen.clear() + monkeypatch.setattr( + harbor_cli, + "_probe_route", + lambda b, k, m, route, i: ( + seen.append(route), + (404, '{"error": {"code": "DeploymentNotFound"}}'), + )[1], + ) + status, _ = harbor_cli._probe_model("https://x/v1", "k", "m") + assert status == 404 + assert seen == ["/responses"] + + # Neither route served: inconclusive, so the run is allowed to proceed. + seen.clear() + monkeypatch.setattr( + harbor_cli, + "_probe_route", + lambda b, k, m, route, i: (seen.append(route), (404, "404"))[1], + ) + status, body = harbor_cli._probe_model("https://x/v1", "k", "m") + assert status is None + assert seen == ["/responses", "/chat/completions"] + assert "not served by this upstream" in body + + +def test_preflight_does_not_block_on_inconclusive_upstream_failures(monkeypatch): + from vero.harbor import cli as harbor_cli + + monkeypatch.setenv("OPENAI_API_KEY", "key") + monkeypatch.setenv("OPENAI_BASE_URL", "https://example.invalid/openai/v1") + + for status, body in ((429, "rate limited"), (503, "unavailable"), (None, "timeout")): + monkeypatch.setattr( + harbor_cli, "_probe_model", lambda b, k, m, s=status, y=body: (s, y) + ) + # Must not raise: a transient upstream blip cannot be allowed to stop a + # run that would have succeeded. + harbor_cli._preflight_models(_Build(_Gateway(["a"], ["b"]))) + + # No credentials and no gateway are both no-ops rather than errors. + monkeypatch.delenv("OPENAI_API_KEY") + harbor_cli._preflight_models(_Build(_Gateway(["a"], ["b"]))) + harbor_cli._preflight_models(_Build(None))