From 4e0186358fdea93f900a7f4aed161836cff8b44c Mon Sep 17 00:00:00 2001 From: Cale Smith Date: Tue, 8 Sep 2026 16:28:48 -0700 Subject: [PATCH] [BENCH-839] Drive the LLM roster from the models table sync-llm, the LLM fetch specs, and the proxy routes now read LLM rows from benchmarks_v2.models instead of a code registry. collected is the switch: sync reconciles the Coval schedule's enabled flag from it, the fetch skips uncollected rows, and the proxy refuses their turns. Client construction stays in code as a per-provider factory map, and startup logs any collected LLM model that has no client. --- runner/src/coval_bench/api/app.py | 24 +++---- runner/src/coval_bench/api/deps.py | 31 ++++++---- .../src/coval_bench/api/routers/llm_proxy.py | 31 +++++----- runner/src/coval_bench/llm/benchmark.py | 33 +++++++++- runner/src/coval_bench/llm/coval_agent.py | 59 ++++++++++++++---- runner/src/coval_bench/llm/phonely.py | 12 +++- runner/src/coval_bench/s2s/fetch_v2v.py | 62 ++++++++++++------- runner/tests/api/test_llm_proxy.py | 22 ++++++- runner/tests/unit/test_coval_agent_sync.py | 56 +++++++++++++++-- runner/tests/unit/test_s2s_fetch.py | 25 ++++++-- 10 files changed, 270 insertions(+), 85 deletions(-) diff --git a/runner/src/coval_bench/api/app.py b/runner/src/coval_bench/api/app.py index 48211720..b87a38c9 100644 --- a/runner/src/coval_bench/api/app.py +++ b/runner/src/coval_bench/api/app.py @@ -61,9 +61,9 @@ ) from coval_bench.config import Settings, get_settings from coval_bench.db.conn import lifespan_pool +from coval_bench.db.registry_store import fetch_models from coval_bench.fixture_sources import install_fixture_providers -from coval_bench.llm.phonely import PhonelyClient -from coval_bench.llm.turn import TurnClient +from coval_bench.llm.benchmark import llm_models, make_clients from coval_bench.logging import configure_logging from coval_bench.mocktools.dispatch import build_dispatcher @@ -121,16 +121,8 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: logger.warning("posthog_init_failed", exc_info=True) posthog_client = None app.state.posthog = posthog_client - llm_clients: dict[str, TurnClient] = {} - phonely_key = resolved.phonely_api_key - if phonely_key and phonely_key.get_secret_value() and resolved.phonely_agent_id: - llm_clients["phonely"] = PhonelyClient( - phonely_key.get_secret_value(), - resolved.phonely_agent_id, - resolved.phonely_base_url, - ) - else: - logger.info("llm_proxy_disabled", provider="phonely") + llm_clients = make_clients(resolved) + logger.info("llm_clients_ready", providers=sorted(llm_clients)) app.state.llm_clients = llm_clients # Built here rather than on first request: loading and cross-checking the # fixtures inside a live call would put that cost on the agent's turn. @@ -149,6 +141,14 @@ async def lifespan(app: FastAPI) -> AsyncIterator[None]: async with lifespan_pool(resolved) as pool: app.state.pool = pool app.state.settings = resolved + try: + roster = await fetch_models(pool) + except Exception: + logger.warning("llm_client_check_skipped", exc_info=True) + else: + for model in llm_models(roster): + if model.collected and model.provider not in llm_clients: + logger.error("llm_client_missing", provider=model.provider) yield finally: for llm_client in llm_clients.values(): diff --git a/runner/src/coval_bench/api/deps.py b/runner/src/coval_bench/api/deps.py index cb4a96ec..351a705e 100644 --- a/runner/src/coval_bench/api/deps.py +++ b/runner/src/coval_bench/api/deps.py @@ -26,7 +26,7 @@ from coval_bench.api import clerk from coval_bench.config import Settings from coval_bench.db.registry_store import fetch_models -from coval_bench.llm.benchmark import LLM_MODELS +from coval_bench.llm.benchmark import ProxiedModel, llm_models from coval_bench.llm.turn import TurnClient from coval_bench.registries import RegisteredModel @@ -69,17 +69,6 @@ def secret_matches(provided: str | None, expected: SecretStr | None) -> bool: return bool(value) and hmac.compare_digest(provided.encode(), value.encode()) -def get_turn_client(provider: str, request: Request) -> TurnClient: - """Return the lifespan-owned client for this provider, or fail closed.""" - if provider not in LLM_MODELS: - raise HTTPException(404, f"{provider} is not an LLM benchmark provider") - clients: dict[str, TurnClient] = getattr(request.app.state, "llm_clients", {}) - client = clients.get(provider) - if client is None: - raise HTTPException(503, f"{provider} proxy is not configured") - return client - - def require_coval_admin( authorization: str | None = Header(default=None), settings: Settings = Depends(get_settings), @@ -143,6 +132,24 @@ async def get_models( raise HTTPException(503, "the model registry is unavailable") from exc +async def get_proxied_model( + provider: str, + request: Request, + models: list[RegisteredModel] = Depends(get_models), +) -> ProxiedModel: + """The collected LLM model behind a /llm/{provider} route, or fail closed.""" + registered = next( + (m for m in llm_models(models) if m.provider == provider and m.collected), None + ) + if registered is None: + raise HTTPException(404, f"{provider} is not a collected LLM benchmark model") + clients: dict[str, TurnClient] = getattr(request.app.state, "llm_clients", {}) + client = clients.get(provider) + if client is None: + raise HTTPException(503, f"{provider} proxy is not configured") + return ProxiedModel(provider=provider, model=registered.model, client=client) + + def get_cache_locks(request: Request) -> defaultdict[Any, asyncio.Lock]: """Return the per-app cache-key locks from app state.""" return cast("defaultdict[Any, asyncio.Lock]", request.app.state.cache_locks) diff --git a/runner/src/coval_bench/api/routers/llm_proxy.py b/runner/src/coval_bench/api/routers/llm_proxy.py index 4f311fee..d463b7f5 100644 --- a/runner/src/coval_bench/api/routers/llm_proxy.py +++ b/runner/src/coval_bench/api/routers/llm_proxy.py @@ -11,7 +11,7 @@ from typing import Any import structlog -from fastapi import APIRouter, BackgroundTasks, Depends, Header, HTTPException, Path +from fastapi import APIRouter, BackgroundTasks, Depends, Header, HTTPException from psycopg_pool import AsyncConnectionPool from pydantic import BaseModel, Field from starlette.responses import JSONResponse @@ -19,14 +19,14 @@ from coval_bench.api.deps import ( bearer_token, get_pool, + get_proxied_model, get_settings, - get_turn_client, secret_matches, ) from coval_bench.config import Settings from coval_bench.db.llm_turns import insert_turn -from coval_bench.llm.benchmark import LLM_MODELS -from coval_bench.llm.turn import TurnClient, TurnError, TurnResult +from coval_bench.llm.benchmark import ProxiedModel +from coval_bench.llm.turn import TurnError, TurnResult logger = structlog.get_logger("coval_bench.api.llm_proxy") @@ -70,7 +70,7 @@ def _messages(messages: list[dict[str, Any]]) -> list[dict[str, Any]]: async def _record_turn( pool: AsyncConnectionPool[Any], *, - provider: str, + target: ProxiedModel, simulation_id: str, turn_index: int, result: TurnResult, @@ -80,8 +80,8 @@ async def _record_turn( pool, simulation_id=simulation_id, turn_index=turn_index, - provider=provider, - model=LLM_MODELS[provider], + provider=target.provider, + model=target.model, ttft_ms=result.ttft_ms, total_ms=result.total_ms, output_tokens=result.output_tokens, @@ -124,14 +124,13 @@ def _completion(call_id: str, result: TurnResult) -> dict[str, Any]: @router.post("/session", dependencies=[Depends(require_proxy_secret)]) async def create_session( - provider: str = Path(), - client: TurnClient = Depends(get_turn_client), + target: ProxiedModel = Depends(get_proxied_model), ) -> dict[str, str | None]: try: - session = await client.create_session() + session = await target.client.create_session() except TurnError as exc: - logger.warning("llm_session_failed", provider=provider, error=str(exc)) - raise HTTPException(502, f"{provider} session creation failed") from exc + logger.warning("llm_session_failed", provider=target.provider, error=str(exc)) + raise HTTPException(502, f"{target.provider} session creation failed") from exc return {"sessionId": session.call_id, "expiresAt": session.expires_at} @@ -139,17 +138,17 @@ async def create_session( async def chat( body: ChatRequest, background: BackgroundTasks, - provider: str = Path(), - client: TurnClient = Depends(get_turn_client), + target: ProxiedModel = Depends(get_proxied_model), pool: AsyncConnectionPool[Any] = Depends(get_pool), ) -> JSONResponse: if body.stream: raise HTTPException(400, "streaming responses are not supported") + provider = target.provider messages = _messages(body.messages) turn_index = sum(message.get("role") == "assistant" for message in messages) try: async with asyncio.timeout(_TURN_TIMEOUT_S): - result = await client.stream_turn(body.model, messages) + result = await target.client.stream_turn(body.model, messages) except TimeoutError as exc: logger.warning("llm_turn_timed_out", provider=provider, turn_index=turn_index) raise HTTPException(504, f"{provider} completion timed out") from exc @@ -161,7 +160,7 @@ async def chat( background.add_task( _record_turn, pool, - provider=provider, + target=target, simulation_id=body.simulation_id, turn_index=turn_index, result=result, diff --git a/runner/src/coval_bench/llm/benchmark.py b/runner/src/coval_bench/llm/benchmark.py index 0be1ce73..884644b3 100644 --- a/runner/src/coval_bench/llm/benchmark.py +++ b/runner/src/coval_bench/llm/benchmark.py @@ -5,10 +5,19 @@ from __future__ import annotations -from collections.abc import Iterable +from collections.abc import Callable, Iterable +from dataclasses import dataclass from typing import Any -LLM_MODELS = {"phonely": "phonely-agent"} +from coval_bench.config import Settings +from coval_bench.llm.phonely import PhonelyClient +from coval_bench.llm.turn import TurnClient +from coval_bench.registries.benchmarks import Benchmark +from coval_bench.registries.models import RegisteredModel + +CLIENT_FACTORIES: dict[str, Callable[[Settings], TurnClient | None]] = { + "phonely": PhonelyClient.from_settings, +} DEFAULT_PERSONA_ID = "PN3xgmsqeLDjsNNEA2e55e" ITERATION_COUNT = 1 TEMPLATE_MANAGED = ("agent_ids", "persona_ids", "test_set_ids", "metric_ids", "iteration_count") @@ -19,6 +28,26 @@ } +@dataclass(frozen=True) +class ProxiedModel: + provider: str + model: str + client: TurnClient + + +def llm_models(models: Iterable[RegisteredModel]) -> list[RegisteredModel]: + return [model for model in models if model.benchmark is Benchmark.LLM] + + +def make_clients(settings: Settings) -> dict[str, TurnClient]: + clients: dict[str, TurnClient] = {} + for provider, factory in CLIENT_FACTORIES.items(): + client = factory(settings) + if client is not None: + clients[provider] = client + return clients + + def run_template_body( display_name: str, agent_id: str, test_set_id: str, metric_id: str ) -> dict[str, Any]: diff --git a/runner/src/coval_bench/llm/coval_agent.py b/runner/src/coval_bench/llm/coval_agent.py index 07e02e42..ae5f90df 100644 --- a/runner/src/coval_bench/llm/coval_agent.py +++ b/runner/src/coval_bench/llm/coval_agent.py @@ -5,19 +5,24 @@ from __future__ import annotations +import asyncio import json from dataclasses import dataclass, field from typing import Any, Self import click import httpx +import psycopg import structlog from pydantic import BaseModel, SecretStr from coval_bench.config import Settings, get_settings +from coval_bench.db.conn import lifespan_pool +from coval_bench.db.registry_store import fetch_models from coval_bench.llm import benchmark from coval_bench.logging import configure_logging from coval_bench.platform_assets import COVAL_API_BASE, COVAL_API_KEY, CovalClient, SyncError, plan +from coval_bench.registries.models import RegisteredModel from coval_bench.variants.platforms import redact # The public API rejects MODEL_TYPE_TEXT; CHAT is the HTTP text simulator. @@ -38,6 +43,7 @@ class CovalTextAgentDefinition(BaseModel, frozen=True): proxy_secret: SecretStr test_set_id: str instruction_metric_id: str + collected: bool = True @property def customer_agent_id(self) -> str: @@ -53,7 +59,12 @@ def run_name(self) -> str: @classmethod def from_settings( - cls, provider: str, settings: Settings, *, test_set_id: str | None = None + cls, + provider: str, + settings: Settings, + *, + test_set_id: str | None = None, + collected: bool = True, ) -> Self: proxy_url = settings.llm_proxy_public_url proxy_secret = settings.llm_proxy_secret @@ -77,6 +88,7 @@ def from_settings( proxy_secret=proxy_secret, test_set_id=dental, instruction_metric_id=metric, + collected=collected, ) def agent_body(self) -> dict[str, Any]: @@ -118,16 +130,24 @@ def run_template_body(self, agent_id: str) -> dict[str, Any]: ) -def scheduled_run_body(run_name: str, run_template_id: str) -> dict[str, Any]: +def scheduled_run_body(run_name: str, run_template_id: str, *, enabled: bool) -> dict[str, Any]: return { "display_name": run_name, "run_template_id": run_template_id, "schedule_expression": SCHEDULE_EXPRESSION, "schedule_timezone": SCHEDULE_TIMEZONE, - "enabled": True, + "enabled": enabled, } +def load_llm_models(settings: Settings) -> list[RegisteredModel]: + async def _load() -> list[RegisteredModel]: + async with lifespan_pool(settings) as pool: + return benchmark.llm_models(await fetch_models(pool)) + + return asyncio.run(_load()) + + class CovalTextClient(CovalClient): def __enter__(self) -> Self: return self @@ -169,6 +189,11 @@ def create_scheduled_run(self, body: dict[str, Any]) -> dict[str, Any]: scheduled = payload.get("scheduled_run") return scheduled if isinstance(scheduled, dict) else payload + def update_scheduled_run(self, scheduled_id: str, body: dict[str, Any]) -> dict[str, Any]: + payload = self._request("PATCH", f"/scheduled-runs/{scheduled_id}", body) + scheduled = payload.get("scheduled_run") + return scheduled if isinstance(scheduled, dict) else payload + @dataclass class SyncResult: @@ -234,12 +259,19 @@ def sync( result.actions.append("run template: unchanged") template_id = str(template["id"]) - if client.find_scheduled_run(template_id) is None: + scheduled = client.find_scheduled_run(template_id) + if scheduled is None: result.actions.append("scheduled run: create") if not dry_run: - client.create_scheduled_run(scheduled_run_body(definition.run_name, template_id)) + client.create_scheduled_run( + scheduled_run_body(definition.run_name, template_id, enabled=definition.collected) + ) + elif bool(scheduled.get("enabled")) != definition.collected: + result.actions.append(f"scheduled run: {'enable' if definition.collected else 'disable'}") + if not dry_run: + client.update_scheduled_run(str(scheduled["id"]), {"enabled": definition.collected}) else: - result.actions.append("scheduled run: exists") + result.actions.append("scheduled run: unchanged") return result @@ -263,8 +295,10 @@ def sync_llm(dry_run: bool, test_set_id: str | None, coval_api_base: str) -> Non run_logger = structlog.get_logger("coval_bench.llm.coval_agent") try: definitions = [ - CovalTextAgentDefinition.from_settings(provider, settings, test_set_id=test_set_id) - for provider in benchmark.LLM_MODELS + CovalTextAgentDefinition.from_settings( + model.provider, settings, test_set_id=test_set_id, collected=model.collected + ) + for model in load_llm_models(settings) ] if dry_run: for definition in definitions: @@ -274,7 +308,7 @@ def sync_llm(dry_run: bool, test_set_id: str | None, coval_api_base: str) -> Non definition.provider: sync(client, definition, dry_run=dry_run) for definition in definitions } - except (SyncError, RuntimeError, httpx.HTTPError) as exc: + except (SyncError, RuntimeError, httpx.HTTPError, psycopg.Error) as exc: if not dry_run: run_logger.error("RUN_FAILED", error=str(exc), exc_info=exc) raise click.ClickException(str(exc)) from exc @@ -287,8 +321,11 @@ def sync_llm(dry_run: bool, test_set_id: str | None, coval_api_base: str) -> Non if dry_run: return fetchable: dict[str, str] = {} - for provider, result in results.items(): - if "scheduled run: create" in result.actions: + for definition, result in zip(definitions, results.values(), strict=True): + provider = definition.provider + if not definition.collected: + run_logger.info("llm_sync_fetch_skipped", provider=provider, reason="not_collected") + elif "scheduled run: create" in result.actions: run_logger.info( "llm_sync_fetch_deferred", provider=provider, reason="scheduled_run_created" ) diff --git a/runner/src/coval_bench/llm/phonely.py b/runner/src/coval_bench/llm/phonely.py index 1fb576e4..d328407e 100644 --- a/runner/src/coval_bench/llm/phonely.py +++ b/runner/src/coval_bench/llm/phonely.py @@ -7,13 +7,16 @@ import json import time -from typing import Any +from typing import TYPE_CHECKING, Any import httpx from coval_bench.llm.turn import Session as PhonelySession from coval_bench.llm.turn import TurnError, TurnResult +if TYPE_CHECKING: + from coval_bench.config import Settings + _SESSION_TIMEOUT = httpx.Timeout(30.0) # Turns are seconds apart; a 5s keepalive would put a TLS handshake inside most TTFTs. _LIMITS = httpx.Limits(max_connections=20, max_keepalive_connections=8, keepalive_expiry=300.0) @@ -169,6 +172,13 @@ def __init__( transport=transport or httpx.AsyncHTTPTransport(http2=True, limits=_LIMITS), ) + @classmethod + def from_settings(cls, settings: Settings) -> PhonelyClient | None: + key = settings.phonely_api_key + if not (key and key.get_secret_value() and settings.phonely_agent_id): + return None + return cls(key.get_secret_value(), settings.phonely_agent_id, settings.phonely_base_url) + def __repr__(self) -> str: return f"PhonelyClient(agent_id={self._agent_id!r})" diff --git a/runner/src/coval_bench/s2s/fetch_v2v.py b/runner/src/coval_bench/s2s/fetch_v2v.py index 974b491e..0121b468 100644 --- a/runner/src/coval_bench/s2s/fetch_v2v.py +++ b/runner/src/coval_bench/s2s/fetch_v2v.py @@ -14,7 +14,7 @@ import hashlib import importlib.resources import random -from collections.abc import Awaitable, Callable, Mapping, Sequence +from collections.abc import Awaitable, Callable, Iterable, Mapping, Sequence from dataclasses import dataclass from datetime import UTC, datetime from typing import Any, cast @@ -26,10 +26,11 @@ from coval_bench.config import Settings, get_settings from coval_bench.db.conn import lifespan_pool from coval_bench.db.models import MetricExecutor, Result, ResultStatus, RunStatus +from coval_bench.db.registry_store import fetch_models from coval_bench.db.writer import RunWriter -from coval_bench.llm.benchmark import LLM_MODELS from coval_bench.registries import METRIC_SPECS, Metric from coval_bench.registries.benchmarks import Benchmark +from coval_bench.registries.models import RegisteredModel from coval_bench.s2s.conditions import ( DATASET_ID, DEFAULT_CONDITION, @@ -129,21 +130,27 @@ class CovalRun: family=FAMILY_DENTAL, publish_samples=False, ), - # The same dental set driven over text through the LLM proxy; TTFT comes from - # the proxy's own turn log rather than from Coval. - *( +) + + +def llm_specs(models: Iterable[RegisteredModel]) -> tuple[AgentSpec, ...]: + """Every collected LLM model, driven over the same dental set through the proxy. + + TTFT comes from the proxy's own turn log rather than from Coval. + """ + return tuple( AgentSpec( - agent_id_attr=f"coval_llm_{provider}_agent_id", - provider=provider, - model=model, + agent_id_attr=f"coval_llm_{model.provider}_agent_id", + provider=model.provider, + model=model.model, test_set_id_attr="coval_s2s_dental_test_set_id", family=FAMILY_LLM_DENTAL, publish_samples=False, benchmark=Benchmark.LLM, ) - for provider, model in LLM_MODELS.items() - ), -) + for model in models + if model.benchmark is Benchmark.LLM and model.collected + ) def _client(settings: Settings) -> httpx.AsyncClient: @@ -1050,6 +1057,21 @@ def note_sample_candidate(coval_run: CovalRun, dataset_id: str) -> None: return RunStatus.FAILED, len(statuses) +def _require_family_test_sets(settings: Settings, specs: Sequence[AgentSpec]) -> None: + """A family's test set is required once one of its agents is configured. + + Unset would otherwise skip the agent with a warning that reads the same as + never having configured it. + """ + for spec in specs: + if not spec.test_set_id_attr or not getattr(settings, spec.agent_id_attr, None): + continue + if not (getattr(settings, spec.test_set_id_attr) or "").strip(): + raise RuntimeError( + f"{spec.test_set_id_attr} is required when {spec.agent_id_attr} is set" + ) + + async def fetch_and_write_v2v( settings: Settings | None = None, *, @@ -1099,16 +1121,9 @@ async def fetch_and_write_v2v( raw_dental = settings.coval_s2s_dental_test_set_id if raw_dental is not None and not raw_dental.strip(): raise RuntimeError("coval_s2s_dental_test_set_id must not be blank") - # A family's test set is required once one of its agents is configured; - # unset would otherwise skip the agent with a warning that reads the same - # as never having configured it. - for spec in specs: - if not spec.test_set_id_attr or not getattr(settings, spec.agent_id_attr): - continue - if not (getattr(settings, spec.test_set_id_attr) or "").strip(): - raise RuntimeError( - f"{spec.test_set_id_attr} is required when {spec.agent_id_attr} is set" - ) + if benchmark is Benchmark.LLM and not raw_dental: + raise RuntimeError("coval_s2s_dental_test_set_id is required for the LLM benchmark") + _require_family_test_sets(settings, specs) # The noisy persona only separates conditions within a test set, so without # one it would silently never take effect. raw_noisy = settings.coval_s2s_noisy_persona_id @@ -1139,13 +1154,16 @@ async def fetch_and_write_v2v( raise RuntimeError(f"no _VALUE_MAPPERS entry for configured metrics: {', '.join(unmapped)}") async with _client(settings) as client, lifespan_pool(settings) as pool: + if benchmark is Benchmark.LLM: + specs = llm_specs(await fetch_models(pool)) + _require_family_test_sets(settings, specs) writer = RunWriter(pool) statuses: dict[str, RunStatus] = {} total_ingested = 0 matched_run_ids: set[str] = set() sampled_runs: list[SampleRun] = [] for spec in specs: - agent_id = getattr(settings, spec.agent_id_attr) + agent_id = getattr(settings, spec.agent_id_attr, None) if not agent_id: logger.warning("agent_id_unset", provider=spec.provider, attr=spec.agent_id_attr) continue diff --git a/runner/tests/api/test_llm_proxy.py b/runner/tests/api/test_llm_proxy.py index e0ef8193..99151cb2 100644 --- a/runner/tests/api/test_llm_proxy.py +++ b/runner/tests/api/test_llm_proxy.py @@ -21,7 +21,9 @@ from pydantic import SecretStr from coval_bench.llm.phonely import PhonelyClient -from tests.api.conftest import LLM_PROXY_KEY, _make_db_url +from coval_bench.registries.benchmarks import Benchmark +from coval_bench.registries.models import RegisteredModel +from tests.api.conftest import LLM_PROXY_KEY, _make_db_url, add_models AUTH = {"Authorization": f"Bearer {LLM_PROXY_KEY}"} Handler = Callable[[httpx.Request], httpx.Response] @@ -48,6 +50,23 @@ def _sse(*deltas: dict[str, Any]) -> bytes: return "\n\n".join(lines).encode() +def _llm_model(provider: str, *, collected: bool) -> RegisteredModel: + return RegisteredModel( + benchmark=Benchmark.LLM, + provider=provider, + model=f"{provider}-agent", + collected=collected, + published=False, + ) + + +@pytest.fixture(autouse=True) +def llm_roster(postgresql: Any) -> None: + add_models( + postgresql, _llm_model("phonely", collected=True), _llm_model("paused", collected=False) + ) + + @pytest_asyncio.fixture async def bind_phonely(app: FastAPI) -> AsyncIterator[Callable[[Handler], None]]: handlers: list[Handler] = [] @@ -88,6 +107,7 @@ async def test_proxy_auth_configuration_and_route_location( ).status_code == 401 assert (await client.post("/v1/llm/phonely/session", json={}, headers=AUTH)).status_code == 404 assert (await client.post("/llm/unknown/session", json={}, headers=AUTH)).status_code == 404 + assert (await client.post("/llm/paused/session", json={}, headers=AUTH)).status_code == 404 app.state.llm_clients = {} assert (await client.post("/llm/phonely/session", json={}, headers=AUTH)).status_code == 503 diff --git a/runner/tests/unit/test_coval_agent_sync.py b/runner/tests/unit/test_coval_agent_sync.py index c5d52178..b0120c58 100644 --- a/runner/tests/unit/test_coval_agent_sync.py +++ b/runner/tests/unit/test_coval_agent_sync.py @@ -23,8 +23,17 @@ sync_llm, ) from coval_bench.platform_assets import SyncError +from coval_bench.registries.benchmarks import Benchmark +from coval_bench.registries.models import RegisteredModel SECRET = "proxy-secret-value" # noqa: S105 +PHONELY = RegisteredModel( + benchmark=Benchmark.LLM, + provider="phonely", + model="phonely-agent", + collected=True, + published=False, +) DEFINITION = CovalTextAgentDefinition( provider="phonely", proxy_url="https://api.example.com", @@ -82,6 +91,10 @@ def handler(request: httpx.Request) -> httpx.Response: return httpx.Response(200, json={"run_template": {**state["run_templates"][0], **body}}) if path == "/scheduled-runs": return httpx.Response(200, json={"scheduled_run": {**body, "id": "S" * 22}}) + if path.startswith("/scheduled-runs/"): + return httpx.Response( + 200, json={"scheduled_run": {**state["scheduled_runs"][0], **body}} + ) return httpx.Response( 400, json={"error": {"code": 400, "status": "INVALID_ARGUMENT", "message": path}} ) @@ -154,6 +167,29 @@ def test_sync_creates_everything_when_absent() -> None: assert template["iteration_count"] == 1 assert "options" not in template assert state["writes"][3][1]["schedule_expression"] == "cron(0 13 * * ? *)" + assert state["writes"][3][1]["enabled"] is True + + +def test_sync_disables_the_schedule_of_an_uncollected_model() -> None: + paused = DEFINITION.model_copy(update={"collected": False}) + state = _state( + agents=[{**DEFINITION.agent_body(), "id": "A"}], + test_set_agents=[{"id": "A"}], + run_templates=[{**DEFINITION.run_template_body("A"), "id": "T"}], + scheduled_runs=[{"id": "S", "run_template_id": "T", "enabled": True}], + ) + with _client(state) as client: + assert sync(client, paused, dry_run=True).actions[3] == "scheduled run: disable" + assert state["writes"] == [] + sync(client, paused) + state["scheduled_runs"][0]["enabled"] = False + assert sync(client, paused).actions[3] == "scheduled run: unchanged" + assert sync(client, DEFINITION).actions[3] == "scheduled run: enable" + + assert state["writes"] == [ + ("/scheduled-runs/S", {"enabled": False}), + ("/scheduled-runs/S", {"enabled": True}), + ] def test_sync_patches_drifted_metadata_wholesale_and_leaves_the_rest() -> None: @@ -163,7 +199,7 @@ def test_sync_patches_drifted_metadata_wholesale_and_leaves_the_rest() -> None: agents=[live], test_set_agents=[{"id": "A"}], run_templates=[{**DEFINITION.run_template_body("A"), "id": "T"}], - scheduled_runs=[{"id": "S", "run_template_id": "T"}], + scheduled_runs=[{"id": "S", "run_template_id": "T", "enabled": True}], ) with _client(state) as client: result = sync(client, DEFINITION) @@ -172,7 +208,7 @@ def test_sync_patches_drifted_metadata_wholesale_and_leaves_the_rest() -> None: "agent: patch ['metadata']", "test set: attached", "run template: unchanged", - "scheduled run: exists", + "scheduled run: unchanged", ] assert state["writes"] == [("/agents/A", {"metadata": DEFINITION.agent_body()["metadata"]})] @@ -188,7 +224,7 @@ def test_sync_patches_only_the_drifted_template_fields() -> None: agents=[{**DEFINITION.agent_body(), "id": "A"}], test_set_agents=[{"id": "A"}], run_templates=[live_template], - scheduled_runs=[{"id": "S", "run_template_id": "T"}], + scheduled_runs=[{"id": "S", "run_template_id": "T", "enabled": True}], ) with _client(state) as client: assert ( @@ -252,6 +288,7 @@ def test_cli_prints_the_agent_id_and_never_the_secret(monkeypatch: pytest.Monkey ), ) monkeypatch.setattr(coval_agent, "CovalTextClient", lambda *_args: _client(state)) + monkeypatch.setattr(coval_agent, "load_llm_models", lambda _settings: [PHONELY]) dry = CliRunner().invoke(sync_llm, ["--dry-run"]) assert dry.exit_code == 0, dry.output @@ -272,7 +309,7 @@ def test_cli_syncs_completed_runs_into_the_database(monkeypatch: pytest.MonkeyPa agents=[{**DEFINITION.agent_body(), "id": agent_id}], test_set_agents=[{"id": agent_id}], run_templates=[{"id": template_id, "display_name": "benchmarks-phonely-text-daily"}], - scheduled_runs=[{"id": "S" * 22, "run_template_id": template_id}], + scheduled_runs=[{"id": "S" * 22, "run_template_id": template_id, "enabled": True}], ) settings = Settings( llm_proxy_public_url="https://api.example.com", @@ -284,6 +321,7 @@ def test_cli_syncs_completed_runs_into_the_database(monkeypatch: pytest.MonkeyPa monkeypatch.setenv("COVAL_API_KEY", "coval-key") monkeypatch.setattr(coval_agent, "get_settings", lambda: settings) monkeypatch.setattr(coval_agent, "CovalTextClient", lambda *_args: _client(state)) + monkeypatch.setattr(coval_agent, "load_llm_models", lambda _settings: [PHONELY]) monkeypatch.setattr("coval_bench.s2s.fetch_v2v._run_fetch", fetch) applied = CliRunner().invoke(sync_llm, []) @@ -292,9 +330,19 @@ def test_cli_syncs_completed_runs_into_the_database(monkeypatch: pytest.MonkeyPa fetch.assert_called_once() assert fetch.call_args.kwargs["settings"].coval_llm_phonely_agent_id == agent_id + paused = PHONELY.model_copy(update={"collected": False}) + monkeypatch.setattr(coval_agent, "load_llm_models", lambda _settings: [paused]) + fetch.reset_mock() + applied = CliRunner().invoke(sync_llm, []) + + assert applied.exit_code == 0, applied.output + assert "phonely scheduled run: disable" in applied.output + fetch.assert_not_called() + def test_cli_logs_automation_failures(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.setattr(coval_agent, "get_settings", Settings) + monkeypatch.setattr(coval_agent, "load_llm_models", lambda _settings: [PHONELY]) failed = CliRunner().invoke(sync_llm, []) diff --git a/runner/tests/unit/test_s2s_fetch.py b/runner/tests/unit/test_s2s_fetch.py index 4844de9d..17f7f0e6 100644 --- a/runner/tests/unit/test_s2s_fetch.py +++ b/runner/tests/unit/test_s2s_fetch.py @@ -22,6 +22,7 @@ from coval_bench.db.models import MetricExecutor, ResultStatus, Run, RunStatus from coval_bench.logging import log_run_failed, log_run_partial from coval_bench.registries import Benchmark, Metric +from coval_bench.registries.models import RegisteredModel from coval_bench.s2s import fetch_v2v from coval_bench.s2s.conditions import ( DATASET_ID_LLM_DENTAL, @@ -41,6 +42,13 @@ ALL_IDS = {**IDS, Metric.INTERRUPTION_RATE: "RID"} SPEC = AgentSpec(agent_id_attr="coval_s2s_openai_agent_id", provider="openai", model="gpt-realtime") +PHONELY = RegisteredModel( + benchmark=Benchmark.LLM, + provider="phonely", + model="phonely-agent", + collected=True, + published=False, +) LLM_SPEC = AgentSpec( agent_id_attr="coval_s2s_openai_agent_id", provider="phonely", @@ -775,7 +783,8 @@ async def test_fetch_and_write_filters_agents_and_allows_llm_without_v2v( ) -> None: settings = Settings( coval_s2s_instruction_metric_id="IID", - coval_s2s_openai_agent_id="llm-agent", + coval_s2s_openai_agent_id="s2s-agent", + coval_llm_phonely_agent_id="llm-agent", coval_s2s_dental_test_set_id="TSD", ) client = _fake_client({}, {}) @@ -786,7 +795,15 @@ async def _fake_pool(_settings: Any) -> AsyncIterator[MagicMock]: yield MagicMock() fetch_one = AsyncMock(return_value=(RunStatus.SUCCEEDED, 0)) - monkeypatch.setattr(fetch_v2v, "AGENTS", (SPEC, LLM_SPEC)) + paused = RegisteredModel( + benchmark=Benchmark.LLM, + provider="paused", + model="paused-agent", + collected=False, + published=False, + ) + monkeypatch.setattr(fetch_v2v, "AGENTS", (SPEC,)) + monkeypatch.setattr(fetch_v2v, "fetch_models", AsyncMock(return_value=[PHONELY, paused])) monkeypatch.setattr(fetch_v2v, "_client", lambda _settings: client) monkeypatch.setattr(fetch_v2v, "lifespan_pool", _fake_pool) monkeypatch.setattr(fetch_v2v, "RunWriter", lambda _pool: writer) @@ -797,12 +814,12 @@ async def _fake_pool(_settings: Any) -> AsyncIterator[MagicMock]: assert statuses == {"phonely:phonely-agent": RunStatus.SUCCEEDED} fetch_one.assert_awaited_once() assert fetch_one.await_args is not None - assert fetch_one.await_args.kwargs["spec"] is LLM_SPEC + assert fetch_one.await_args.kwargs["spec"] == fetch_v2v.llm_specs([PHONELY])[0] assert fetch_one.await_args.kwargs["metric_ids"] == {Metric.INSTRUCTION_FOLLOWING: "IID"} def test_phonely_spec_is_the_llm_dental_text_agent() -> None: - spec = next(spec for spec in fetch_v2v.AGENTS if spec.provider == "phonely") + (spec,) = fetch_v2v.llm_specs([PHONELY, PHONELY.model_copy(update={"collected": False})]) assert spec == AgentSpec( agent_id_attr="coval_llm_phonely_agent_id", provider="phonely",