diff --git a/README.md b/README.md index 0a2e7ff..debd40c 100644 --- a/README.md +++ b/README.md @@ -139,6 +139,22 @@ Override the default model, highest precedence first: an explicit argument, > OpenAI-compatible endpoint, local models — is a change to that resolution layer and the > config schema rather than a change to how evaluators, datasets, or runs work. +## Setup + +`valcore serve` shows a setup card on the Overview page listing each key valcore knows +about, whether it is currently set, and what it unlocks. Keys are never entered through +the web UI — no secret crosses HTTP — and are always set from the CLI: + +```bash +valcore config set-key # required: runs and generation +valcore config set-logfire-token # optional: sends traces to Logfire +valcore config set-logfire-key # optional: pushes datasets to Logfire +``` + +Without the gateway key, generation and runs are unavailable, and the UI shows why. Manual +authoring, dataset upload, editing, hand-labeling, and every export still work with no key +configured at all. + ## Commands | Command | What it does | @@ -146,12 +162,16 @@ Override the default model, highest precedence first: an explicit argument, | `valcore serve` | Serve the web UI and API (`--port`, `--host`, `--no-browser`). | | `valcore list ` | List resources as a table or, with `--json`, as JSON. | | `valcore run ` | Run an evaluator version over a dataset. | +| `valcore experiment ` | Run an evaluator version over a dataset via `pydantic_evals.Dataset.evaluate`. | | `valcore export ` | Export an evaluator (and, with `--dataset`, a dataset) as a Python script or, with `--format json`, a portable eval package. | | `valcore import ` | Import a JSON eval package back into the local database. | | `valcore config set-key [KEY]` | Store the gateway API key in the config file. | +| `valcore config set-logfire-token [TOKEN]` | Store the Logfire write token in the config file. | +| `valcore config set-logfire-key [KEY]` | Store the Logfire API key in the config file. | | `valcore config get` | Show the current config (the key is masked unless `--show-key`). | | `valcore config path` | Print the path to the config file. | | `valcore config edit` | Open the config file in `$EDITOR`. | +| `valcore logfire push ` | Push a dataset to Logfire's hosted dataset store. | | `valcore skills install` | Install the bundled agent skills (`--claude`, `--copilot`, …). | | `valcore skills list` | Show the bundled skills and where each is installed. | | `valcore skills uninstall` | Remove the bundled skills from the selected directories. | @@ -281,6 +301,32 @@ Exit codes: `--min-accuracy` requires a categorical accuracy metric; numeric or unlabeled runs have no accuracy and error rather than silently passing. +## Logfire + +`logfire` is an optional extra — install it to get traces: + +```bash +uv tool install 'valcore[logfire]' +``` + +With a Logfire token configured (see [Setup](#setup)), each `valcore run` opens a +`valcore.run` span carrying the run's evaluator version, dataset, and concurrency, with a +`valcore.score_row` child span per row; on close, the run span records its status and each +agreement metric as attributes, so a Logfire query can filter runs by accuracy directly. +The [Pydantic AI Gateway](https://ai.pydantic.dev/gateway/) already reports the LLM calls +themselves — valcore adds only the surrounding run and row context around them, and +deliberately does not re-report the calls, which would double-count tokens and cost. + +`valcore experiment ` runs the same evaluation through +`pydantic_evals.Dataset.evaluate` instead of `run`'s own engine, so it also appears in +Logfire's experiments view. It persists a run the same way `run` does, so it shows up on +the Runs page too. Unlike `run`, it cannot be cancelled, because `Dataset.evaluate` has no +cancellation. + +`valcore logfire push ` publishes a dataset to Logfire's hosted dataset store. +It needs a Logfire API key (see [Setup](#setup)) with the `project:read_datasets` and +`project:write_datasets` scopes. + ## `~/.valcore` All state lives under `~/.valcore` (mode `0700`). Set `VALCORE_HOME` to relocate it. diff --git a/pyproject.toml b/pyproject.toml index 9aaed14..df0ffe8 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,6 +32,11 @@ dependencies = [ [project.scripts] valcore = "valcore.cli:main" +[project.optional-dependencies] +# The `fastapi` sub-extra brings in opentelemetry-instrumentation-fastapi, required by +# `logfire.instrument_fastapi` in valcore.api.main.create_app. +logfire = ["logfire[fastapi]>=4.39,<5"] + [dependency-groups] dev = [ "pytest", @@ -43,6 +48,10 @@ dev = [ # unpinned bump would change lint results in CI and locally at once. "ruff>=0.16,<0.17", "pre-commit>=4,<5", + # Not a runtime dependency — the `logfire` extra. Declared here so the span tests in + # tests/test_tracing.py and tests/test_runner.py run rather than skipping themselves, and + # so create_app's logfire.instrument_fastapi call has opentelemetry-instrumentation-fastapi. + "logfire[fastapi]>=4.39,<5", ] [build-system] diff --git a/src/valcore/api/main.py b/src/valcore/api/main.py index 8afd3ea..d4ebd1b 100644 --- a/src/valcore/api/main.py +++ b/src/valcore/api/main.py @@ -4,12 +4,14 @@ from importlib.resources import files as _package_files from pathlib import Path +import logfire_api as logfire from fastapi import FastAPI, Request from fastapi.middleware.cors import CORSMiddleware from fastapi.responses import JSONResponse from fastapi.staticfiles import StaticFiles from valcore.api.dtos import ErrorBody, ErrorResponse +from valcore.config import load_config from valcore.errors import ( ConfigError, ContractError, @@ -22,6 +24,7 @@ from valcore.models import VALID_CAPABILITIES from valcore.settings import MODEL_CATALOG from valcore.tools import tool_names +from valcore.tracing import configure_tracing _STATUS_BY_ERROR: tuple[tuple[type[ValcoreError], int], ...] = ( (NotFoundError, 404), @@ -78,7 +81,7 @@ async def handler(_request: Request, exc: ValcoreError) -> JSONResponse: def _include_routers(app: FastAPI) -> None: """Discover and mount resource routers, tolerating ones that do not exist yet.""" - for module_name in ("evaluators", "datasets", "runs", "overview"): + for module_name in ("evaluators", "datasets", "runs", "overview", "setup"): try: module = importlib.import_module(f"valcore.api.routes.{module_name}") except ImportError: @@ -100,6 +103,9 @@ def create_app() -> FastAPI: _register_exception_handlers(app) + configure_tracing(load_config()) + logfire.instrument_fastapi(app) + @app.get("/api/health") async def health() -> dict[str, str]: """Liveness probe.""" diff --git a/src/valcore/api/routes/datasets.py b/src/valcore/api/routes/datasets.py index deafdde..e608291 100644 --- a/src/valcore/api/routes/datasets.py +++ b/src/valcore/api/routes/datasets.py @@ -5,16 +5,18 @@ import json import re from datetime import datetime -from typing import Annotated +from typing import Annotated, Literal from fastapi import APIRouter, Depends, File, Form, UploadFile from pydantic import BaseModel, ConfigDict +from valcore import config from valcore.api.deps import get_store from valcore.config_io import EvalPackage from valcore.datagen import generate_rows from valcore.errors import ContractError from valcore.export import render_dataset_module, render_judge_module +from valcore.logfire_io import push_dataset from valcore.models import LabelSchema, LabelSource from valcore.schema_migration import label_matches_schema from valcore.seeding import dataset_shape_from_version @@ -99,6 +101,14 @@ class RowsAppend(BaseModel): rows: list[dict] +class LogfirePushRequest(BaseModel): + """Request body to push a dataset to Logfire's hosted dataset store.""" + + name: str | None = None + description: str | None = None + on_conflict: Literal["update", "error"] = "update" + + class RowPatch(BaseModel): """Request body to relabel or annotate a single row.""" @@ -420,6 +430,7 @@ def _check_column_notes(column_notes: dict[str, str] | None, columns: list[str]) @router.post("/generate") async def generate_dataset(body: DatasetGenerate, store: StoreDep) -> DatasetCreatedOut: """Generate a dataset and its rows with suggested labels.""" + config.require_gateway_key() if body.count > _MAX_GENERATE_COUNT: raise ContractError(f"count may not exceed {_MAX_GENERATE_COUNT}.") @@ -467,6 +478,7 @@ async def generate_dataset_from_version( body: DatasetGenerateFromVersion, store: StoreDep ) -> DatasetCreatedOut: """Generate a dataset shaped by an evaluator version, runnable against it by construction.""" + config.require_gateway_key() if body.count > _MAX_GENERATE_COUNT: raise ContractError(f"count may not exceed {_MAX_GENERATE_COUNT}.") @@ -545,6 +557,7 @@ async def generate_more_rows(id: str, body: RowsGenerate, store: StoreDep) -> li the new rows stay compatible with the existing ones and with any evaluator that already runs against them. """ + config.require_gateway_key() if body.count > _MAX_GENERATE_COUNT: raise ContractError(f"count may not exceed {_MAX_GENERATE_COUNT}.") @@ -731,3 +744,17 @@ async def export_dataset_json( agent_filename = f"{stem}.agent.json" if split else f"{stem}.json" files["valcore_judge.py"] = render_judge_module(version, agent_filename) return ExportFilesResponse(files=files) + + +@router.post("/{id}/logfire/push") +async def push_dataset_to_logfire(id: str, body: LogfirePushRequest, store: StoreDep) -> dict: + """Push a dataset and its rows to Logfire's hosted dataset store.""" + dataset = store.get_dataset(id) + rows = store.list_rows(id) + return await push_dataset( + dataset, + rows, + name=body.name, + description=body.description, + on_conflict=body.on_conflict, + ) diff --git a/src/valcore/api/routes/evaluators.py b/src/valcore/api/routes/evaluators.py index f0c4cfb..e740724 100644 --- a/src/valcore/api/routes/evaluators.py +++ b/src/valcore/api/routes/evaluators.py @@ -7,7 +7,7 @@ from fastapi import APIRouter, Depends from pydantic import BaseModel, ConfigDict -from valcore import generator +from valcore import config, generator from valcore.api.deps import get_store from valcore.config_io import EvalPackage from valcore.errors import ContractError, FrozenVersionError @@ -405,6 +405,7 @@ async def generate(body: GenerateRequest, store: StoreDep) -> GeneratedConfig: Calls an LLM and can take tens of seconds; the UI presents the returned config as an editable draft saved as a version separately. """ + config.require_gateway_key() columns, label_schema = _resolve_seed(body, store) return await generator.generate_config( body.criteria, @@ -422,6 +423,7 @@ async def generate_version(id: str, body: GenerateRequest, store: StoreDep) -> G of seconds; the UI presents the returned config as an editable draft that the user saves as a version separately. """ + config.require_gateway_key() store.get_evaluator(id) columns, label_schema = _resolve_seed(body, store) return await generator.generate_config( @@ -439,4 +441,5 @@ async def refine_version(body: RefineRequest) -> RefinedConfig: Calls an LLM and can take tens of seconds. The response includes ``changed_fields`` for the diff view; nothing is saved until the user submits a new version. """ + config.require_gateway_key() return await generator.refine_config(body.config, body.instruction) diff --git a/src/valcore/api/routes/runs.py b/src/valcore/api/routes/runs.py index 961e329..721c923 100644 --- a/src/valcore/api/routes/runs.py +++ b/src/valcore/api/routes/runs.py @@ -16,6 +16,7 @@ from pydantic_ai import Agent from sse_starlette.sse import EventSourceResponse +from valcore import config from valcore.api.deps import get_store from valcore.api.events import bus from valcore.errors import ContractError @@ -149,12 +150,17 @@ async def _run_to_completion( Any exception escaping the runner is recorded on the run as ``FAILED`` so a run is never left stuck in ``RUNNING``. The bus is always closed so subscribers stop. + + Guards on ``config.require_gateway_key()`` before ever building an agent or calling + ``execute_run``: ``defer_model_check=True`` lets ``build_agent`` succeed with no key, + so without this a keyless run would instead fail once per row inside ``_score_row``. """ async def on_event(event: RunEvent) -> None: bus.publish(run_id, {"type": event.type, "run_id": run_id, "payload": event.payload}) try: + config.require_gateway_key() agent: Agent | None = None if agent_factory is not None: run = await asyncio.to_thread(store.get_run, run_id) @@ -231,7 +237,12 @@ async def create_run(body: RunCreate, store: StoreDep, agent_factory: AgentFacto """Create a run (freezing its version) and launch it in the background. Returns immediately with status ``PENDING``; the run is never awaited in the handler. + + Guards on ``config.require_gateway_key()`` before ``store.create_run``: a missing key + must surface synchronously as a ``ConfigError``, with no run ever persisted, rather than + as an asynchronous ``FAILED`` transition discovered by polling. """ + config.require_gateway_key() run = store.create_run( kind=body.kind, version_id=body.version_id, @@ -397,7 +408,12 @@ async def retry_failed(id: str, store: StoreDep, agent_factory: AgentFactoryDep) The run is reset to ``PENDING`` before relaunching so a client polling status is never fooled by the previous run's stale terminal state. + + Guards on ``config.require_gateway_key()`` before any other work: a missing key + must surface synchronously as a ``ConfigError``, with the prior run's status and + results untouched and no background task launched. """ + config.require_gateway_key() store.get_run(id) failed_row_ids = store.failed_result_row_ids(id) run = store.update_run_status(id, RunStatus.PENDING, error=None, finished_at=None) diff --git a/src/valcore/api/routes/setup.py b/src/valcore/api/routes/setup.py new file mode 100644 index 0000000..4ff1725 --- /dev/null +++ b/src/valcore/api/routes/setup.py @@ -0,0 +1,70 @@ +"""Read-only setup status: which configuration keys are effectively set. + +Keys are written only via the CLI (``valcore config set-key`` and its Logfire siblings), so +there is no POST here and no key value ever crosses this endpoint -- only booleans, computed +from the same ``*_present`` helpers ``require_gateway_key`` relies on, so presence reflects an +exported env var exactly as it does everywhere else in the codebase. +""" + +from fastapi import APIRouter +from pydantic import BaseModel + +from valcore.config import ( + gateway_key_present, + load_config, + logfire_api_key_present, + logfire_token_present, +) + +router = APIRouter(prefix="/api/setup", tags=["setup"]) + + +class KeyStatus(BaseModel): + """Presence and static metadata for one configuration key. Never carries its value.""" + + name: str + set: bool + required: bool + label: str + command: str + purpose: str + + +class SetupOut(BaseModel): + """The full setup status: one entry per documented configuration key.""" + + keys: list[KeyStatus] + + +@router.get("", response_model=SetupOut) +async def get_setup() -> SetupOut: + """Report effective presence for the gateway key and both Logfire credentials.""" + cfg = load_config() + return SetupOut( + keys=[ + KeyStatus( + name="gateway_api_key", + set=gateway_key_present(cfg), + required=True, + label="Pydantic AI Gateway key", + command="valcore config set-key", + purpose="Runs evaluators and generates evaluators and datasets.", + ), + KeyStatus( + name="logfire_token", + set=logfire_token_present(cfg), + required=False, + label="Logfire write token", + command="valcore config set-logfire-token", + purpose="Sends run traces to Logfire.", + ), + KeyStatus( + name="logfire_api_key", + set=logfire_api_key_present(cfg), + required=False, + label="Logfire API key", + command="valcore config set-logfire-key", + purpose="Pushes datasets to Logfire's hosted store.", + ), + ] + ) diff --git a/src/valcore/cli/main.py b/src/valcore/cli/main.py index 99e1c8f..696756a 100644 --- a/src/valcore/cli/main.py +++ b/src/valcore/cli/main.py @@ -18,6 +18,8 @@ import click +from valcore import config as config_module +from valcore import experiment, logfire_io, tracing from valcore.cli.output import emit from valcore.cli.resolve import resolve_dataset, resolve_evaluator, resolve_version from valcore.cli.skills import skills @@ -92,6 +94,7 @@ def _store(ctx: click.Context) -> Store: def cli(ctx: click.Context, db: Path | None) -> None: """Develop, run, and export agentic evaluations from the command line.""" apply_gateway_key(load_config()) + tracing.configure_tracing(load_config()) db_path = db if db is not None else get_settings().db_path if _LOCAL_DB.exists() and not Path(db_path).exists(): @@ -422,6 +425,7 @@ def run( min_accuracy: float | None, ) -> None: """Run an evaluator version over a dataset.""" + config_module.require_gateway_key() store = _store(ctx) ev = resolve_evaluator(store, evaluator) ver = resolve_version(store, ev, version_name) @@ -451,6 +455,67 @@ def run( sys.exit(2) +# -- experiment ----------------------------------------------------------------- + + +async def _drive_experiment(store: Store, run_id: str) -> Run: + """Drive ``experiment.execute_experiment``, streaming progress to stderr as it goes. + + Mirrors ``_drive_run``'s non-``--watch`` progress line; there is no ``--watch`` mode + here because ``Dataset.evaluate`` has no cancellation to make one meaningful. + """ + state = {"done": 0, "total": 0} + + async def on_event(event: RunEvent) -> None: + if event.type == "started": + state["total"] = event.payload["total"] + elif event.type == "row": + state["done"] += 1 + click.echo(f"\r{state['done']}/{state['total']}", nl=False, err=True) + + run = await experiment.execute_experiment(store, run_id, on_event=on_event) + if state["total"]: + click.echo("", err=True) + return run + + +@cli.command(name="experiment") +@click.argument("evaluator") +@click.argument("dataset") +@click.option("--version", "version_name", default=None, help="Version name (default: active).") +@click.option("--concurrency", type=int, default=None, help="Max concurrent rows.") +@click.option("--json", "as_json", is_flag=True, help="Emit JSON results to stdout.") +@click.pass_context +def experiment_cmd( + ctx: click.Context, + evaluator: str, + dataset: str, + version_name: str | None, + concurrency: int | None, + as_json: bool, +) -> None: + """Run an evaluator version over a dataset via ``pydantic_evals.Dataset.evaluate``. + + A second engine over the same data as ``run``, interchangeable with it at the CLI + level. It has no ``--watch`` and no cancellation, because ``Dataset.evaluate`` offers + neither. + """ + config_module.require_gateway_key() + store = _store(ctx) + ev = resolve_evaluator(store, evaluator) + ver = resolve_version(store, ev, version_name) + ds = resolve_dataset(store, dataset) + + workers = concurrency if concurrency is not None else get_settings().default_concurrency + created = store.create_run(RunKind.VALIDATION, ver.id, ds.id, workers) + + finished = asyncio.run(_drive_experiment(store, created.id)) + if finished.status is RunStatus.FAILED: + raise ValcoreError(finished.error or "Experiment failed.") + + _emit_run(store, finished, as_json) + + # -- config ------------------------------------------------------------------- @@ -473,11 +538,18 @@ def config_set_key(key: str | None) -> None: @click.option("--show-key", is_flag=True, help="Reveal the full gateway API key.") @click.option("--json", "as_json", is_flag=True, help="Emit JSON instead of a table.") def config_get(show_key: bool, as_json: bool) -> None: - """Show the current config, masking the gateway key by default.""" + """Show the current config, masking the gateway key by default. + + The Logfire token and API key are never revealed, even with ``--show-key``: that flag + already governs revealing the gateway key specifically, and does not newly govern these. + Only their presence is shown. + """ cfg = load_config() data = cfg.model_dump(mode="json") - if cfg.gateway_api_key and not show_key: - data["gateway_api_key"] = f"sk-…{cfg.gateway_api_key[-4:]}" + if config_module.gateway_key_present(cfg) and not show_key: + data["gateway_api_key"] = f"sk-…{cfg.gateway_api_key[-4:]}" if cfg.gateway_api_key else True + data["logfire_token"] = config_module.logfire_token_present(cfg) + data["logfire_api_key"] = config_module.logfire_api_key_present(cfg) emit(data, as_json, columns=list(data.keys())) @@ -496,6 +568,70 @@ def config_edit() -> None: click.edit(filename=str(path)) +@config.command("set-logfire-token") +@click.argument("token", required=False) +def config_set_logfire_token(token: str | None) -> None: + """Store the Logfire write token (for tracing) in the config file.""" + if token is None: + token = click.prompt("Logfire token", hide_input=True) + config_module.set_logfire_token(token) + click.echo(f"Saved Logfire token to {config_path()}", err=True) + + +@config.command("set-logfire-key") +@click.argument("key", required=False) +def config_set_logfire_key(key: str | None) -> None: + """Store the Logfire API key (for the hosted datasets API) in the config file.""" + if key is None: + key = click.prompt("Logfire API key", hide_input=True) + config_module.set_logfire_api_key(key) + click.echo(f"Saved Logfire API key to {config_path()}", err=True) + + +# -- logfire -------------------------------------------------------------------- + + +@cli.group(name="logfire") +def logfire_group() -> None: + """Interact with Logfire's hosted dataset store.""" + + +@logfire_group.command("push") +@click.argument("dataset") +@click.option( + "--name", "name", default=None, help="Name for the pushed dataset (default: its own)." +) +@click.option( + "--description", "description", default=None, help="Description for the pushed dataset." +) +@click.option( + "--on-conflict", + "on_conflict", + type=click.Choice(["update", "error"]), + default="update", + help="How to handle a case ID that already exists on the hosted dataset.", +) +@click.pass_context +def logfire_push( + ctx: click.Context, + dataset: str, + name: str | None, + description: str | None, + on_conflict: str, +) -> None: + """Push a dataset to Logfire's hosted dataset store.""" + store = _store(ctx) + ds = resolve_dataset(store, dataset) + rows = store.list_rows(ds.id) + + result = asyncio.run( + logfire_io.push_dataset( + ds, rows, name=name, description=description, on_conflict=on_conflict + ) + ) + emit(result, as_json=False, columns=["id", "name", "case_count"]) + + def main() -> None: """Console-script entry point.""" cli() diff --git a/src/valcore/config.py b/src/valcore/config.py index bad4c9b..0e9e519 100644 --- a/src/valcore/config.py +++ b/src/valcore/config.py @@ -1,9 +1,12 @@ """TOML config layer stored at ``~/.valcore/config.toml``. -Read with the stdlib :mod:`tomllib`; written by hand (five keys does not justify -a TOML-writing dependency). ``apply_gateway_key`` is the single bridge between the -stored config and pydantic-ai: nothing else in the codebase reads, stores, or -passes ``PYDANTIC_AI_GATEWAY_API_KEY``. +Read with the stdlib :mod:`tomllib`; written by hand (seven keys does not justify +a TOML-writing dependency). ``apply_gateway_key`` and ``apply_logfire_token`` are +the only bridges between the stored config and the environment variables that +pydantic-ai and logfire read; nothing else in the codebase reads, stores, or +passes ``PYDANTIC_AI_GATEWAY_API_KEY`` or ``LOGFIRE_TOKEN``. The Logfire API key +has no env var and is never exported; it is read directly from ``FileConfig`` by +whatever calls the datasets API. """ import os @@ -14,9 +17,11 @@ from pydantic import BaseModel +from valcore.errors import ConfigError from valcore.paths import config_path _GATEWAY_KEY_ENV = "PYDANTIC_AI_GATEWAY_API_KEY" +_LOGFIRE_TOKEN_ENV = "LOGFIRE_TOKEN" class FileConfig(BaseModel): @@ -27,6 +32,8 @@ class FileConfig(BaseModel): port: int | None = None concurrency: int | None = None db_path: Path | None = None + logfire_token: str | None = None + logfire_api_key: str | None = None def _toml_str(value: str) -> str: @@ -48,6 +55,10 @@ def _dump_toml(cfg: FileConfig) -> str: lines.append(f"concurrency = {cfg.concurrency}") if cfg.db_path is not None: lines.append(f"db_path = {_toml_str(str(cfg.db_path))}") + if cfg.logfire_token is not None: + lines.append(f"logfire_token = {_toml_str(cfg.logfire_token)}") + if cfg.logfire_api_key is not None: + lines.append(f"logfire_api_key = {_toml_str(cfg.logfire_api_key)}") return "\n".join(lines) + ("\n" if lines else "") @@ -113,3 +124,60 @@ def apply_gateway_key(cfg: FileConfig) -> bool: return False os.environ[_GATEWAY_KEY_ENV] = cfg.gateway_api_key return True + + +def set_logfire_token(token: str) -> None: + """Persist ``token`` as the Logfire write token, preserving other config values.""" + cfg = load_config() + cfg.logfire_token = token + save_config(cfg) + + +def set_logfire_api_key(key: str) -> None: + """Persist ``key`` as the Logfire management API key, preserving other config values.""" + cfg = load_config() + cfg.logfire_api_key = key + save_config(cfg) + + +def apply_logfire_token(cfg: FileConfig) -> bool: + """Export the stored Logfire token to the environment when it is not already set. + + Returns ``True`` if the environment variable was set from ``cfg``. An + explicitly exported ``LOGFIRE_TOKEN`` always wins. + """ + if cfg.logfire_token is None: + return False + if _LOGFIRE_TOKEN_ENV in os.environ: + return False + os.environ[_LOGFIRE_TOKEN_ENV] = cfg.logfire_token + return True + + +def gateway_key_present(cfg: FileConfig) -> bool: + """Report whether the gateway key is effectively present, from env or ``cfg``.""" + return _GATEWAY_KEY_ENV in os.environ or cfg.gateway_api_key is not None + + +def logfire_token_present(cfg: FileConfig) -> bool: + """Report whether the Logfire token is effectively present, from env or ``cfg``.""" + return _LOGFIRE_TOKEN_ENV in os.environ or cfg.logfire_token is not None + + +def logfire_api_key_present(cfg: FileConfig) -> bool: + """Report whether the Logfire API key is present. File-only; there is no env var.""" + return cfg.logfire_api_key is not None + + +def require_gateway_key() -> None: + """Raise :class:`ConfigError` unless the gateway key is effectively present. + + A missing gateway key otherwise fails deep inside request handling: the + provider raises a bare ``UserError`` that becomes a 500, and because + ``build_agent`` defers the check, a run instead records one failure per row. + """ + if not gateway_key_present(load_config()): + raise ConfigError( + "No gateway API key configured. Run 'valcore config set-key' or export " + f"{_GATEWAY_KEY_ENV}." + ) diff --git a/src/valcore/experiment.py b/src/valcore/experiment.py new file mode 100644 index 0000000..e5ba7a6 --- /dev/null +++ b/src/valcore/experiment.py @@ -0,0 +1,337 @@ +"""Second execution mode for scoring an evaluator version: ``pydantic_evals.Dataset.evaluate``. + +``runner.execute_run`` stays the primary engine -- it keeps cancellation and row-subset +retry, which ``evaluate()`` cannot express. This module trades those for what +``pydantic_evals`` gives for free: concurrency, retries, and (via ``spec.dataset_to_evals`` +and ``tracing``) a shape Logfire's experiment view can render directly. + +The task/evaluator mapping here is the inverse of the *exported* package: there the +consumer's task is measured by a ``ValcoreJudge`` evaluator; here the judge itself is the +task being measured, and agreement with the human label is the evaluator. Both engines +call ``metrics.compute_metrics`` on the same kind of ``(predicted, label)`` pairs read back +from persisted ``RunResult`` rows, never from ``pydantic_evals``' own evaluator output -- +that is what keeps a run and an experiment over the same data reporting identical numbers. +""" + +import asyncio +from collections.abc import Awaitable, Callable +from contextvars import ContextVar +from dataclasses import dataclass +from datetime import UTC, datetime + +from pydantic_ai import Agent +from pydantic_evals import Case, CaseLifecycle, increment_eval_metric, set_eval_attribute +from pydantic_evals.evaluators import Evaluator, EvaluatorContext +from pydantic_evals.evaluators.common import EqualsExpected +from pydantic_evals.reporting import ReportCase, ReportCaseFailure + +from valcore.errors import ContractError +from valcore.factory import build_agent, extract_score, render_prompt +from valcore.metrics import compute_metrics +from valcore.models import ( + DatasetRow, + EvaluatorVersion, + Run, + RunKind, + RunStatus, + ScoreKind, + check_dataset_compatibility, +) +from valcore.runner import RunEvent, _agreement, _label_value +from valcore.spec import dataset_to_evals +from valcore.store import Store +from valcore.tracing import row_span, run_span + +# Set by ``PersistResults.setup`` and read inside the task so the Gateway's per-call span +# nests under the right ``valcore.score_row`` span. Safe under concurrency: ``evaluate()`` +# schedules each case as its own asyncio task, so each gets an independent copy of the +# context created at that point -- setting this in one case's ``setup`` never leaks into +# another case running at the same time. +_current_row: ContextVar[DatasetRow | None] = ContextVar("_experiment_current_row", default=None) + +_USAGE_KEYS = ("input_tokens", "output_tokens", "total_tokens", "requests") + + +@dataclass(repr=False) +class NumericDelta(Evaluator[object, object, object]): + """Signed delta of the task's output against the expected label. + + ``EqualsExpected`` is exact-match, which is wrong for numeric scores where a "close" + prediction should not fail identically to a wildly off one. Delegates to + ``runner._agreement`` rather than reimplementing it, so this can never drift from what + the runner engine reports for the same pair. + """ + + def evaluate(self, ctx: EvaluatorContext[object, object, object]) -> float: + """Return ``predicted - label``, matching ``runner._agreement``'s numeric branch.""" + return _agreement(ScoreKind.NUMERIC, ctx.output, ctx.expected_output) + + +def _usage_from_metrics(metrics: dict[str, float | int]) -> dict | None: + """Reassemble a usage dict from the per-case metrics the task recorded, if complete.""" + if not all(key in metrics for key in _USAGE_KEYS): + return None + return {key: metrics[key] for key in _USAGE_KEYS} + + +def _make_task(version: EvaluatorVersion, agent: Agent) -> Callable[[dict], Awaitable[str | float]]: + """Build the ``pydantic_evals`` task: the same three calls ``runner._score_row`` makes. + + Returns the score alone -- not the full structured output -- because the agreement + evaluators (``EqualsExpected``/``NumericDelta``) compare ``ctx.output`` directly against + the human label. The full output and token usage are not lost: they are recorded via + ``set_eval_attribute``/``increment_eval_metric`` so ``PersistResults.teardown`` can + still persist them on ``RunResult``. + """ + + async def task(inputs: dict) -> str | float: + row = _current_row.get() + with row_span(row): + prompt = render_prompt(version, inputs) + result = await agent.run(prompt) + output = result.output + score = extract_score(version, output) + set_eval_attribute("output", output.model_dump(mode="json")) + usage = result.usage + increment_eval_metric("input_tokens", usage.input_tokens) + increment_eval_metric("output_tokens", usage.output_tokens) + increment_eval_metric("total_tokens", usage.total_tokens) + increment_eval_metric("requests", usage.requests) + return score + + return task + + +class PersistResults(CaseLifecycle): + """The persistence seam: writes one ``RunResult`` and emits one ``row`` event per case. + + A new instance is created per case by ``execute_experiment``'s lifecycle factory, which + is what supplies the extra context (store, run, version) the bare ``CaseLifecycle`` + contract does not carry. + """ + + def __init__( + self, + case: Case, + *, + store: Store, + run_id: str, + version: EvaluatorVersion, + want_agreement: bool, + rows_by_id: dict[str, DatasetRow], + emit_row: Callable[[str, bool, str | float | None], Awaitable[None]], + ) -> None: + super().__init__(case) + self._store = store + self._run_id = run_id + self._version = version + self._want_agreement = want_agreement + self._emit_row = emit_row + self._row = rows_by_id[case.name] + + async def setup(self) -> None: + """Publish this case's row so the task can open the matching ``valcore.score_row`` span.""" + _current_row.set(self._row) + + async def teardown(self, result: ReportCase | ReportCaseFailure | None) -> None: + """Persist the case's outcome and emit its ``row`` event. + + ``result`` is ``None`` when the evaluation was interrupted before a report object + existed for this case; that is recorded as an errored result rather than raised, so + a single interruption cannot leave a case with no ``RunResult`` at all. + """ + row_id = self._row.id + + if result is None or isinstance(result, ReportCaseFailure): + error = ( + result.error_message + if isinstance(result, ReportCaseFailure) + else "Case was interrupted before it could complete." + ) + await asyncio.to_thread( + self._store.add_result, self._run_id, row_id=row_id, output=None, error=error + ) + await self._emit_row(row_id, False, None) + return + + score = result.output + agreement = None + if self._want_agreement and result.expected_output is not None: + agreement = _agreement(self._version.score_kind, score, result.expected_output) + + await asyncio.to_thread( + self._store.add_result, + self._run_id, + row_id=row_id, + output=result.attributes.get("output"), + score_value=score, + agreement=agreement, + latency_ms=int(result.task_duration * 1000), + usage=_usage_from_metrics(result.metrics), + ) + await self._emit_row(row_id, True, score) + + +async def execute_experiment( + store: Store, + run_id: str, + *, + on_event: Callable[[RunEvent], Awaitable[None]] | None = None, +) -> Run: + """Run an evaluator version over its dataset via ``pydantic_evals.Dataset.evaluate``. + + Mirrors ``runner.execute_run``'s contract -- setup failures (incompatible dataset, + agent build) abort with status ``FAILED`` before any result is written; per-case + failures are recorded and never abort the run. Unlike the runner, there is no + cancellation: ``evaluate()`` has none, so none is polled for here. + """ + + async def emit(kind: str, payload: dict) -> None: + if on_event is not None: + await on_event(RunEvent(type=kind, run_id=run_id, payload=payload)) # type: ignore[arg-type] + + async def emit_row(row_id: str, success: bool, score_value: str | float | None) -> None: + await emit("row", {"row_id": row_id, "success": success, "score_value": score_value}) + + run = await asyncio.to_thread(store.get_run, run_id) + + try: + version = await asyncio.to_thread(store.get_version, run.version_id) + dataset = await asyncio.to_thread(store.get_dataset, run.dataset_id) + check_dataset_compatibility(version, dataset) + rows = await asyncio.to_thread(store.list_rows, dataset.id) + agent = build_agent(version) + except Exception as exc: # noqa: BLE001 — any setup failure becomes a FAILED run + failed = await asyncio.to_thread( + store.update_run_status, + run_id, + RunStatus.FAILED, + error=str(exc), + finished_at=datetime.now(UTC), + ) + await emit("error", {"error": str(exc)}) + return failed + + # The missing-label contract is a caller error, not a FAILED run: it must + # propagate rather than be recorded on the run, matching ``runner.execute_run``. + if run.kind is RunKind.VALIDATION: + unlabeled = sum(1 for row in rows if row.label is None) + if unlabeled: + raise ContractError( + f"Validation run requires every row to carry a label; " + f"{unlabeled} row(s) are unlabeled." + ) + + # Marks this run as experiment-produced before ``RUNNING``/``evaluate()`` even start, + # so ``request_cancel`` fails honestly for it from the moment it becomes active -- + # ``evaluate()`` has no cancellation hook to honor, and a marker written only after + # completion would let a cancel request silently no-op for the whole active run. + await asyncio.to_thread( + store.set_experiment, + run_id, + experiment_name=version.version_name, + case_count=len(rows), + ) + + with run_span(run, version, dataset, len(rows)) as span: + try: + await asyncio.to_thread( + store.update_run_status, run_id, RunStatus.RUNNING, started_at=datetime.now(UTC) + ) + await emit("started", {"total": len(rows)}) + + # An EVAL-kind run has no labels to agree with, mirroring ``runner``'s + # ``want_agreement``. + want_agreement = run.kind is RunKind.VALIDATION + evaluators: list[Evaluator] = [] + if want_agreement: + evaluators.append( + EqualsExpected() + if version.score_kind is ScoreKind.CATEGORICAL + else NumericDelta() + ) + + evals_dataset = dataset_to_evals(dataset, rows, evaluators) + task = _make_task(version, agent) + rows_by_id = {row.id: row for row in rows} + + def lifecycle_factory(case: Case) -> PersistResults: + return PersistResults( + case, + store=store, + run_id=run_id, + version=version, + want_agreement=want_agreement, + rows_by_id=rows_by_id, + emit_row=emit_row, + ) + + report = await evals_dataset.evaluate( + task, + name=version.version_name, + max_concurrency=run.concurrency, + progress=False, + lifecycle=lifecycle_factory, + ) + + # Derived from every persisted result, exactly as ``runner.execute_run`` does, + # so the two engines can never disagree about the run's terminal status or + # metrics. + persisted = await asyncio.to_thread(store.list_results, run_id) + any_error = any(result.error is not None for result in persisted) + status = RunStatus.COMPLETED_WITH_ERRORS if any_error else RunStatus.COMPLETED + + metrics: dict | None = None + if want_agreement: + label_by_row = {row.id: _label_value(row) for row in rows} + pairs = [ + (result.score_value, label_by_row.get(result.row_id)) + for result in persisted + if result.error is None and label_by_row.get(result.row_id) is not None + ] + if pairs: + labels = ( + version.score_labels + if version.score_kind is ScoreKind.CATEGORICAL + else None + ) + metrics = compute_metrics(pairs, version.score_kind, labels) + + finished = await asyncio.to_thread( + store.update_run_status, + run_id, + status, + finished_at=datetime.now(UTC), + metrics=metrics, + ) + # Replaces the initial ``len(rows)`` marker with the actual case count now + # that ``evaluate()`` has finished. + await asyncio.to_thread( + store.set_experiment, + run_id, + experiment_name=version.version_name, + case_count=len(report.cases), + ) + + span.set_attribute("status", status.value) + if metrics is not None: + for key, value in metrics.items(): + span.set_attribute(key, value) + except Exception as exc: # noqa: BLE001 — an unexpected lifecycle/evaluate failure, + # as opposed to an ordinary per-case failure (which ``PersistResults.teardown`` + # already records without raising), must still leave the run terminal rather + # than stuck ``RUNNING`` forever. Handled while the span is still open so + # ``status`` is attached before it closes. + failed = await asyncio.to_thread( + store.update_run_status, + run_id, + RunStatus.FAILED, + error=str(exc), + finished_at=datetime.now(UTC), + ) + span.set_attribute("status", RunStatus.FAILED.value) + await emit("error", {"error": str(exc)}) + return failed + + await emit("finished", {"status": status.value, "metrics": metrics}) + return finished diff --git a/src/valcore/logfire_io.py b/src/valcore/logfire_io.py new file mode 100644 index 0000000..5bcad77 --- /dev/null +++ b/src/valcore/logfire_io.py @@ -0,0 +1,89 @@ +"""Push valcore datasets to Logfire's hosted dataset store. + +The only module that imports ``logfire.experimental``. That path exists only in a full +``logfire`` install (not the always-importable ``logfire_api`` shim) and is documented as +experimental, so the import is deferred to inside :func:`push_dataset` rather than taken at +module scope — importing this module must not require the ``logfire`` extra. + +The upload runs inside an async FastAPI handler, so it uses ``AsyncLogfireAPIClient``: the sync +client would block the event loop for the whole upload. This module never reads or requires the +Logfire write token (``LOGFIRE_TOKEN``) — that credential belongs to tracing, not the datasets +API, which authenticates with a separate API key scoped to ``project:read_datasets`` / +``project:write_datasets``. +""" + +from typing import Literal + +from valcore import config +from valcore.errors import ConfigError, ContractError +from valcore.models import Dataset as VDataset +from valcore.models import DatasetRow +from valcore.spec import dataset_to_evals + +_SET_KEY_COMMAND = "valcore config set-logfire-key" +_REQUIRED_SCOPES = ("project:read_datasets", "project:write_datasets") + + +def _resolve_api_key(api_key: str | None) -> str: + """Resolve the Logfire API key from the argument, falling back to stored config. + + Raises :class:`ConfigError` naming both the CLI command to set the key and the two scopes it + must carry — an under-scoped key otherwise fails with an authorization error that looks like + a tracing misconfiguration rather than a missing datasets scope. + """ + if api_key is not None: + return api_key + stored = config.load_config().logfire_api_key + if stored is not None: + return stored + scopes = " and ".join(_REQUIRED_SCOPES) + raise ConfigError( + f"No Logfire API key configured. Run '{_SET_KEY_COMMAND}' with a key that carries " + f"the {scopes} scopes." + ) + + +async def push_dataset( + dataset: VDataset, + rows: list[DatasetRow], + *, + api_key: str | None = None, + name: str | None = None, + description: str | None = None, + on_conflict: Literal["update", "error"] = "update", +) -> dict: + """Push ``dataset`` and ``rows`` to Logfire's hosted dataset store. + + Uses the async datasets client so the upload does not block the event loop of the FastAPI + handler that calls this. Returns a plain dict built from the returned ``DatasetDetail`` + (a ``TypedDict`` whose only required keys are ``id`` and ``name``); absent optional keys + become ``None`` rather than being omitted, so the shape is stable for callers. There is no + URL field on ``DatasetDetail`` and none is synthesized here. + """ + resolved_key = _resolve_api_key(api_key) + + try: + from logfire.experimental.api_client import AsyncLogfireAPIClient + except ImportError as exc: + raise ConfigError( + "The 'logfire' extra is required to push datasets to Logfire's hosted store." + ) from exc + + evals_dataset = dataset_to_evals(dataset, rows, evaluators=[]) + try: + async with AsyncLogfireAPIClient(api_key=resolved_key) as client: + detail = await client.push_dataset( + evals_dataset, + name=name, + description=description, + on_case_conflict=on_conflict, + ) + except Exception as exc: + raise ContractError(str(exc)) from exc + + return { + "id": str(detail["id"]), + "name": detail["name"], + "case_count": detail.get("case_count"), + "output_schema": detail.get("output_schema"), + } diff --git a/src/valcore/models.py b/src/valcore/models.py index 1132edd..5fe992d 100644 --- a/src/valcore/models.py +++ b/src/valcore/models.py @@ -252,6 +252,22 @@ class Run(SQLModel, table=True): cancel_requested: bool = False +class ExperimentRun(SQLModel, table=True): + """Marks a Run as produced by the pydantic-evals experiment engine rather than the runner. + + A separate table rather than a Run column: ``init_db`` is a bare ``create_all``, which adds + missing tables but never missing columns, so a new field here reaches an existing database + while a new ``Run`` field would not. A run with no row is a runner run, which is correct for + every run that already exists. + """ + + id: str = Field(default_factory=lambda: uuid4().hex, primary_key=True) + created_at: datetime = Field(default_factory=lambda: datetime.now(UTC)) + run_id: str = Field(index=True) + experiment_name: str + case_count: int = 0 + + class RunResult(SQLModel, table=True): """The outcome of scoring one dataset row within a run.""" diff --git a/src/valcore/runner.py b/src/valcore/runner.py index d5d9487..1682f4d 100644 --- a/src/valcore/runner.py +++ b/src/valcore/runner.py @@ -17,6 +17,7 @@ from pydantic_ai.usage import RunUsage from sqlmodel import select +from valcore import tracing from valcore.errors import ContractError from valcore.factory import build_agent, extract_score, render_prompt from valcore.metrics import compute_metrics @@ -141,81 +142,87 @@ async def emit(kind: str, payload: dict) -> None: f"{unlabeled} row(s) are unlabeled." ) - if only_row_ids is not None: - await asyncio.to_thread(_clear_results, store, run_id, list(only_row_ids)) - - await asyncio.to_thread( - store.update_run_status, run_id, RunStatus.RUNNING, started_at=datetime.now(UTC) - ) - await emit("started", {"total": len(rows)}) - - want_agreement = run.kind is RunKind.VALIDATION - semaphore = asyncio.Semaphore(run.concurrency) + with tracing.run_span(run, version, dataset, row_count=len(rows)) as span: + if only_row_ids is not None: + await asyncio.to_thread(_clear_results, store, run_id, list(only_row_ids)) - async def process(row: DatasetRow) -> _Outcome: - try: - outcome = await _score_row(store, run_id, version, built_agent, row, want_agreement) - await emit( - "row", - { - "row_id": row.id, - "success": outcome.success, - "score_value": outcome.predicted, - }, - ) - return outcome - finally: - semaphore.release() - - tasks: list[asyncio.Task[_Outcome]] = [] - cancelled = False - for row in rows: - await semaphore.acquire() - current = await asyncio.to_thread(store.get_run, run_id) - if current.cancel_requested: - semaphore.release() - cancelled = True - break - tasks.append(asyncio.create_task(process(row))) - - await asyncio.gather(*tasks) - - # Derive terminal status and metrics from every persisted result, not just - # this batch's outcomes: a retry via ``only_row_ids`` must summarize the whole - # run, whose store now holds both the replaced rows and the untouched ones. - persisted = await asyncio.to_thread(store.list_results, run_id) - any_error = any(result.error is not None for result in persisted) - - if cancelled: - status = RunStatus.CANCELLED - elif any_error: - status = RunStatus.COMPLETED_WITH_ERRORS - else: - status = RunStatus.COMPLETED - - # A cancelled run's partial agreement would misrepresent the dataset, so - # metrics are computed only for terminal states that ran to completion. - metrics: dict | None = None - if want_agreement and not cancelled: - label_by_row = {row.id: _label_value(row) for row in all_rows} - pairs = [ - (result.score_value, label_by_row.get(result.row_id)) - for result in persisted - if result.error is None and label_by_row.get(result.row_id) is not None - ] - if pairs: - labels = version.score_labels if version.score_kind is ScoreKind.CATEGORICAL else None - metrics = compute_metrics(pairs, version.score_kind, labels) - - finished = await asyncio.to_thread( - store.update_run_status, - run_id, - status, - finished_at=datetime.now(UTC), - metrics=metrics, - ) - await emit("finished", {"status": status.value, "metrics": metrics}) - return finished + await asyncio.to_thread( + store.update_run_status, run_id, RunStatus.RUNNING, started_at=datetime.now(UTC) + ) + await emit("started", {"total": len(rows)}) + + want_agreement = run.kind is RunKind.VALIDATION + semaphore = asyncio.Semaphore(run.concurrency) + + async def process(row: DatasetRow) -> _Outcome: + try: + outcome = await _score_row(store, run_id, version, built_agent, row, want_agreement) + await emit( + "row", + { + "row_id": row.id, + "success": outcome.success, + "score_value": outcome.predicted, + }, + ) + return outcome + finally: + semaphore.release() + + tasks: list[asyncio.Task[_Outcome]] = [] + cancelled = False + for row in rows: + await semaphore.acquire() + current = await asyncio.to_thread(store.get_run, run_id) + if current.cancel_requested: + semaphore.release() + cancelled = True + break + tasks.append(asyncio.create_task(process(row))) + + await asyncio.gather(*tasks) + + # Derive terminal status and metrics from every persisted result, not just + # this batch's outcomes: a retry via ``only_row_ids`` must summarize the whole + # run, whose store now holds both the replaced rows and the untouched ones. + persisted = await asyncio.to_thread(store.list_results, run_id) + any_error = any(result.error is not None for result in persisted) + + if cancelled: + status = RunStatus.CANCELLED + elif any_error: + status = RunStatus.COMPLETED_WITH_ERRORS + else: + status = RunStatus.COMPLETED + + # A cancelled run's partial agreement would misrepresent the dataset, so + # metrics are computed only for terminal states that ran to completion. + metrics: dict | None = None + if want_agreement and not cancelled: + label_by_row = {row.id: _label_value(row) for row in all_rows} + pairs = [ + (result.score_value, label_by_row.get(result.row_id)) + for result in persisted + if result.error is None and label_by_row.get(result.row_id) is not None + ] + if pairs: + labels = ( + version.score_labels if version.score_kind is ScoreKind.CATEGORICAL else None + ) + metrics = compute_metrics(pairs, version.score_kind, labels) + + finished = await asyncio.to_thread( + store.update_run_status, + run_id, + status, + finished_at=datetime.now(UTC), + metrics=metrics, + ) + span.set_attribute("status", status.value) + for key, value in (metrics or {}).items(): + span.set_attribute(key, value) + await emit("finished", {"status": status.value, "metrics": metrics}) + return finished async def _score_row( @@ -227,38 +234,39 @@ async def _score_row( want_agreement: bool, ) -> _Outcome: """Score one row and persist its result; row failures are recorded, not raised.""" - label_value = _label_value(row) if want_agreement else None - start = time.perf_counter() - try: - prompt = render_prompt(version, row.data) - result = await agent.run(prompt) - latency_ms = int((time.perf_counter() - start) * 1000) - output: BaseModel = result.output - score = extract_score(version, output) - agreement = ( - _agreement(version.score_kind, score, label_value) - if want_agreement and label_value is not None - else None - ) - await asyncio.to_thread( - store.add_result, - run_id, - row_id=row.id, - output=output.model_dump(mode="json"), - score_value=score, - agreement=agreement, - latency_ms=latency_ms, - usage=_usage_dict(result.usage), - ) - return _Outcome(row.id, True, score) - except Exception as exc: # noqa: BLE001 — a row failure is recorded, never fatal - latency_ms = int((time.perf_counter() - start) * 1000) - await asyncio.to_thread( - store.add_result, - run_id, - row_id=row.id, - output=None, - error=str(exc), - latency_ms=latency_ms, - ) - return _Outcome(row.id, False, None) + with tracing.row_span(row): + label_value = _label_value(row) if want_agreement else None + start = time.perf_counter() + try: + prompt = render_prompt(version, row.data) + result = await agent.run(prompt) + latency_ms = int((time.perf_counter() - start) * 1000) + output: BaseModel = result.output + score = extract_score(version, output) + agreement = ( + _agreement(version.score_kind, score, label_value) + if want_agreement and label_value is not None + else None + ) + await asyncio.to_thread( + store.add_result, + run_id, + row_id=row.id, + output=output.model_dump(mode="json"), + score_value=score, + agreement=agreement, + latency_ms=latency_ms, + usage=_usage_dict(result.usage), + ) + return _Outcome(row.id, True, score) + except Exception as exc: # noqa: BLE001 — a row failure is recorded, never fatal + latency_ms = int((time.perf_counter() - start) * 1000) + await asyncio.to_thread( + store.add_result, + run_id, + row_id=row.id, + output=None, + error=str(exc), + latency_ms=latency_ms, + ) + return _Outcome(row.id, False, None) diff --git a/src/valcore/spec.py b/src/valcore/spec.py index 0ee95e3..4575403 100644 --- a/src/valcore/spec.py +++ b/src/valcore/spec.py @@ -14,6 +14,8 @@ ``from_file``. """ +from typing import Any, Literal + from pydantic_ai.agent.spec import AgentSpec from pydantic_evals import Dataset as EvalsDataset from pydantic_evals.dataset import Case @@ -222,16 +224,38 @@ def _row_to_case(row: DatasetRow) -> Case: ) +def _output_type(dataset: VDataset) -> Any: + """Derive ``OutputT`` from the dataset's label schema so a hosted push carries a real schema. + + A bare ``object`` infers to ``{}`` in ``TypeAdapter(...).json_schema()``, which is what + ``LogfireAPIClient.push_dataset`` reads to build the hosted expected-output schema. valcore + knows the label space exactly, so it is encoded here rather than left to infer to nothing. + """ + kind = dataset.label_schema.get("kind") + if kind == "categorical": + labels = dataset.label_schema.get("labels") or [] + if labels: + return Literal[tuple(labels)] # type: ignore[valid-type] + return str + if kind == "numeric": + return float + return str + + def dataset_to_evals( dataset: VDataset, rows: list[DatasetRow], evaluators: list[dict] ) -> EvalsDataset: """Map a valcore dataset and its rows onto a ``pydantic_evals.Dataset``. Concrete generics are used deliberately: constructing ``EvalsDataset`` with unparameterized - generics emits a ``UserWarning``. + generics emits a ``UserWarning``. ``OutputT`` is derived from the dataset's label schema + rather than left as ``object`` so a hosted push infers a real expected-output schema. """ cases = [_row_to_case(row) for row in rows] - return EvalsDataset[dict, object, dict](name=dataset.name, cases=cases, evaluators=evaluators) + output_type = _output_type(dataset) + return EvalsDataset[dict[str, Any], output_type, dict[str, Any]]( # type: ignore[valid-type] + name=dataset.name, cases=cases, evaluators=evaluators + ) def _infer_columns(cases: list[Case]) -> list[str]: diff --git a/src/valcore/store.py b/src/valcore/store.py index 4c8a962..6d71b6e 100644 --- a/src/valcore/store.py +++ b/src/valcore/store.py @@ -26,6 +26,7 @@ DatasetRow, Evaluator, EvaluatorVersion, + ExperimentRun, LabelSchema, LabelSource, Run, @@ -605,13 +606,46 @@ def update_run_status(self, id: str, status: RunStatus, **fields: object) -> Run return run def request_cancel(self, id: str) -> Run: - """Flag a run for cancellation.""" + """Flag a run for cancellation. + + Raises ContractError for an experiment-engine run: ``Dataset.evaluate`` has no + cancellation hook, so silently setting the flag would look like it worked while + doing nothing. + """ with session_scope(self.engine) as session: run = _require(session, Run, id) + experiment = session.exec( + select(ExperimentRun).where(ExperimentRun.run_id == id) + ).first() + if experiment is not None: + raise ContractError( + f"Run {id!r} was produced by the experiment engine; experiment runs " + "cannot be cancelled." + ) run.cancel_requested = True session.add(run) return run + def set_experiment(self, run_id: str, **fields: object) -> ExperimentRun: + """Mark ``run_id`` as produced by the experiment engine, replacing any existing row.""" + with session_scope(self.engine) as session: + _require(session, Run, run_id) + existing = session.exec( + select(ExperimentRun).where(ExperimentRun.run_id == run_id) + ).first() + if existing is not None: + session.delete(existing) + session.flush() + experiment = ExperimentRun(run_id=run_id, **fields) + session.add(experiment) + return experiment + + def get_experiment(self, run_id: str) -> ExperimentRun | None: + """Return the experiment-engine marker for ``run_id``, or None for a runner run.""" + with session_scope(self.engine) as session: + _require(session, Run, run_id) + return session.exec(select(ExperimentRun).where(ExperimentRun.run_id == run_id)).first() + def add_result(self, run_id: str, **fields: object) -> RunResult: """Record the outcome of scoring one row within a run.""" with session_scope(self.engine) as session: diff --git a/src/valcore/tracing.py b/src/valcore/tracing.py new file mode 100644 index 0000000..3d6ef6d --- /dev/null +++ b/src/valcore/tracing.py @@ -0,0 +1,129 @@ +"""Logfire span shaping: the only module that talks to Logfire's instrumentation surface. + +The Pydantic AI Gateway already reports every LLM call server-side and injects a +W3C ``traceparent`` header into each request, so its spans nest under whatever +local span is active. valcore's job is not to re-report LLM calls -- it is to +supply the parent context (``valcore.run`` / ``valcore.score_row``) that gives the +Gateway's spans structure. See ``docs/superpowers/specs/2026-08-08-logfire- +integration-design.md`` for the full design. + +``logfire_api`` is a hard dependency via ``pydantic-evals``/``pydantic-graph`` +(reached through ``pydantic-ai``), and it forwards to real ``logfire`` when the +extra is installed and no-ops otherwise. Importing it unconditionally -- never +``try/except ImportError`` -- covers both cases with one line. +""" + +import importlib.util +import warnings +from collections.abc import Iterator +from contextlib import contextmanager +from typing import Any + +import logfire_api as logfire + +from valcore.config import FileConfig, apply_logfire_token, logfire_token_present +from valcore.models import Dataset, DatasetRow, EvaluatorVersion, Run + +_configured = False + + +class _NoOpSpan: + """Stand-in yielded by ``run_span``/``row_span`` when tracing is unconfigured. + + Calling ``logfire.span`` before ``logfire.configure`` runs -- e.g. when the + real ``logfire`` package is installed but a caller never opted in -- emits + ``LogfireNotConfiguredWarning``. Yielding this instead keeps the context + managers genuinely silent no-ops, matching what unconfigured callers expect. + """ + + def set_attribute(self, *args: Any, **kwargs: Any) -> None: + """Discard the attribute; there is no span to attach it to.""" + + +def configure_tracing(cfg: FileConfig) -> None: + """Apply the configured Logfire token to the environment and configure Logfire. + + Idempotent -- safe to call from both the CLI group callback and the FastAPI + lifespan without reconfiguring. ``console=False`` because valcore's CLI + renders its own progress and tables, which Logfire's console exporter would + otherwise interleave with. + + Warns exactly once when a token is configured but the real ``logfire`` + package is absent, since the shim silently discards the token in that case + and a user who configured one expects traces. Attribute checks cannot tell + the shim apart from the real module -- it masquerades as it -- so presence + is detected with ``importlib.util.find_spec``. + + The idempotency flag is set only after every step below succeeds. If + ``logfire.configure`` (or an earlier step) raises, the module must not be + left permanently marked configured -- that would strand it in a state + where spans quietly stay unconfigured forever with no way to retry. + """ + global _configured + if _configured: + return + + apply_logfire_token(cfg) + + if logfire_token_present(cfg) and importlib.util.find_spec("logfire") is None: + warnings.warn( + "A Logfire token is configured but the 'logfire' extra is not installed; " + "traces will not be sent. Install it with: uv tool install 'valcore[logfire]'", + UserWarning, + stacklevel=2, + ) + + logfire.configure( + send_to_logfire="if-token-present", + service_name="valcore", + console=False, + ) + + _configured = True + + +@contextmanager +def run_span( + run: Run, version: EvaluatorVersion, dataset: Dataset, row_count: int +) -> Iterator[Any]: + """Open the ``valcore.run`` span that parents every ``valcore.score_row`` span. + + Yields the span so the caller can set ``status`` and each metrics key as + attributes before it closes -- metrics as attributes rather than a log line + so a Logfire query can filter runs by accuracy without a join. + + A no-op when ``configure_tracing`` has never successfully run, so callers + need no conditionals and unconfigured processes never touch ``logfire.span``. + """ + if not _configured: + yield _NoOpSpan() + return + with logfire.span( + "valcore.run", + run_id=run.id, + kind=run.kind.value, + version_id=version.id, + version_name=version.version_name, + dataset_id=dataset.id, + dataset_name=dataset.name, + row_count=row_count, + concurrency=run.concurrency, + ) as span: + yield span + + +@contextmanager +def row_span(row: DatasetRow) -> Iterator[Any]: + """Open the ``valcore.score_row`` span for a single row. + + The Gateway's own LLM span attaches beneath this automatically via the + injected ``traceparent``; this module emits nothing for the LLM call itself. + + A no-op when ``configure_tracing`` has never successfully run, so callers + need no conditionals and unconfigured processes never touch ``logfire.span``. + """ + if not _configured: + yield _NoOpSpan() + return + with logfire.span("valcore.score_row", row_id=row.id, idx=row.idx) as span: + yield span diff --git a/tests/test_api_datasets.py b/tests/test_api_datasets.py index e3942bc..c919411 100644 --- a/tests/test_api_datasets.py +++ b/tests/test_api_datasets.py @@ -35,6 +35,18 @@ async def client(store: Store) -> AsyncIterator[httpx.AsyncClient]: yield c +@pytest.fixture(autouse=True) +def _gateway_key_present(monkeypatch: pytest.MonkeyPatch) -> None: + """Present a gateway key by default. + + The three generate-rows handlers now guard on ``config.require_gateway_key()`` before + doing any work; without this, every pre-existing generate test below (which monkeypatches + ``generate_rows`` itself) would fail on the guard before its stub ever ran. Tests that + target the guard clear the key explicitly. + """ + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-test-gateway-key") + + # -- Upload ------------------------------------------------------------------ @@ -1852,3 +1864,225 @@ async def test_upload_full_package_imports_dataset_only_no_evaluator( {"question": "q1", "answer": "a1"}, {"question": "q2", "answer": "a2"}, ] + + +# -- Gateway guard: the three generate-rows handlers --------------------------- +# +# Today the gateway provider raises a bare UserError deep inside model plumbing, which +# _register_exception_handlers cannot map, so it becomes a 500. Each generative handler +# must instead call config.require_gateway_key() before doing any other work, so a missing +# key surfaces as the documented ConfigError (422) and generate_rows is never even invoked. + + +@pytest.mark.anyio +async def test_generate_without_gateway_key_is_client_error_not_500( + client: httpx.AsyncClient, monkeypatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + calls: list[dict] = [] + _install_recording_generate(monkeypatch, calls) + + resp = await client.post( + "/api/datasets/generate", + json={ + "name": "gen", + "description": "d", + "columns": ["prompt"], + "label_schema": CATEGORICAL_SCHEMA, + "count": 1, + }, + ) + assert resp.status_code < 500 + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] + + +@pytest.mark.anyio +async def test_generate_from_version_without_gateway_key_is_client_error_not_500( + client: httpx.AsyncClient, store: Store, monkeypatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + version = _make_version(store) + calls: list[dict] = [] + _install_recording_generate(monkeypatch, calls) + + resp = await client.post( + "/api/datasets/generate-from-version", + json={"version_id": version.id, "name": "seeded", "count": 1}, + ) + assert resp.status_code < 500 + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] + + +@pytest.mark.anyio +async def test_generate_rows_without_gateway_key_is_client_error_not_500( + client: httpx.AsyncClient, monkeypatch +) -> None: + calls: list[dict] = [] + _install_recording_generate(monkeypatch, calls) + + created = await client.post( + "/api/datasets/generate", + json={ + "name": "gen", + "description": "d", + "columns": ["prompt"], + "label_schema": CATEGORICAL_SCHEMA, + "count": 1, + }, + ) + ds_id = created.json()["dataset"]["id"] + calls.clear() + + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + resp = await client.post(f"/api/datasets/{ds_id}/generate-rows", json={"count": 1}) + assert resp.status_code < 500 + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] + + +# -- Push a dataset to Logfire's hosted store ---------------------------------- + + +@pytest.mark.anyio +async def test_push_dataset_returns_the_documented_shape( + client: httpx.AsyncClient, store: Store, monkeypatch +) -> None: + ds_id, _ = _seed_rows(store, CATEGORICAL_SCHEMA, [{"data": {"prompt": "p0"}}]) + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + return { + "id": "logfire-dataset-id", + "name": name or dataset.name, + "case_count": len(rows), + "output_schema": {"enum": ["good", "bad"], "type": "string"}, + } + + monkeypatch.setattr("valcore.api.routes.datasets.push_dataset", fake_push_dataset) + + resp = await client.post(f"/api/datasets/{ds_id}/logfire/push", json={"name": "custom-name"}) + assert resp.status_code == 200, resp.text + assert resp.json() == { + "id": "logfire-dataset-id", + "name": "custom-name", + "case_count": 1, + "output_schema": {"enum": ["good", "bad"], "type": "string"}, + } + + +@pytest.mark.anyio +async def test_push_dataset_body_is_fully_optional( + client: httpx.AsyncClient, store: Store, monkeypatch +) -> None: + ds_id, _ = _seed_rows(store, CATEGORICAL_SCHEMA, [{"data": {"prompt": "p0"}}]) + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + return {"id": "x", "name": dataset.name, "case_count": len(rows), "output_schema": None} + + monkeypatch.setattr("valcore.api.routes.datasets.push_dataset", fake_push_dataset) + + resp = await client.post(f"/api/datasets/{ds_id}/logfire/push", json={}) + assert resp.status_code == 200, resp.text + + +@pytest.mark.anyio +async def test_push_dataset_defaults_on_conflict_to_update_when_omitted( + client: httpx.AsyncClient, store: Store, monkeypatch +) -> None: + """An omitted ``on_conflict`` must reach ``push_dataset`` as ``"update"``, not ``None``.""" + ds_id, _ = _seed_rows(store, CATEGORICAL_SCHEMA, [{"data": {"prompt": "p0"}}]) + calls: list[dict] = [] + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + calls.append({"on_conflict": on_conflict}) + return {"id": "x", "name": dataset.name, "case_count": len(rows), "output_schema": None} + + monkeypatch.setattr("valcore.api.routes.datasets.push_dataset", fake_push_dataset) + + resp = await client.post(f"/api/datasets/{ds_id}/logfire/push", json={"name": "n"}) + assert resp.status_code == 200, resp.text + assert calls == [{"on_conflict": "update"}] + + +@pytest.mark.anyio +async def test_push_dataset_forwards_the_resolved_dataset_rows_and_body_fields( + client: httpx.AsyncClient, store: Store, monkeypatch +) -> None: + ds_id, row_ids = _seed_rows( + store, CATEGORICAL_SCHEMA, [{"data": {"prompt": "p0"}}, {"data": {"prompt": "p1"}}] + ) + calls: list[dict] = [] + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + calls.append( + { + "dataset_id": dataset.id, + "row_count": len(rows), + "name": name, + "description": description, + "on_conflict": on_conflict, + } + ) + return { + "id": "x", + "name": name or dataset.name, + "case_count": len(rows), + "output_schema": None, + } + + monkeypatch.setattr("valcore.api.routes.datasets.push_dataset", fake_push_dataset) + + resp = await client.post( + f"/api/datasets/{ds_id}/logfire/push", + json={"name": "custom-name", "description": "custom-desc", "on_conflict": "error"}, + ) + assert resp.status_code == 200, resp.text + assert calls == [ + { + "dataset_id": ds_id, + "row_count": len(row_ids), + "name": "custom-name", + "description": "custom-desc", + "on_conflict": "error", + } + ] + + +@pytest.mark.anyio +async def test_push_dataset_missing_logfire_api_key_is_client_error( + client: httpx.AsyncClient, store: Store +) -> None: + # No stub installed here: the real push_dataset resolves the API key and raises + # ConfigError, which must surface as a 422 client error naming the config command. + ds_id, _ = _seed_rows(store, CATEGORICAL_SCHEMA, [{"data": {"prompt": "p0"}}]) + + resp = await client.post(f"/api/datasets/{ds_id}/logfire/push", json={}) + assert resp.status_code < 500 + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-logfire-key" in error["message"] + + +@pytest.mark.anyio +async def test_push_dataset_unknown_dataset_id_is_404(client: httpx.AsyncClient) -> None: + resp = await client.post("/api/datasets/does-not-exist/logfire/push", json={}) + assert resp.status_code == 404, resp.text diff --git a/tests/test_api_evaluators.py b/tests/test_api_evaluators.py index 9957388..ebfe809 100644 --- a/tests/test_api_evaluators.py +++ b/tests/test_api_evaluators.py @@ -34,6 +34,18 @@ def _client(app) -> httpx.AsyncClient: return httpx.AsyncClient(transport=transport, base_url="http://test") +@pytest.fixture(autouse=True) +def _gateway_key_present(monkeypatch: pytest.MonkeyPatch) -> None: + """Present a gateway key by default. + + generate/refine now guard on ``config.require_gateway_key()`` before doing any work; + without this, every pre-existing generate/refine test below (which monkeypatches the + generator itself) would fail on the guard before its stub ever ran. Tests that target the + guard clear the key explicitly. + """ + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-test-gateway-key") + + def _valid_version_body() -> dict: """Return a config body that passes both Pydantic parsing and store validation.""" return { @@ -984,3 +996,103 @@ async def test_generate_version_column_notes_without_columns_rejected(app, monke assert response.status_code == 422 assert response.json()["error"]["type"] == "ContractError" assert calls == [] + + +# -- Gateway guard: a missing key must be a client error, never a 500 ---------- +# +# Today the gateway provider raises a bare UserError deep inside model plumbing, which +# _register_exception_handlers cannot map, so it becomes a 500. Each generative handler +# must instead call config.require_gateway_key() before doing any other work, so a missing +# key surfaces as the documented ConfigError (422) and the generator is never even invoked. + + +@pytest.mark.anyio +async def test_generate_without_gateway_key_is_client_error_not_500( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + calls: list[dict] = [] + monkeypatch.setattr(generator, "generate_config", _recording_generate(calls)) + + async with _client(app) as client: + response = await client.post("/api/evaluators/generate", json={"criteria": "x"}) + + assert response.status_code < 500 + assert response.status_code == 422, response.text + error = response.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + # The guard fires before generate_config is ever reached. + assert calls == [] + + +@pytest.mark.anyio +async def test_generate_version_without_gateway_key_is_client_error_not_500( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + calls: list[dict] = [] + monkeypatch.setattr(generator, "generate_config", _recording_generate(calls)) + + async with _client(app) as client: + eval_id = (await client.post("/api/evaluators", json={"name": "E"})).json()["id"] + response = await client.post(f"/api/evaluators/{eval_id}/generate", json={"criteria": "x"}) + + assert response.status_code < 500 + assert response.status_code == 422, response.text + error = response.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] + + +@pytest.mark.anyio +async def test_generate_version_without_gateway_key_is_client_error_even_for_unknown_evaluator( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + """The guard runs before ``store.get_evaluator``, so it fires first even for a bad id.""" + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + calls: list[dict] = [] + monkeypatch.setattr(generator, "generate_config", _recording_generate(calls)) + + async with _client(app) as client: + response = await client.post( + "/api/evaluators/does-not-exist/generate", json={"criteria": "x"} + ) + + assert response.status_code < 500 + assert response.status_code == 422, response.text + error = response.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] + + +@pytest.mark.anyio +async def test_refine_without_gateway_key_is_client_error_not_500( + app, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + calls: list[dict] = [] + + async def fake_refine(config: GeneratedConfig, instruction: str) -> RefinedConfig: + calls.append({"config": config, "instruction": instruction}) + return RefinedConfig(config=config, changed_fields=[], summary="unused") + + monkeypatch.setattr(generator, "refine_config", fake_refine) + + async with _client(app) as client: + response = await client.post( + "/api/evaluators/refine", + json={ + "config": _canned_generated().model_dump(mode="json"), + "instruction": "be stricter", + }, + ) + + assert response.status_code < 500 + assert response.status_code == 422, response.text + error = response.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + assert calls == [] diff --git a/tests/test_api_runs.py b/tests/test_api_runs.py index 129181e..18eec40 100644 --- a/tests/test_api_runs.py +++ b/tests/test_api_runs.py @@ -16,7 +16,7 @@ from valcore.api.deps import get_store from valcore.api.main import create_app -from valcore.api.routes.runs import get_agent_factory +from valcore.api.routes.runs import _tasks, get_agent_factory from valcore.factory import build_output_model from valcore.models import LabelSource, RunKind, RunStatus, ScoreKind from valcore.store import Store, create_engine, init_db @@ -61,6 +61,19 @@ def store(tmp_path) -> Store: return Store(engine) +@pytest.fixture(autouse=True) +def _gateway_key_present(monkeypatch: pytest.MonkeyPatch) -> None: + """Present a gateway key by default. + + Launching a run now guards on ``config.require_gateway_key()`` before ever reaching + ``execute_run``, regardless of whether a test injects its own agent via the + ``get_agent_factory`` override. Without this, every pre-existing run test below would fail + on the guard before its injected agent ever ran. The test that targets the guard itself + clears the key explicitly. + """ + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-test-gateway-key") + + def _client(store: Store, agent_factory) -> httpx.AsyncClient: """Build an ASGI client with the store and agent-factory dependencies overridden.""" app = create_app() @@ -432,3 +445,77 @@ async def test_background_task_failure_marks_run_failed(store: Store) -> None: assert final["status"] == RunStatus.FAILED.value assert final["error"] + + +# -- Gateway guard -------------------------------------------------------------- +# +# defer_model_check=True lets build_agent succeed with no gateway key, so today's failure +# lands deep inside runner._score_row and is recorded as one error per row (20 rows -> 20 +# failed results, COMPLETED_WITH_ERRORS). The guard sits in create_run before store.create_run, +# so a keyless run fails synchronously at request time: one 422 ConfigError, no persisted run, +# and zero RunResult rows -- not a PENDING run whose failure is only discoverable by polling. + + +@pytest.mark.anyio +async def test_run_without_gateway_key_fails_cleanly_with_no_results( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + version = make_version(store) + dataset, _ = make_dataset(store, ["pass", "fail", "pass"]) + + async with _client(store, constant_factory()) as client: + resp = await client.post( + "/api/runs", + json={"kind": "validation", "version_id": version.id, "dataset_id": dataset.id}, + ) + + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + # No run was ever created, let alone any RunResult rows. + assert store.list_runs(dataset_id=dataset.id) == [] + + +@pytest.mark.anyio +async def test_retry_failed_without_gateway_key_fails_cleanly( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """The guard must sit before any other work in ``retry_failed``. + + Without it, a keyless retry would reset the prior terminal run to ``PENDING`` + and launch a background task that later fails it row-by-row -- mutating a run + that a 200 response has already told the client succeeded. + """ + version = make_version(store) + dataset, _rows = make_dataset(store, ["pass", "pass", "pass"], inputs=["ok0", "BOOM", "ok2"]) + + def respond(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: + if "BOOM" in str(messages): + raise RuntimeError("kaboom") + name = info.output_tools[0].name + return ModelResponse(parts=[ToolCallPart(tool_name=name, args={"verdict": "pass"})]) + + factory = lambda v: Agent(FunctionModel(respond), output_type=build_output_model(v)) + + async with _client(store, factory) as client: + body = await _start_run(client, version.id, dataset.id, concurrency=1) + run_id = body["id"] + before = await _poll_until_terminal(client, run_id) + assert before["status"] == RunStatus.COMPLETED_WITH_ERRORS.value + before_results = {r.row_id: r.id for r in store.list_results(run_id)} + + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + resp = await client.post(f"/api/runs/{run_id}/retry-failed") + + assert resp.status_code == 422, resp.text + error = resp.json()["error"] + assert error["type"] == "ConfigError" + assert "valcore config set-key" in error["message"] + + run = store.get_run(run_id) + assert run.status is RunStatus.COMPLETED_WITH_ERRORS + after_results = {r.row_id: r.id for r in store.list_results(run_id)} + assert after_results == before_results + assert run_id not in _tasks diff --git a/tests/test_api_setup.py b/tests/test_api_setup.py new file mode 100644 index 0000000..7e89442 --- /dev/null +++ b/tests/test_api_setup.py @@ -0,0 +1,333 @@ +"""Tests for the read-only setup endpoint: key presence, envelope shape, and secret hygiene. + +``GET /api/setup`` is the only surface for reporting configuration status; keys are written +only via the CLI, so there is no POST here. Presence must reflect the *effective* value (an +exported env var counts, matching ``apply_gateway_key``'s env-wins precedence), and the response +must never carry a key's actual value -- only booleans -- so a future field addition that leaked +one would be caught here rather than in production. +""" + +from collections.abc import AsyncIterator + +import httpx +import pytest + +from valcore.api.deps import get_store +from valcore.api.main import create_app +from valcore.config import FileConfig, save_config +from valcore.store import Store, create_engine, init_db + +GATEWAY_ENV = "PYDANTIC_AI_GATEWAY_API_KEY" +LOGFIRE_TOKEN_ENV = "LOGFIRE_TOKEN" + +CATEGORICAL_SCHEMA = {"kind": "categorical", "labels": ["good", "bad"]} + + +@pytest.fixture(autouse=True) +def _no_ambient_keys(monkeypatch: pytest.MonkeyPatch) -> None: + """Start every test with neither env var set, so presence reflects only what the test sets.""" + monkeypatch.delenv(GATEWAY_ENV, raising=False) + monkeypatch.delenv(LOGFIRE_TOKEN_ENV, raising=False) + + +def _client(app) -> httpx.AsyncClient: + """Return an ASGI-backed client bound to the given app.""" + transport = httpx.ASGITransport(app=app) + return httpx.AsyncClient(transport=transport, base_url="http://test") + + +async def _get_setup(app) -> dict: + async with _client(app) as client: + resp = await client.get("/api/setup") + assert resp.status_code == 200, resp.text + return resp.json() + + +def _by_name(body: dict) -> dict[str, dict]: + return {entry["name"]: entry for entry in body["keys"]} + + +# -- Envelope shape ------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_setup_lists_exactly_the_three_documented_keys() -> None: + body = await _get_setup(create_app()) + assert {entry["name"] for entry in body["keys"]} == { + "gateway_api_key", + "logfire_token", + "logfire_api_key", + } + + +@pytest.mark.anyio +async def test_gateway_key_metadata_matches_the_documented_contract() -> None: + body = await _get_setup(create_app()) + entry = _by_name(body)["gateway_api_key"] + assert entry["required"] is True + assert entry["label"] == "Pydantic AI Gateway key" + assert entry["command"] == "valcore config set-key" + assert entry["purpose"] == "Runs evaluators and generates evaluators and datasets." + + +@pytest.mark.anyio +async def test_logfire_token_metadata_matches_the_documented_contract() -> None: + body = await _get_setup(create_app()) + entry = _by_name(body)["logfire_token"] + assert entry["required"] is False + assert entry["label"] == "Logfire write token" + assert entry["command"] == "valcore config set-logfire-token" + assert entry["purpose"] == "Sends run traces to Logfire." + + +@pytest.mark.anyio +async def test_logfire_api_key_metadata_matches_the_documented_contract() -> None: + body = await _get_setup(create_app()) + entry = _by_name(body)["logfire_api_key"] + assert entry["required"] is False + assert entry["label"] == "Logfire API key" + assert entry["command"] == "valcore config set-logfire-key" + assert entry["purpose"] == "Pushes datasets to Logfire's hosted store." + + +# -- Effective presence: gateway_api_key (env + file, four cases) -------------- + + +@pytest.mark.anyio +async def test_gateway_key_absent_from_neither() -> None: + body = await _get_setup(create_app()) + assert _by_name(body)["gateway_api_key"]["set"] is False + + +@pytest.mark.anyio +async def test_gateway_key_present_from_env_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(GATEWAY_ENV, "sk-from-env") + body = await _get_setup(create_app()) + assert _by_name(body)["gateway_api_key"]["set"] is True + + +@pytest.mark.anyio +async def test_gateway_key_present_from_file_only() -> None: + save_config(FileConfig(gateway_api_key="sk-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["gateway_api_key"]["set"] is True + + +@pytest.mark.anyio +async def test_gateway_key_present_from_both(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(GATEWAY_ENV, "sk-from-env") + save_config(FileConfig(gateway_api_key="sk-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["gateway_api_key"]["set"] is True + + +# -- Effective presence: logfire_token (env + file, four cases) ---------------- + + +@pytest.mark.anyio +async def test_logfire_token_absent_from_neither() -> None: + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_token"]["set"] is False + + +@pytest.mark.anyio +async def test_logfire_token_present_from_env_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(LOGFIRE_TOKEN_ENV, "lf-from-env") + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_token"]["set"] is True + + +@pytest.mark.anyio +async def test_logfire_token_present_from_file_only() -> None: + save_config(FileConfig(logfire_token="lf-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_token"]["set"] is True + + +@pytest.mark.anyio +async def test_logfire_token_present_from_both(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv(LOGFIRE_TOKEN_ENV, "lf-from-env") + save_config(FileConfig(logfire_token="lf-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_token"]["set"] is True + + +# -- Effective presence: logfire_api_key (file-only; env never counts) -------- + + +@pytest.mark.anyio +async def test_logfire_api_key_absent_by_default() -> None: + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_api_key"]["set"] is False + + +@pytest.mark.anyio +async def test_logfire_api_key_present_from_file() -> None: + save_config(FileConfig(logfire_api_key="lf-api-key-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_api_key"]["set"] is True + + +@pytest.mark.anyio +async def test_logfire_api_key_ignores_a_same_named_env_var( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """There is no env var for this key; even one with a plausible name must not count.""" + monkeypatch.setenv("LOGFIRE_API_KEY", "lf-from-env") + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_api_key"]["set"] is False + + +@pytest.mark.anyio +async def test_logfire_api_key_present_from_both_file_and_env_lookalike( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setenv("LOGFIRE_API_KEY", "lf-from-env") + save_config(FileConfig(logfire_api_key="lf-api-key-from-file")) + body = await _get_setup(create_app()) + assert _by_name(body)["logfire_api_key"]["set"] is True + + +# -- No key value ever appears in the response --------------------------------- + + +@pytest.mark.anyio +async def test_no_secret_value_leaks_into_the_response(monkeypatch: pytest.MonkeyPatch) -> None: + """The regression guard against a future field leaking a configured secret.""" + monkeypatch.setenv(GATEWAY_ENV, "sk-super-secret-gateway-value") + save_config( + FileConfig( + logfire_token="lf-super-secret-token-value", + logfire_api_key="lf-super-secret-apikey-value", + ) + ) + async with _client(create_app()) as client: + resp = await client.get("/api/setup") + assert resp.status_code == 200, resp.text + raw = resp.text + for secret in ( + "sk-super-secret-gateway-value", + "lf-super-secret-token-value", + "lf-super-secret-apikey-value", + ): + assert secret not in raw + + +# -- No POST route -------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_post_setup_is_not_allowed() -> None: + async with _client(create_app()) as client: + resp = await client.post("/api/setup", json={}) + assert resp.status_code == 405 + + +# -- App starts and serves health with no Logfire token configured ------------- + + +@pytest.mark.anyio +async def test_health_still_works_with_no_logfire_token_configured() -> None: + async with _client(create_app()) as client: + resp = await client.get("/api/health") + assert resp.status_code == 200 + assert resp.json() == {"status": "ok"} + + +@pytest.mark.anyio +async def test_create_app_is_idempotent_across_repeated_calls() -> None: + """configure_tracing/instrument_fastapi must tolerate create_app() running more than once.""" + create_app() + app_again = create_app() + async with _client(app_again) as client: + resp = await client.get("/api/health") + assert resp.status_code == 200 + + +# -- Ungated endpoints keep working with no gateway key ------------------------ + + +@pytest.fixture +def store(tmp_path) -> Store: + """A fresh file-backed store isolated per test.""" + engine = create_engine(tmp_path / "setup.db") + init_db(engine) + return Store(engine) + + +@pytest.fixture +async def client(store: Store) -> AsyncIterator[httpx.AsyncClient]: + """An ASGI client whose store dependency is overridden with the test store.""" + app = create_app() + app.dependency_overrides[get_store] = lambda: store + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient(transport=transport, base_url="http://test") as c: + yield c + + +@pytest.mark.anyio +async def test_ungated_endpoints_still_work_with_no_gateway_key( + client: httpx.AsyncClient, +) -> None: + """Manual authoring, upload, labeling, and export must never require the gateway key.""" + created = await client.post( + "/api/datasets", + json={ + "name": "blank", + "description": "", + "columns": ["question"], + "label_schema": CATEGORICAL_SCHEMA, + }, + ) + assert created.status_code == 200, created.text + ds_id = created.json()["id"] + + csv = b"question,answer\nq1,a1\n" + uploaded = await client.post( + "/api/datasets/upload", + files={"file": ("d.csv", csv, "text/csv")}, + data={"name": "uploaded"}, + ) + assert uploaded.status_code == 200, uploaded.text + + appended = await client.post(f"/api/datasets/{ds_id}/rows", json={"rows": [{"question": "q"}]}) + assert appended.status_code == 200, appended.text + row_id = appended.json()[0]["id"] + + patched = await client.patch(f"/api/datasets/rows/{row_id}", json={"label": "good"}) + assert patched.status_code == 200, patched.text + assert patched.json()["label"] == {"value": "good"} + + eval_created = await client.post("/api/evaluators", json={"name": "E"}) + assert eval_created.status_code == 200, eval_created.text + eval_id = eval_created.json()["id"] + + version = await client.post( + f"/api/evaluators/{eval_id}/versions", + json={ + "version_name": "v1", + "notes": "", + "model": "gateway/anthropic:claude-sonnet-5", + "instructions": "Judge.", + "prompt_template": "Input: {question}", + "required_columns": ["question"], + "output_fields": [ + { + "name": "verdict", + "type": "enum", + "description": "v", + "enum_values": ["good", "bad"], + } + ], + "score_field": "verdict", + "score_kind": "categorical", + "score_labels": ["good", "bad"], + }, + ) + assert version.status_code == 200, version.text + + exported_version = await client.get(f"/api/evaluators/versions/{version.json()['id']}/export") + assert exported_version.status_code == 200, exported_version.text + + exported_dataset = await client.get(f"/api/datasets/{ds_id}/export.json") + assert exported_dataset.status_code == 200, exported_dataset.text diff --git a/tests/test_cli.py b/tests/test_cli.py index 16c280b..23d9cae 100644 --- a/tests/test_cli.py +++ b/tests/test_cli.py @@ -2,8 +2,16 @@ No network and no real home directory: ``VALCORE_HOME`` is pointed at ``tmp_path`` by the autouse fixture in ``conftest.py``, the store is a fresh ``tmp_path`` SQLite -DB, and agent behavior is driven by a ``FunctionModel`` injected via a monkeypatch -of ``valcore.runner.build_agent``. +DB, and agent behavior for ``run`` is driven by a ``FunctionModel`` injected via a +monkeypatch of ``valcore.runner.build_agent``; ``experiment`` tests use ``TestModel`` +against ``valcore.experiment.build_agent`` instead, per the test plan. ``logfire push`` +is exercised by monkeypatching +``valcore.logfire_io.push_dataset`` with an async stub, following the module-qualified +call convention the task interfaces describe (``config.require_gateway_key()``, +``experiment.execute_experiment(...)``, ``logfire_io.push_dataset(...)``, +``tracing.configure_tracing(...)``) -- patching the source module's attribute works +regardless of how ``cli.main`` imports the module, as long as it calls through a +module reference rather than a name bound at import time. """ import json @@ -14,9 +22,11 @@ from pydantic_ai import Agent from pydantic_ai.messages import ModelMessage, ModelResponse, ToolCallPart from pydantic_ai.models.function import AgentInfo, FunctionModel +from pydantic_ai.models.test import TestModel from valcore.cli.main import cli from valcore.cli.resolve import resolve_dataset, resolve_evaluator, resolve_version +from valcore.config import load_config from valcore.config_io import EvalPackage from valcore.errors import ContractError, NotFoundError from valcore.export import render_script @@ -88,6 +98,22 @@ def respond(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: return build +def _constant_test_model_agent_builder(verdict: str = "pass"): + """Return a ``build_agent`` replacement using ``TestModel`` that always emits ``verdict``. + + Used for the ``experiment`` tests per the test plan's ``TestModel`` requirement; + ``TestModel(custom_output_args=...)`` pins the structured output without a network call. + """ + + def build(version) -> Agent: + return Agent( + TestModel(custom_output_args={"verdict": verdict}), + output_type=build_output_model(version), + ) + + return build + + def _invoke(runner: CliRunner, db_path, *args: str, **kwargs): """Invoke the CLI with ``--db`` bound to the test database.""" return runner.invoke(cli, ["--db", str(db_path), *args], **kwargs) @@ -98,6 +124,18 @@ def runner() -> CliRunner: return CliRunner() +@pytest.fixture(autouse=True) +def _gateway_key_present(monkeypatch: pytest.MonkeyPatch) -> None: + """Present a gateway key by default. + + ``run`` and ``experiment`` now guard on ``config.require_gateway_key()`` before doing any + work. Without this, every pre-existing happy-path test below would fail on the guard before + its injected agent ever ran -- mirroring the identical fixture in ``test_api_runs.py`` for the + same guard on the API surface. Tests that target the guard itself clear the key explicitly. + """ + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-test-gateway-key") + + # -- version ------------------------------------------------------------------ @@ -107,6 +145,30 @@ def test_version(runner, db_path): assert result.output.strip() == package_version("valcore") +# -- tracing -------------------------------------------------------------------- + + +def test_cli_group_configures_tracing_once_per_invocation(runner, db_path, monkeypatch): + """The ``cli`` group callback must call ``configure_tracing(load_config())`` exactly once. + + Patches the whole function (not just its internals) so the module's own idempotency + flag is irrelevant here -- this only checks that the CLI actually calls it, once, on + every invocation, regardless of what command is run. + """ + calls = [] + monkeypatch.setattr("valcore.tracing.configure_tracing", lambda cfg: calls.append(cfg)) + result = _invoke(runner, db_path, "version") + assert result.exit_code == 0 + assert len(calls) == 1 + + +def test_cli_group_configures_tracing_with_no_token_without_error(runner, db_path): + """With no Logfire token configured, tracing configuration must still be a silent no-op.""" + result = _invoke(runner, db_path, "version") + assert result.exit_code == 0 + assert result.exception is None + + # -- list --------------------------------------------------------------------- @@ -242,6 +304,141 @@ def test_run_unresolvable_evaluator_exits_1(runner, store, db_path): assert "error:" in result.stderr +# -- gateway guard -------------------------------------------------------------- +# +# A keyless invocation must exit non-zero with a message naming `valcore config +# set-key`, and must persist no RunResult rows -- never the N-failed-rows outcome +# `require_gateway_key` exists to prevent (see config.require_gateway_key's docstring). + + +def test_run_no_gateway_key_exits_nonzero_naming_set_key(runner, store, db_path, monkeypatch): + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + result = _invoke(runner, db_path, "run", "judge", "cases") + assert result.exit_code != 0 + assert "valcore config set-key" in result.stderr + for run in store.list_runs(): + assert store.list_results(run.id) == [] + + +def test_experiment_no_gateway_key_exits_nonzero_naming_set_key( + runner, store, db_path, monkeypatch +): + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + result = _invoke(runner, db_path, "experiment", "judge", "cases") + assert result.exit_code != 0 + assert "valcore config set-key" in result.stderr + for run in store.list_runs(): + assert store.list_results(run.id) == [] + + +def test_export_succeeds_without_gateway_key(runner, store, db_path, monkeypatch): + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + result = _invoke(runner, db_path, "export", "judge") + assert result.exit_code == 0 + + +def test_list_succeeds_without_gateway_key(runner, store, db_path, monkeypatch): + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + result = _invoke(runner, db_path, "list", "evaluators") + assert result.exit_code == 0 + + +def test_import_succeeds_without_gateway_key(runner, store, db_path, tmp_path, monkeypatch): + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + out = tmp_path / "pkg.json" + exported = _invoke( + runner, db_path, "export", "judge", "--dataset", "cases", "--format", "json", "-o", str(out) + ) + assert exported.exit_code == 0 + + dest = tmp_path / "imported.db" + result = _invoke(runner, dest, "import", str(out)) + assert result.exit_code == 0 + + +# -- experiment ----------------------------------------------------------------- + + +def test_experiment_happy_path_writes_results(runner, store, db_path, monkeypatch): + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + result = _invoke(runner, db_path, "experiment", "judge", "cases") + assert result.exit_code == 0 + runs = store.list_runs() + assert len(runs) == 1 + assert len(store.list_results(runs[0].id)) == 4 + + +def test_experiment_json_stdout_is_pure_json(runner, store, db_path, monkeypatch): + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + result = _invoke(runner, db_path, "experiment", "judge", "cases", "--json") + assert result.exit_code == 0 + payload = json.loads(result.stdout) + assert payload["metrics"]["n"] == 4 + assert "accuracy" in payload["metrics"] + + +def test_experiment_concurrency_option_sets_run_concurrency(runner, store, db_path, monkeypatch): + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + result = _invoke(runner, db_path, "experiment", "judge", "cases", "--concurrency", "7") + assert result.exit_code == 0 + assert store.list_runs()[0].concurrency == 7 + + +def test_experiment_unresolvable_evaluator_exits_1(runner, store, db_path): + result = _invoke(runner, db_path, "experiment", "no-such-evaluator", "cases") + assert result.exit_code == 1 + assert "error:" in result.stderr + + +def test_experiment_has_no_watch_option(runner, store, db_path, monkeypatch): + """``Dataset.evaluate`` cannot be cancelled, so ``experiment`` must not gain ``--watch``.""" + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + result = _invoke(runner, db_path, "experiment", "judge", "cases", "--watch") + assert result.exit_code != 0 + + +def test_experiment_and_run_agree(runner, store, db_path, monkeypatch): + """The CLI-level version of the two-engines-agree guarantee: identical metrics.""" + monkeypatch.setattr("valcore.runner.build_agent", _constant_test_model_agent_builder("pass")) + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + + run_result = _invoke(runner, db_path, "run", "judge", "cases", "--json") + assert run_result.exit_code == 0 + run_payload = json.loads(run_result.stdout) + + experiment_result = _invoke(runner, db_path, "experiment", "judge", "cases", "--json") + assert experiment_result.exit_code == 0 + experiment_payload = json.loads(experiment_result.stdout) + + assert experiment_payload["status"] == "completed" + assert experiment_payload["metrics"] == run_payload["metrics"] + + +def test_experiment_json_payload_shape_matches_run(runner, store, db_path, monkeypatch): + monkeypatch.setattr("valcore.runner.build_agent", _constant_test_model_agent_builder("pass")) + monkeypatch.setattr( + "valcore.experiment.build_agent", _constant_test_model_agent_builder("pass") + ) + + run_payload = json.loads(_invoke(runner, db_path, "run", "judge", "cases", "--json").stdout) + experiment_payload = json.loads( + _invoke(runner, db_path, "experiment", "judge", "cases", "--json").stdout + ) + + assert set(experiment_payload.keys()) == set(run_payload.keys()) + assert set(experiment_payload["results"][0].keys()) == set(run_payload["results"][0].keys()) + + # -- export ------------------------------------------------------------------- @@ -497,6 +694,206 @@ def test_config_get_show_key_reveals(runner, db_path): assert "sk-secret-1234" in result.output +def test_config_set_logfire_token_persists_and_preserves_others(runner, db_path): + _invoke(runner, db_path, "config", "set-key", "sk-secret-1234") + result = _invoke(runner, db_path, "config", "set-logfire-token", "lf-token-5678") + assert result.exit_code == 0 + + cfg = load_config() + assert cfg.logfire_token == "lf-token-5678" + assert cfg.gateway_api_key == "sk-secret-1234" + + +def test_config_set_logfire_token_prompts_when_omitted(runner, db_path): + result = _invoke(runner, db_path, "config", "set-logfire-token", input="lf-prompted\n") + assert result.exit_code == 0 + assert load_config().logfire_token == "lf-prompted" + # Hidden input: the typed value never echoes to output. + assert "lf-prompted" not in result.output + + +def test_config_set_logfire_key_persists_and_preserves_others(runner, db_path): + _invoke(runner, db_path, "config", "set-logfire-token", "lf-existing-token") + result = _invoke(runner, db_path, "config", "set-logfire-key", "lf-key-9999") + assert result.exit_code == 0 + + cfg = load_config() + assert cfg.logfire_api_key == "lf-key-9999" + assert cfg.logfire_token == "lf-existing-token" + + +def test_config_set_logfire_key_prompts_when_omitted(runner, db_path): + result = _invoke(runner, db_path, "config", "set-logfire-key", input="lf-key-prompted\n") + assert result.exit_code == 0 + assert load_config().logfire_api_key == "lf-key-prompted" + assert "lf-key-prompted" not in result.output + + +def test_config_get_logfire_presence_changes_when_set_and_never_leaks_values(runner, db_path): + before = json.loads(_invoke(runner, db_path, "config", "get", "--json").output) + + _invoke(runner, db_path, "config", "set-logfire-token", "lf-secret-token") + _invoke(runner, db_path, "config", "set-logfire-key", "lf-secret-apikey") + + after_result = _invoke(runner, db_path, "config", "get", "--json") + assert after_result.exit_code == 0 + after = json.loads(after_result.output) + + # Presence is reflected somehow -- as a masked string or boolean, format-agnostic here -- + # but the raw secret is never the field's value, and never appears anywhere in output. + assert after["logfire_token"] != before["logfire_token"] + assert after["logfire_api_key"] != before["logfire_api_key"] + assert after["logfire_token"] != "lf-secret-token" + assert after["logfire_api_key"] != "lf-secret-apikey" + assert "lf-secret-token" not in after_result.output + assert "lf-secret-apikey" not in after_result.output + + +def test_config_get_reports_effective_presence_from_env_only(runner, db_path, monkeypatch): + """An env-only key or token, never written to the config file, must report as present. + + ``gateway_key_present``/``logfire_token_present`` treat an exported env var as + effectively set, matching ``apply_gateway_key``'s env-wins precedence -- ``config get`` + must agree rather than fall back to the raw (``None``) file value and report a false + absence. + """ + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-env-only-1234") + monkeypatch.setenv("LOGFIRE_TOKEN", "lf-env-only-token") + + result = _invoke(runner, db_path, "config", "get", "--json") + assert result.exit_code == 0 + payload = json.loads(result.output) + + assert payload["gateway_api_key"] not in (None, False) + assert payload["logfire_token"] is True + assert load_config().gateway_api_key is None + assert load_config().logfire_token is None + assert "sk-env-only-1234" not in result.output + assert "lf-env-only-token" not in result.output + + +def test_config_get_show_key_does_not_reveal_logfire_secrets(runner, db_path): + """``--show-key`` already governs revealing the gateway key; it must not newly govern these.""" + _invoke(runner, db_path, "config", "set-key", "sk-secret-1234") + _invoke(runner, db_path, "config", "set-logfire-token", "lf-secret-token") + _invoke(runner, db_path, "config", "set-logfire-key", "lf-secret-apikey") + + result = _invoke(runner, db_path, "config", "get", "--show-key") + assert result.exit_code == 0 + assert "sk-secret-1234" in result.output + assert "lf-secret-token" not in result.output + assert "lf-secret-apikey" not in result.output + + +# -- logfire -------------------------------------------------------------------- + + +def test_logfire_push_prints_id_name_and_case_count_never_a_url( + runner, store, db_path, monkeypatch +): + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + return { + "id": "abc-123", + "name": "pushed-cases", + "case_count": 4, + "output_schema": {"type": "string"}, + } + + monkeypatch.setattr("valcore.logfire_io.push_dataset", fake_push_dataset) + result = _invoke(runner, db_path, "logfire", "push", "cases") + assert result.exit_code == 0 + assert "abc-123" in result.output + assert "pushed-cases" in result.output + assert "4" in result.output + assert "http" not in result.output.lower() + assert "url" not in result.output.lower() + + +def test_logfire_push_resolves_dataset_and_passes_its_rows(runner, store, db_path, monkeypatch): + captured = {} + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + captured["dataset_name"] = dataset.name + captured["row_count"] = len(rows) + return {"id": "x", "name": dataset.name, "case_count": len(rows), "output_schema": None} + + monkeypatch.setattr("valcore.logfire_io.push_dataset", fake_push_dataset) + result = _invoke(runner, db_path, "logfire", "push", "cases") + assert result.exit_code == 0 + assert captured == {"dataset_name": "cases", "row_count": 4} + + +def test_logfire_push_defaults_have_no_name_or_description(runner, store, db_path, monkeypatch): + calls = {} + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + calls.update(name=name, description=description, on_conflict=on_conflict) + return {"id": "x", "name": "cases", "case_count": len(rows), "output_schema": None} + + monkeypatch.setattr("valcore.logfire_io.push_dataset", fake_push_dataset) + result = _invoke(runner, db_path, "logfire", "push", "cases") + assert result.exit_code == 0 + assert calls == {"name": None, "description": None, "on_conflict": "update"} + + +def test_logfire_push_passes_name_description_and_on_conflict_through( + runner, store, db_path, monkeypatch +): + calls = {} + + async def fake_push_dataset( + dataset, rows, *, api_key=None, name=None, description=None, on_conflict="update" + ): + calls.update(name=name, description=description, on_conflict=on_conflict) + return {"id": "x", "name": name, "case_count": len(rows), "output_schema": None} + + monkeypatch.setattr("valcore.logfire_io.push_dataset", fake_push_dataset) + result = _invoke( + runner, + db_path, + "logfire", + "push", + "cases", + "--name", + "custom-name", + "--description", + "custom description", + "--on-conflict", + "error", + ) + assert result.exit_code == 0 + assert calls == { + "name": "custom-name", + "description": "custom description", + "on_conflict": "error", + } + + +def test_logfire_push_invalid_on_conflict_choice_exits_nonzero(runner, store, db_path): + result = _invoke(runner, db_path, "logfire", "push", "cases", "--on-conflict", "bogus") + assert result.exit_code != 0 + + +def test_logfire_push_unresolvable_dataset_exits_1(runner, store, db_path): + result = _invoke(runner, db_path, "logfire", "push", "no-such-dataset") + assert result.exit_code == 1 + assert "error:" in result.stderr + + +def test_logfire_push_no_api_key_exits_nonzero_naming_set_logfire_key(runner, store, db_path): + # No stub installed: with no key configured, `push_dataset` must fail before any + # network-facing import or call, exactly as `test_logfire_io.py` pins directly. + result = _invoke(runner, db_path, "logfire", "push", "cases") + assert result.exit_code != 0 + assert "valcore config set-logfire-key" in result.stderr + + # -- ./valcore.db startup notice -------------------------------------------- diff --git a/tests/test_config.py b/tests/test_config.py index 79be89a..0c51e86 100644 --- a/tests/test_config.py +++ b/tests/test_config.py @@ -1,5 +1,6 @@ """Tests for the home directory, TOML config layer, and settings precedence.""" +import importlib.util import os import stat import warnings @@ -11,10 +12,18 @@ from valcore.config import ( FileConfig, apply_gateway_key, + apply_logfire_token, + gateway_key_present, load_config, + logfire_api_key_present, + logfire_token_present, + require_gateway_key, save_config, set_key, + set_logfire_api_key, + set_logfire_token, ) +from valcore.errors import ConfigError from valcore.paths import config_path, default_db_path, home_dir @@ -52,6 +61,8 @@ def test_load_config_missing_returns_all_none(_home: Path) -> None: assert cfg.port is None assert cfg.concurrency is None assert cfg.db_path is None + assert cfg.logfire_token is None + assert cfg.logfire_api_key is None def test_save_load_round_trips_every_field() -> None: @@ -61,12 +72,41 @@ def test_save_load_round_trips_every_field() -> None: port=9123, concurrency=4, db_path=Path("/tmp/custom.db"), + logfire_token="lf-write-token", + logfire_api_key="lf-api-key", ) save_config(cfg) loaded = load_config() assert loaded == cfg +def test_save_load_round_trips_logfire_fields_only() -> None: + """A config with only the logfire fields set dumps only those lines.""" + cfg = FileConfig(logfire_token="lf-write-token", logfire_api_key="lf-api-key") + save_config(cfg) + + content = config_path().read_text() + assert 'logfire_token = "lf-write-token"' in content + assert 'logfire_api_key = "lf-api-key"' in content + # No other fields were set, so no other lines should appear. + assert "gateway_api_key" not in content + assert "model" not in content + assert "port" not in content + assert "concurrency" not in content + assert "db_path" not in content + + loaded = load_config() + assert loaded == cfg + + +def test_dump_toml_omits_unset_logfire_fields() -> None: + """The dumper writes only non-None fields; logfire fields are no exception.""" + save_config(FileConfig(gateway_api_key="sk-secret")) + content = config_path().read_text() + assert "logfire_token" not in content + assert "logfire_api_key" not in content + + def test_saved_file_mode_is_0600() -> None: save_config(FileConfig(model="gateway/openai:gpt-5")) assert _mode(config_path()) == 0o600 @@ -98,6 +138,32 @@ def test_apply_gateway_key_respects_existing_env(monkeypatch: pytest.MonkeyPatch assert os.environ["PYDANTIC_AI_GATEWAY_API_KEY"] == "sk-from-env" +def test_apply_logfire_token_sets_when_absent(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("LOGFIRE_TOKEN", raising=False) + cfg = FileConfig(logfire_token="lf-from-config") + + assert apply_logfire_token(cfg) is True + assert os.environ["LOGFIRE_TOKEN"] == "lf-from-config" + + +def test_apply_logfire_token_respects_existing_env(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LOGFIRE_TOKEN", "lf-from-env") + cfg = FileConfig(logfire_token="lf-from-config") + + assert apply_logfire_token(cfg) is False + assert os.environ["LOGFIRE_TOKEN"] == "lf-from-env" + + +def test_apply_logfire_token_returns_false_with_no_token( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.delenv("LOGFIRE_TOKEN", raising=False) + cfg = FileConfig() + + assert apply_logfire_token(cfg) is False + assert "LOGFIRE_TOKEN" not in os.environ + + def test_set_key_preserves_other_fields() -> None: save_config(FileConfig(model="gateway/openai:gpt-5", concurrency=3)) set_key("sk-added") @@ -107,6 +173,42 @@ def test_set_key_preserves_other_fields() -> None: assert loaded.concurrency == 3 +def test_set_logfire_token_preserves_other_fields() -> None: + save_config( + FileConfig( + gateway_api_key="sk-existing", + model="gateway/openai:gpt-5", + concurrency=3, + logfire_api_key="lf-existing-api-key", + ) + ) + set_logfire_token("lf-added-token") + loaded = load_config() + assert loaded.logfire_token == "lf-added-token" + assert loaded.gateway_api_key == "sk-existing" + assert loaded.model == "gateway/openai:gpt-5" + assert loaded.concurrency == 3 + assert loaded.logfire_api_key == "lf-existing-api-key" + + +def test_set_logfire_api_key_preserves_other_fields() -> None: + save_config( + FileConfig( + gateway_api_key="sk-existing", + model="gateway/openai:gpt-5", + concurrency=3, + logfire_token="lf-existing-token", + ) + ) + set_logfire_api_key("lf-added-api-key") + loaded = load_config() + assert loaded.logfire_api_key == "lf-added-api-key" + assert loaded.gateway_api_key == "sk-existing" + assert loaded.model == "gateway/openai:gpt-5" + assert loaded.concurrency == 3 + assert loaded.logfire_token == "lf-existing-token" + + def test_db_path_precedence(monkeypatch: pytest.MonkeyPatch) -> None: # default: no env, no config -> default_db_path() settings.get_settings.cache_clear() @@ -166,3 +268,97 @@ def _boom(src: str, dst: str) -> None: assert load_config().model == "gateway/openai:gpt-5" leftovers = list(config_path().parent.glob(".config-*.toml")) assert leftovers == [] + + +# --- Effective-presence helpers ------------------------------------------------- +# +# A key is "present" when either its env var is exported or the file config +# carries it. gateway_key_present and logfire_token_present each have an env +# source, so four cases apiece; logfire_api_key_present is file-only, so three. + + +def test_gateway_key_present_false_from_neither(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + assert gateway_key_present(FileConfig()) is False + + +def test_gateway_key_present_true_from_env_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-from-env") + assert gateway_key_present(FileConfig()) is True + + +def test_gateway_key_present_true_from_file_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + assert gateway_key_present(FileConfig(gateway_api_key="sk-from-file")) is True + + +def test_gateway_key_present_true_from_both(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-from-env") + assert gateway_key_present(FileConfig(gateway_api_key="sk-from-file")) is True + + +def test_logfire_token_present_false_from_neither(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("LOGFIRE_TOKEN", raising=False) + assert logfire_token_present(FileConfig()) is False + + +def test_logfire_token_present_true_from_env_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LOGFIRE_TOKEN", "lf-from-env") + assert logfire_token_present(FileConfig()) is True + + +def test_logfire_token_present_true_from_file_only(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("LOGFIRE_TOKEN", raising=False) + assert logfire_token_present(FileConfig(logfire_token="lf-from-file")) is True + + +def test_logfire_token_present_true_from_both(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("LOGFIRE_TOKEN", "lf-from-env") + assert logfire_token_present(FileConfig(logfire_token="lf-from-file")) is True + + +def test_logfire_api_key_present_false_with_none() -> None: + assert logfire_api_key_present(FileConfig()) is False + + +def test_logfire_api_key_present_true_from_file() -> None: + assert logfire_api_key_present(FileConfig(logfire_api_key="lf-api-key")) is True + + +def test_logfire_api_key_present_ignores_env(monkeypatch: pytest.MonkeyPatch) -> None: + """The API key has no env var; even a same-named env var must not count.""" + monkeypatch.setenv("LOGFIRE_API_KEY", "lf-from-env") + assert logfire_api_key_present(FileConfig()) is False + + +# --- require_gateway_key --------------------------------------------------------- + + +def test_require_gateway_key_passes_with_env_key(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setenv("PYDANTIC_AI_GATEWAY_API_KEY", "sk-from-env") + require_gateway_key() # must not raise + + +def test_require_gateway_key_passes_with_file_key(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + save_config(FileConfig(gateway_api_key="sk-from-file")) + require_gateway_key() # must not raise + + +def test_require_gateway_key_raises_with_neither(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv("PYDANTIC_AI_GATEWAY_API_KEY", raising=False) + with pytest.raises(ConfigError, match="valcore config set-key"): + require_gateway_key() + + +# --- dev-group extra guard --------------------------------------------------------- + + +def test_logfire_extra_is_present_in_dev_environment() -> None: + """Guards the pyproject.toml dev-group entry. + + Without this, a dropped `logfire` dev-group line would silently skip the + span tests in test_tracing.py and test_runner.py forever instead of + failing loudly here. + """ + assert importlib.util.find_spec("logfire") is not None diff --git a/tests/test_experiment.py b/tests/test_experiment.py new file mode 100644 index 0000000..7d29b03 --- /dev/null +++ b/tests/test_experiment.py @@ -0,0 +1,832 @@ +"""Tests for the experiment engine: execution over ``pydantic_evals.Dataset.evaluate``. + +No network: agent behavior is driven by ``TestModel`` agents throughout. A single +``_FlakyTestModel`` subclass (still ``TestModel``-based -- it delegates to +``TestModel.request`` for every call except the one under test) supplies the minimum +custom behavior needed to make one specific call fail, for the tests that exercise a +row/case failure. ``execute_experiment`` exposes no ``agent=`` override (unlike +``runner.execute_run``), so tests monkeypatch ``valcore.experiment.build_agent`` to hand +back a network-free test agent instead of one built from the version's live model string. + +The most important test here is ``test_experiment_and_run_agree``: both engines must +report identical metrics and per-row agreement over the same version and dataset, since +that is the whole point of routing both through ``metrics.compute_metrics``. +""" + +import asyncio +import importlib.util + +import pytest +from pydantic_ai import Agent +from pydantic_ai.models.test import TestModel + +from valcore import tracing +from valcore.errors import ContractError +from valcore.factory import build_output_model +from valcore.models import LabelSource, RunKind, RunStatus, ScoreKind +from valcore.runner import RunEvent, execute_run +from valcore.store import Store, create_engine, init_db + +_LOGFIRE_PRESENT = importlib.util.find_spec("logfire") is not None + +CATEGORICAL_SCHEMA = {"kind": "categorical", "labels": ["pass", "fail"]} +NUMERIC_SCHEMA = {"kind": "numeric"} + +CATEGORICAL_VERSION_FIELDS = { + "version_name": "v1", + "model": "gateway/anthropic:claude-sonnet-5", + "instructions": "Judge the row.", + "prompt_template": "Input: {input} Output: {output}", + "required_columns": ["input", "output"], + "output_fields": [ + { + "name": "verdict", + "type": "enum", + "description": "pass or fail", + "enum_values": ["pass", "fail"], + } + ], + "score_field": "verdict", + "score_kind": ScoreKind.CATEGORICAL, + "score_labels": ["pass", "fail"], +} + +NUMERIC_VERSION_FIELDS = { + "version_name": "v-numeric", + "model": "gateway/anthropic:claude-sonnet-5", + "instructions": "Score the row.", + "prompt_template": "Input: {input} Output: {output}", + "required_columns": ["input", "output"], + "output_fields": [ + {"name": "score", "type": "float", "description": "a numeric score"}, + ], + "score_field": "score", + "score_kind": ScoreKind.NUMERIC, +} + + +@pytest.fixture +def store(tmp_path) -> Store: + """A real Store backed by a fresh SQLite DB under tmp_path.""" + engine = create_engine(tmp_path / "eval.db") + init_db(engine) + return Store(engine) + + +def make_version(store: Store, **overrides): + """Create an evaluator and a valid categorical version, returning the version.""" + evaluator = store.create_evaluator("ev") + fields = {**CATEGORICAL_VERSION_FIELDS, **overrides} + return store.create_version(evaluator.id, **fields) + + +def make_numeric_version(store: Store, **overrides): + """Create an evaluator and a valid numeric version, returning the version.""" + evaluator = store.create_evaluator("ev-numeric") + fields = {**NUMERIC_VERSION_FIELDS, **overrides} + return store.create_version(evaluator.id, **fields) + + +def make_dataset( + store: Store, + labels: list[str | float | None], + *, + columns: list[str] | None = None, + schema: dict | None = None, +): + """Create a dataset with one row per entry in ``labels`` (None = unlabeled).""" + dataset = store.create_dataset( + "ds", + "", + columns if columns is not None else ["input", "output"], + schema if schema is not None else CATEGORICAL_SCHEMA, + ) + rows = store.add_rows( + dataset.id, [{"input": f"in{i}", "output": f"out{i}"} for i in range(len(labels))] + ) + for row, label in zip(rows, labels, strict=True): + if label is not None: + store.set_label(row.id, {"value": label}, LabelSource.MANUAL) + return dataset + + +class _FlakyTestModel(TestModel): + """A ``TestModel`` that raises on one specific call, to exercise a case failure. + + Every call except ``fail_on_call`` behaves exactly like plain ``TestModel`` (delegating + to ``custom_output_args``); this is the minimum custom behavior needed to inject a + single failing row without reaching for a fully custom model. + """ + + def __init__(self, *, fail_on_call: int, error_message: str, **kwargs) -> None: + super().__init__(**kwargs) + self._fail_on_call = fail_on_call + self._error_message = error_message + self._calls = 0 + + async def request(self, messages, model_settings, model_request_parameters): + """Raise on the ``fail_on_call``-th invocation; otherwise defer to ``TestModel``.""" + self._calls += 1 + if self._calls == self._fail_on_call: + raise RuntimeError(self._error_message) + return await super().request(messages, model_settings, model_request_parameters) + + +def constant_agent(version, verdict: str = "pass") -> Agent: + """An agent whose model always emits the given categorical verdict.""" + model = TestModel(custom_output_args={"verdict": verdict}) + return Agent(model, output_type=build_output_model(version)) + + +def constant_numeric_agent(version, score: float) -> Agent: + """An agent whose model always emits the given numeric score.""" + model = TestModel(custom_output_args={"score": score}) + return Agent(model, output_type=build_output_model(version)) + + +def flaky_agent(version, *, fail_on_call: int, verdict: str = "pass", error_message: str) -> Agent: + """An agent whose ``fail_on_call``-th row raises; every other row emits ``verdict``.""" + model = _FlakyTestModel( + fail_on_call=fail_on_call, + error_message=error_message, + custom_output_args={"verdict": verdict}, + ) + return Agent(model, output_type=build_output_model(version)) + + +def patch_build_agent(monkeypatch: pytest.MonkeyPatch, agent: Agent) -> None: + """Force ``execute_experiment`` to use a network-free test agent. + + ``execute_experiment`` has no ``agent=`` override (unlike ``runner.execute_run``), so + the only way to avoid a real model call in a test is to intercept the factory call + it makes internally. + """ + monkeypatch.setattr("valcore.experiment.build_agent", lambda version: agent) + + +def capture_evaluators(monkeypatch: pytest.MonkeyPatch) -> list: + """Patch ``dataset_to_evals`` to record the evaluators list it is actually called with. + + Agreement and metrics are recomputed independently from persisted ``RunResult`` rows, + so a test that only inspects the final results would still pass if the wrong evaluator + (or none at all) were attached to the constructed dataset. Capturing the real call is + the only way to verify the task/evaluator mapping itself. + """ + import valcore.experiment as experiment_module + + captured: list = [] + original = experiment_module.dataset_to_evals + + def spy(dataset, rows, evaluators): + captured.append(evaluators) + return original(dataset, rows, evaluators) + + monkeypatch.setattr(experiment_module, "dataset_to_evals", spy) + return captured + + +# -- Experiment and run agree --------------------------------------------------- + + +@pytest.mark.anyio +async def test_experiment_and_run_agree(store: Store, monkeypatch: pytest.MonkeyPatch) -> None: + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail", "pass"]) + + run_via_runner = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + run_via_experiment = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + runner_result = await execute_run(store, run_via_runner.id, agent=constant_agent(version)) + + patch_build_agent(monkeypatch, constant_agent(version)) + experiment_result = await execute_experiment(store, run_via_experiment.id) + + assert runner_result.status is RunStatus.COMPLETED + assert experiment_result.status is RunStatus.COMPLETED + assert runner_result.metrics == experiment_result.metrics + + runner_scores = {r.row_id: r.score_value for r in store.list_results(run_via_runner.id)} + experiment_scores = {r.row_id: r.score_value for r in store.list_results(run_via_experiment.id)} + assert runner_scores == experiment_scores + + runner_agreement = {r.row_id: r.agreement for r in store.list_results(run_via_runner.id)} + experiment_agreement = { + r.row_id: r.agreement for r in store.list_results(run_via_experiment.id) + } + assert runner_agreement == experiment_agreement + + +@pytest.mark.anyio +async def test_experiment_and_run_agree_numeric( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """The same equivalence holds for numeric scores, exercising NumericDelta end to end.""" + from valcore.experiment import execute_experiment + + version = make_numeric_version(store) + dataset = make_dataset( + store, [1.0, 4.0, 2.5, 9.0, 0.0], columns=["input", "output"], schema=NUMERIC_SCHEMA + ) + + run_via_runner = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + run_via_experiment = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + runner_result = await execute_run( + store, run_via_runner.id, agent=constant_numeric_agent(version, 5.0) + ) + + patch_build_agent(monkeypatch, constant_numeric_agent(version, 5.0)) + experiment_result = await execute_experiment(store, run_via_experiment.id) + + assert runner_result.metrics == experiment_result.metrics + + runner_agreement = {r.row_id: r.agreement for r in store.list_results(run_via_runner.id)} + experiment_agreement = { + r.row_id: r.agreement for r in store.list_results(run_via_experiment.id) + } + assert runner_agreement == experiment_agreement + # Signed deltas, not booleans; at least one row has a non-zero delta. + assert any(v != 0 for v in experiment_agreement.values()) + + +# -- NumericDelta matches runner._agreement -------------------------------------- + + +@pytest.mark.parametrize( + "predicted,label", + [ + (5.0, 5.0), + (5.0, 3.0), + (3.0, 5.0), + (-2.0, 4.0), + (0.0, 0.0), + (10.5, 10.5), + (-1.5, -3.5), + ], +) +def test_numeric_delta_matches_agreement(predicted: float, label: float) -> None: + """``NumericDelta.evaluate`` is exactly the signed delta ``runner._agreement`` computes.""" + from types import SimpleNamespace + + from valcore.experiment import NumericDelta + from valcore.runner import _agreement + + ctx = SimpleNamespace(output=predicted, expected_output=label) + evaluator = NumericDelta() + + expected = _agreement(ScoreKind.NUMERIC, predicted, label) + assert evaluator.evaluate(ctx) == pytest.approx(expected) + # In particular, negative deltas survive (not, e.g., accidentally abs()'d). + if predicted < label: + assert evaluator.evaluate(ctx) < 0 + + +# -- Categorical agreement mirrors EqualsExpected -------------------------------- + + +@pytest.mark.anyio +async def test_categorical_agreement_is_exact_match( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + from pydantic_evals.evaluators.common import EqualsExpected + + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + evaluator_calls = capture_evaluators(monkeypatch) + patch_build_agent(monkeypatch, constant_agent(version, verdict="pass")) + result = await execute_experiment(store, run.id) + + # The construction itself carries EqualsExpected, not just the agreement values that + # get recomputed independently during persistence. + assert len(evaluator_calls) == 1 + (evaluators,) = evaluator_calls + assert len(evaluators) == 1 + assert isinstance(evaluators[0], EqualsExpected) + + assert result.status is RunStatus.COMPLETED + rows = store.list_rows(dataset.id) + results = {r.row_id: r for r in store.list_results(run.id)} + expected_agreement = [True, False, True, False, True] + for row, expected in zip(rows, expected_agreement, strict=True): + assert results[row.id].agreement is expected + assert isinstance(results[row.id].agreement, bool) + assert results[row.id].score_value == "pass" + assert result.metrics is not None + assert result.metrics["n"] == 5 + assert result.metrics["accuracy"] == pytest.approx(3 / 5) + + +@pytest.mark.anyio +async def test_numeric_run_attaches_numeric_delta( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """A numeric VALIDATION run's constructed dataset carries ``NumericDelta``, not ``EqualsExpected``. + + The two evaluators produce identical agreement only by coincidence on exact matches, + so this must be checked at construction time, not inferred from persisted results. + """ + from valcore.experiment import NumericDelta, execute_experiment + + version = make_numeric_version(store) + dataset = make_dataset(store, [1.0, 2.0], columns=["input", "output"], schema=NUMERIC_SCHEMA) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + evaluator_calls = capture_evaluators(monkeypatch) + patch_build_agent(monkeypatch, constant_numeric_agent(version, 1.0)) + await execute_experiment(store, run.id) + + assert len(evaluator_calls) == 1 + (evaluators,) = evaluator_calls + assert len(evaluators) == 1 + assert isinstance(evaluators[0], NumericDelta) + + +# -- EVAL runs attach no agreement evaluator ------------------------------------- + + +@pytest.mark.anyio +async def test_eval_run_has_no_agreement(store: Store, monkeypatch: pytest.MonkeyPatch) -> None: + """An EVAL-kind run computes no agreement even when the dataset carries labels.""" + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass"]) + run = store.create_run(RunKind.EVAL, version.id, dataset.id, concurrency=2) + + evaluator_calls = capture_evaluators(monkeypatch) + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id) + + # No agreement evaluator is attached at all -- not just "attached but ignored". + assert evaluator_calls == [[]] + + assert result.status is RunStatus.COMPLETED + assert result.metrics is None + results = store.list_results(run.id) + assert len(results) == 3 + assert all(r.agreement is None for r in results) + assert all(r.error is None for r in results) + + +# -- One result and one event per case ------------------------------------------- + + +@pytest.mark.anyio +async def test_one_result_and_one_event_per_case( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + events: list[RunEvent] = [] + + async def on_event(event: RunEvent) -> None: + events.append(event) + + patch_build_agent(monkeypatch, constant_agent(version)) + await execute_experiment(store, run.id, on_event=on_event) + + assert len(store.list_results(run.id)) == 3 + kinds = [e.type for e in events] + assert kinds.count("started") == 1 + assert kinds.count("finished") == 1 + assert kinds.count("row") == 3 + + +@pytest.mark.anyio +async def test_case_failure_is_recorded_not_raised( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """A failing case's ``teardown`` records an error rather than raising or being dropped.""" + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "pass", "pass", "pass", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + agent = flaky_agent(version, fail_on_call=3, error_message="boom on third row") + + events: list[RunEvent] = [] + + async def on_event(event: RunEvent) -> None: + events.append(event) + + patch_build_agent(monkeypatch, agent) + result = await execute_experiment(store, run.id, on_event=on_event) + + assert result.status is RunStatus.COMPLETED_WITH_ERRORS + results = store.list_results(run.id) + assert len(results) == 5 + errored = [r for r in results if r.error is not None] + assert len(errored) == 1 + assert "boom on third row" in errored[0].error + assert errored[0].output is None + assert [e.type for e in events].count("row") == 5 + # Metrics computed over the 4 successes only, matching the runner. + assert result.metrics is not None + assert result.metrics["n"] == 4 + + +@pytest.mark.anyio +async def test_teardown_none_records_error(store: Store) -> None: + """``teardown(None)`` -- the interrupted-case path -- records an error, not a raise. + + ``Dataset.evaluate`` calls ``teardown(None)`` only when the run is interrupted before a + report object exists for the case; there is no public way to trigger that through + ``execute_experiment`` (this engine has no cancellation), so ``PersistResults`` is driven + directly to exercise the branch the task/test plan calls out explicitly. + """ + from pydantic_evals import Case + + from valcore.experiment import PersistResults + + version = make_version(store) + dataset = make_dataset(store, ["pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + row = store.list_rows(dataset.id)[0] + + events: list[tuple[str, bool, str | float | None]] = [] + + async def emit_row(row_id: str, success: bool, score_value: str | float | None) -> None: + events.append((row_id, success, score_value)) + + case = Case(name=row.id, inputs=row.data, expected_output="pass") + lifecycle = PersistResults( + case, + store=store, + run_id=run.id, + version=version, + want_agreement=True, + rows_by_id={row.id: row}, + emit_row=emit_row, + ) + + await lifecycle.setup() + await lifecycle.teardown(None) + + results = store.list_results(run.id) + assert len(results) == 1 + assert results[0].error is not None + assert results[0].output is None + assert results[0].score_value is None + assert events == [(row.id, False, None)] + + +# -- ExperimentRun marker and cancellation --------------------------------------- + + +@pytest.mark.anyio +async def test_experiment_run_marker_written_and_blocks_cancel( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + assert store.get_experiment(run.id) is None + + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id) + + assert result.status is RunStatus.COMPLETED + experiment = store.get_experiment(run.id) + assert experiment is not None + assert experiment.experiment_name == version.version_name + assert experiment.case_count == 4 + + with pytest.raises(ContractError): + store.request_cancel(run.id) + + +@pytest.mark.anyio +async def test_request_cancel_raises_while_experiment_is_still_running( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """The ``ExperimentRun`` marker must exist before the run reaches ``RUNNING``. + + A marker written only after ``evaluate()`` completes would leave ``request_cancel`` + finding no row for the entire active run, silently accepting a cancellation that + ``evaluate()`` never polls -- exactly the race this finding closes. This blocks an + in-flight model call so the run is still ``RUNNING`` when ``request_cancel`` is called. + """ + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + release = asyncio.Event() + + class _BlockingTestModel(TestModel): + """A ``TestModel`` whose call blocks until released, keeping the run RUNNING.""" + + async def request(self, messages, model_settings, model_request_parameters): + await release.wait() + return await super().request(messages, model_settings, model_request_parameters) + + agent = Agent( + _BlockingTestModel(custom_output_args={"verdict": "pass"}), + output_type=build_output_model(version), + ) + patch_build_agent(monkeypatch, agent) + + task = asyncio.create_task(execute_experiment(store, run.id)) + try: + async with asyncio.timeout(5.0): + while store.get_run(run.id).status is not RunStatus.RUNNING: + await asyncio.sleep(0) + + with pytest.raises(ContractError): + store.request_cancel(run.id) + finally: + release.set() + result = await task + + assert result.status is RunStatus.COMPLETED + assert store.get_run(run.id).cancel_requested is False + + +@pytest.mark.anyio +async def test_experiment_run_case_count_excludes_failed_cases( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """``case_count`` reflects ``report.cases`` (successes), not the total row count.""" + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "pass", "pass", "pass", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + agent = flaky_agent(version, fail_on_call=2, error_message="boom") + + patch_build_agent(monkeypatch, agent) + result = await execute_experiment(store, run.id) + + assert result.status is RunStatus.COMPLETED_WITH_ERRORS + experiment = store.get_experiment(run.id) + assert experiment is not None + assert experiment.case_count == 4 + + +# -- Incompatible dataset --------------------------------------------------------- + + +@pytest.mark.anyio +async def test_incompatible_dataset_fails(store: Store, monkeypatch: pytest.MonkeyPatch) -> None: + from valcore.experiment import execute_experiment + + version = make_version(store) + # Dataset missing the required "output" column. + dataset = make_dataset(store, ["pass", "fail"], columns=["input"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id) + + assert result.status is RunStatus.FAILED + assert result.error is not None + assert "output" in result.error + assert store.list_results(run.id) == [] + assert store.get_experiment(run.id) is None + + +# -- Missing labels ----------------------------------------------------------------- + + +@pytest.mark.anyio +async def test_partially_unlabeled_validation_dataset_raises( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """A VALIDATION run must reject a dataset with even one unlabeled row. + + ``runner.execute_run`` raises ``ContractError`` before entering ``RUNNING`` for this + exact case; without the matching check here, this engine would silently evaluate the + unlabeled row, persist ``agreement=None`` for it, and quietly compute metrics over + only the labeled subset instead of refusing to run at all. + """ + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", None, "fail"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + patch_build_agent(monkeypatch, constant_agent(version)) + with pytest.raises(ContractError): + await execute_experiment(store, run.id) + + assert store.list_results(run.id) == [] + assert store.get_run(run.id).status is RunStatus.PENDING + assert store.get_experiment(run.id) is None + + +@pytest.mark.anyio +async def test_wholly_unlabeled_validation_dataset_raises( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """The same rejection holds when no row at all carries a label.""" + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, [None, None, None]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + patch_build_agent(monkeypatch, constant_agent(version)) + with pytest.raises(ContractError): + await execute_experiment(store, run.id) + + assert store.list_results(run.id) == [] + assert store.get_run(run.id).status is RunStatus.PENDING + + +@pytest.mark.anyio +async def test_eval_run_tolerates_unlabeled_rows( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """The missing-label check is VALIDATION-only -- an EVAL run has no labels by design.""" + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, [None, None, None]) + run = store.create_run(RunKind.EVAL, version.id, dataset.id, concurrency=2) + + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id) + + assert result.status is RunStatus.COMPLETED + + +# -- Unexpected lifecycle failures leave the run FAILED, not stuck RUNNING --------- + + +@pytest.mark.anyio +async def test_malformed_numeric_label_fails_run_not_stuck_running( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """A label that can't be coerced to a float breaks ``_agreement`` inside ``teardown``. + + ``pydantic_evals`` explicitly propagates a lifecycle's exceptions to ``evaluate()``'s + caller, so this must resolve to a terminal ``FAILED`` status with ``finished_at`` set + and an ``error`` event -- not leave the run permanently ``RUNNING``. + """ + from valcore.experiment import execute_experiment + + version = make_numeric_version(store) + dataset = store.create_dataset("ds", "", ["input", "output"], NUMERIC_SCHEMA) + rows = store.add_rows(dataset.id, [{"input": "in0", "output": "out0"}]) + store.set_label(rows[0].id, {"value": "not-a-number"}, LabelSource.MANUAL) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + events: list[RunEvent] = [] + + async def on_event(event: RunEvent) -> None: + events.append(event) + + patch_build_agent(monkeypatch, constant_numeric_agent(version, 1.0)) + result = await execute_experiment(store, run.id, on_event=on_event) + + assert result.status is RunStatus.FAILED + assert result.finished_at is not None + assert result.error is not None + assert [e.type for e in events].count("error") == 1 + + +@pytest.mark.anyio +async def test_event_callback_failure_fails_run_not_stuck_running( + store: Store, monkeypatch: pytest.MonkeyPatch +) -> None: + """An ``on_event`` callback that raises during a ``row`` event must not hang the run. + + ``teardown`` awaits the event callback directly, and the same "lifecycle exceptions + propagate" rule applies here -- the run must resolve to FAILED with ``finished_at`` + set rather than being abandoned mid-``RUNNING``. + """ + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + async def on_event(event: RunEvent) -> None: + if event.type == "row": + raise RuntimeError("callback exploded") + + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id, on_event=on_event) + + assert result.status is RunStatus.FAILED + assert result.finished_at is not None + assert result.error is not None + + +# -- Tracing ---------------------------------------------------------------------- +# +# execute_experiment wraps the transition to RUNNING through the terminal status +# update in tracing.run_span, exactly as runner.execute_run does. These tests prove +# the span closes with a status attribute both on the happy path and when execution +# fails after entering the span (eval-dataset construction, evaluate() itself). + + +@pytest.fixture(autouse=True) +def _reset_tracing_configured_guard(monkeypatch: pytest.MonkeyPatch) -> None: + """Reset tracing's module-global idempotency guard before every test in this file.""" + monkeypatch.setattr(tracing, "_configured", False, raising=False) + + +@pytest.fixture +def traced(capfire, monkeypatch: pytest.MonkeyPatch): + """Mark tracing as configured against ``capfire``'s in-memory exporter.""" + monkeypatch.setattr(tracing, "_configured", True) + return capfire + + +@pytest.mark.skipif(not _LOGFIRE_PRESENT, reason="logfire extra not installed") +class TestExperimentSpan: + """With tracing configured, execute_experiment must produce a real span tree.""" + + @pytest.mark.anyio + async def test_successful_run_span_carries_status_and_metrics( + self, store: Store, traced, monkeypatch: pytest.MonkeyPatch + ) -> None: + from valcore.experiment import execute_experiment + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + patch_build_agent(monkeypatch, constant_agent(version)) + result = await execute_experiment(store, run.id) + + assert result.metrics is not None + spans = traced.exporter.exported_spans_as_dict(parse_json_attributes=True) + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + attrs = run_spans[0]["attributes"] + assert attrs["status"] == result.status.value + for key, value in result.metrics.items(): + assert attrs[key] == value + + @pytest.mark.anyio + async def test_eval_dataset_construction_failure_closes_span_as_failed( + self, store: Store, traced, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A failure building the ``EvalsDataset`` happens inside the span (it needs + the agent and rows resolved during setup) and must still close the span with + ``status=failed`` rather than leaving it open or attribute-less.""" + import valcore.experiment as experiment_module + + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + def boom(dataset, rows, evaluators): + raise RuntimeError("dataset construction exploded") + + monkeypatch.setattr(experiment_module, "dataset_to_evals", boom) + patch_build_agent(monkeypatch, constant_agent(version)) + + result = await experiment_module.execute_experiment(store, run.id) + + assert result.status is RunStatus.FAILED + assert result.finished_at is not None + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + assert run_spans[0]["attributes"]["status"] == RunStatus.FAILED.value + + @pytest.mark.anyio + async def test_evaluate_failure_closes_span_as_failed( + self, store: Store, traced, monkeypatch: pytest.MonkeyPatch + ) -> None: + """A malformed label breaks ``_agreement`` inside ``teardown``, which + ``pydantic_evals`` propagates out of ``evaluate()`` itself -- the span must + still close with ``status=failed`` rather than staying open.""" + from valcore.experiment import execute_experiment + + version = make_numeric_version(store) + dataset = store.create_dataset("ds", "", ["input", "output"], NUMERIC_SCHEMA) + rows = store.add_rows(dataset.id, [{"input": "in0", "output": "out0"}]) + store.set_label(rows[0].id, {"value": "not-a-number"}, LabelSource.MANUAL) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + patch_build_agent(monkeypatch, constant_numeric_agent(version, 1.0)) + result = await execute_experiment(store, run.id) + + assert result.status is RunStatus.FAILED + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + assert run_spans[0]["attributes"]["status"] == RunStatus.FAILED.value diff --git a/tests/test_logfire_io.py b/tests/test_logfire_io.py new file mode 100644 index 0000000..d746018 --- /dev/null +++ b/tests/test_logfire_io.py @@ -0,0 +1,363 @@ +"""Tests for pushing datasets to Logfire's hosted store. + +Pins ``logfire_io.push_dataset``'s behavior: API key resolution (argument over config), the +``on_conflict`` -> ``on_case_conflict`` rename, the ``DatasetDetail`` -> dict shape (absent +optional keys become ``None``, no URL field), that a client failure surfaces as +``ContractError``, that the client's async context manager is entered and exited on both success +and failure (so its underlying ``httpx.AsyncClient`` is never leaked), and -- the one no +functional test would otherwise catch -- that it is the *async* client, not the blocking one, +that gets constructed and awaited. + +Every test stubs ``logfire.experimental.api_client`` by name so nothing here makes a network +call, needs a real API key, or requires the ``logfire`` extra to be installed. +""" + +import inspect +import sys +import types +import uuid +from dataclasses import dataclass, field +from typing import Self + +import pytest +from pydantic import TypeAdapter + +from valcore.config import FileConfig, save_config +from valcore.errors import ConfigError, ContractError +from valcore.models import Dataset as VDataset +from valcore.models import DatasetRow, LabelSource + + +def make_rows() -> list[DatasetRow]: + """One row labeled with a value from the dataset's categorical label space.""" + return [ + DatasetRow( + dataset_id="d1", + idx=0, + data={"question": "Q1", "answer": "A1"}, + label={"value": "a"}, + label_source=LabelSource.MANUAL, + ), + ] + + +def make_dataset() -> VDataset: + """A categorical dataset whose label enum must survive into the pushed schema.""" + return VDataset( + name="refusal-quality", + columns=["question", "answer"], + label_schema={"kind": "categorical", "labels": ["a", "b"]}, + ) + + +@dataclass +class _Recorder: + """Captures what ``push_dataset`` does to the stubbed ``AsyncLogfireAPIClient``.""" + + calls: list[dict] = field(default_factory=list) + detail: dict = field(default_factory=lambda: {"id": uuid.uuid4(), "name": "pushed"}) + error: Exception | None = None + sync_client_constructed: bool = False + entered: bool = False + exited: bool = False + exited_with_exc: bool = False + + +def _install_stub_client(monkeypatch: pytest.MonkeyPatch, recorder: _Recorder) -> None: + """Replace ``logfire.experimental.api_client`` with a stub module in ``sys.modules``. + + The real ``push_dataset`` must import that module lazily, inside the function body, so + inserting a fake module under its dotted name intercepts the import without needing a real + ``logfire`` install or any network access. + """ + + class StubAsyncClient: + def __init__(self, api_key: str | None = None) -> None: + self.api_key = api_key + + async def __aenter__(self) -> Self: + recorder.entered = True + return self + + async def __aexit__(self, exc_type: object, exc: object, tb: object) -> bool: + recorder.exited = True + recorder.exited_with_exc = exc_type is not None + return False + + async def push_dataset( + self, + dataset: object, + *, + name: str | None = None, + description: str | None = None, + on_case_conflict: str = "update", + ) -> dict: + recorder.calls.append( + { + "dataset": dataset, + "name": name, + "description": description, + "on_case_conflict": on_case_conflict, + "api_key": self.api_key, + } + ) + if recorder.error is not None: + raise recorder.error + return recorder.detail + + class StubSyncClient: + """Stands in for the blocking ``LogfireAPIClient``; must never be constructed.""" + + def __init__(self, *args: object, **kwargs: object) -> None: + recorder.sync_client_constructed = True + raise AssertionError("sync LogfireAPIClient must never be constructed") + + fake_module = types.ModuleType("logfire.experimental.api_client") + fake_module.AsyncLogfireAPIClient = StubAsyncClient # type: ignore[attr-defined] + fake_module.LogfireAPIClient = StubSyncClient # type: ignore[attr-defined] + monkeypatch.setitem(sys.modules, "logfire.experimental.api_client", fake_module) + + +@pytest.fixture +def recorder(monkeypatch: pytest.MonkeyPatch) -> _Recorder: + """A fresh recorder with the stub client installed for this test only.""" + rec = _Recorder() + _install_stub_client(monkeypatch, rec) + return rec + + +# --- API key resolution -------------------------------------------------------- + + +@pytest.mark.anyio +async def test_missing_api_key_raises_config_error_naming_command_and_scopes( + recorder: _Recorder, +) -> None: + from valcore.logfire_io import push_dataset + + with pytest.raises(ConfigError) as exc: + await push_dataset(make_dataset(), make_rows()) + message = str(exc.value) + assert "valcore config set-logfire-key" in message + assert "project:read_datasets" in message + assert "project:write_datasets" in message + assert recorder.calls == [] + + +@pytest.mark.anyio +async def test_api_key_argument_wins_over_config(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-from-file")) + await push_dataset(make_dataset(), make_rows(), api_key="lf-from-arg") + assert recorder.calls[0]["api_key"] == "lf-from-arg" + + +@pytest.mark.anyio +async def test_api_key_falls_back_to_config_when_argument_is_none(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-from-file")) + await push_dataset(make_dataset(), make_rows(), api_key=None) + assert recorder.calls[0]["api_key"] == "lf-from-file" + + +# --- generics fix is observable here ------------------------------------------- + + +@pytest.mark.anyio +async def test_pushed_dataset_output_schema_is_the_label_enum_not_empty( + recorder: _Recorder, +) -> None: + """Pins the point of the spec-generics fix: a bare ``object`` OutputT infers to ``{}``.""" + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows()) + + pushed = recorder.calls[0]["dataset"] + output_type = pushed.__class__.__pydantic_generic_metadata__["args"][1] + schema = TypeAdapter(output_type).json_schema() + assert schema == {"enum": ["a", "b"], "type": "string"} + assert schema != {} + assert pushed.evaluators == [] + + +# --- on_conflict -> on_case_conflict rename ------------------------------------ + + +@pytest.mark.anyio +async def test_on_conflict_is_threaded_through_as_on_case_conflict(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows(), on_conflict="error") + assert recorder.calls[0]["on_case_conflict"] == "error" + + +@pytest.mark.anyio +async def test_on_conflict_defaults_to_update(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows()) + assert recorder.calls[0]["on_case_conflict"] == "update" + + +# --- name / description passthrough -------------------------------------------- + + +@pytest.mark.anyio +async def test_name_and_description_are_passed_through(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows(), name="custom-name", description="custom-desc") + assert recorder.calls[0]["name"] == "custom-name" + assert recorder.calls[0]["description"] == "custom-desc" + + +# --- DatasetDetail -> dict shape ------------------------------------------------- + + +@pytest.mark.anyio +async def test_result_dict_has_none_for_absent_optional_fields(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + detail_id = uuid.uuid4() + recorder.detail = {"id": detail_id, "name": "pushed-dataset"} + + result = await push_dataset(make_dataset(), make_rows()) + + assert result == { + "id": str(detail_id), + "name": "pushed-dataset", + "case_count": None, + "output_schema": None, + } + assert isinstance(result["id"], str) + + +@pytest.mark.anyio +async def test_result_dict_carries_present_optional_fields(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + recorder.detail = { + "id": uuid.uuid4(), + "name": "pushed-dataset", + "case_count": 3, + "output_schema": {"type": "string"}, + } + + result = await push_dataset(make_dataset(), make_rows()) + + assert result["case_count"] == 3 + assert result["output_schema"] == {"type": "string"} + + +@pytest.mark.anyio +async def test_result_never_has_a_url_field(recorder: _Recorder) -> None: + """``DatasetDetail`` has no URL field; ``push_dataset`` must never synthesize one.""" + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + result = await push_dataset(make_dataset(), make_rows()) + assert "url" not in result + + +# --- client errors surface as ContractError -------------------------------------- + + +@pytest.mark.anyio +async def test_client_exception_surfaces_as_contract_error(recorder: _Recorder) -> None: + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + recorder.error = RuntimeError("under-scoped API key") + + with pytest.raises(ContractError, match="under-scoped API key"): + await push_dataset(make_dataset(), make_rows()) + + +# --- client context manager is always entered and exited -------------------------- + + +@pytest.mark.anyio +async def test_client_context_manager_is_entered_and_exited_on_success( + recorder: _Recorder, +) -> None: + """The client owns an ``httpx.AsyncClient``; its context manager must close it.""" + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows()) + + assert recorder.entered is True + assert recorder.exited is True + assert recorder.exited_with_exc is False + + +@pytest.mark.anyio +async def test_client_context_manager_is_exited_on_failure(recorder: _Recorder) -> None: + """Cleanup must still happen when the upload itself raises, not just on success.""" + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + recorder.error = RuntimeError("boom") + + with pytest.raises(ContractError): + await push_dataset(make_dataset(), make_rows()) + + assert recorder.entered is True + assert recorder.exited is True + assert recorder.exited_with_exc is True + + +# --- missing `logfire` extra ------------------------------------------------------- + + +@pytest.mark.anyio +async def test_missing_logfire_extra_raises_config_error_naming_it( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """Simulate the ``logfire`` extra being absent by poisoning its import. + + Setting a module to ``None`` in ``sys.modules`` is the standard way to force + ``ImportError`` on a subsequent ``import``/``from ... import`` of that dotted name, without + needing to actually uninstall anything. + """ + from valcore.logfire_io import push_dataset + + monkeypatch.setitem(sys.modules, "logfire.experimental.api_client", None) + save_config(FileConfig(logfire_api_key="lf-key")) + + with pytest.raises(ConfigError, match="logfire"): + await push_dataset(make_dataset(), make_rows()) + + +# --- the async client, never the sync one, is used -------------------------------- + + +@pytest.mark.anyio +async def test_uses_the_async_client_and_awaits_it(recorder: _Recorder) -> None: + """Guards against swapping in the sync ``LogfireAPIClient``. + + That would block the event loop for the whole upload inside the async FastAPI handler that + calls this -- a defect no purely functional test would catch, since the sync client would + still return a usable result. + """ + from valcore.logfire_io import push_dataset + + save_config(FileConfig(logfire_api_key="lf-key")) + await push_dataset(make_dataset(), make_rows()) + + # The stub's `push_dataset` body only runs -- appending to `recorder.calls` -- if it was + # actually awaited; an un-awaited coroutine would leave this empty. + assert len(recorder.calls) == 1 + assert recorder.sync_client_constructed is False + + stub_module = sys.modules["logfire.experimental.api_client"] + assert inspect.iscoroutinefunction(stub_module.AsyncLogfireAPIClient.push_dataset) + assert not inspect.iscoroutinefunction(stub_module.LogfireAPIClient.__init__) diff --git a/tests/test_models.py b/tests/test_models.py index 4dc827d..a19e7f7 100644 --- a/tests/test_models.py +++ b/tests/test_models.py @@ -7,6 +7,7 @@ from valcore.models import ( Dataset, EvaluatorVersion, + ExperimentRun, FieldType, LabelSchema, OutputField, @@ -309,3 +310,24 @@ def test_version_json_round_trip() -> None: validate_version(restored) fields = parse_output_fields(restored) assert [f.name for f in fields] == ["verdict"] + + +# -- ExperimentRun ------------------------------------------------------------ + + +def test_experiment_run_defaults() -> None: + """A run with no ExperimentRun row is a runner run; case_count defaults to 0.""" + experiment = ExperimentRun(run_id="run1", experiment_name="exp1") + + assert experiment.run_id == "run1" + assert experiment.experiment_name == "exp1" + assert experiment.case_count == 0 + assert experiment.id + assert experiment.created_at is not None + + +def test_experiment_run_ids_are_unique_per_instance() -> None: + first = ExperimentRun(run_id="run1", experiment_name="exp1") + second = ExperimentRun(run_id="run1", experiment_name="exp1") + + assert first.id != second.id diff --git a/tests/test_runner.py b/tests/test_runner.py index bfdc6df..2d5b9df 100644 --- a/tests/test_runner.py +++ b/tests/test_runner.py @@ -5,18 +5,22 @@ """ import asyncio +import importlib.util import pytest from pydantic_ai import Agent from pydantic_ai.messages import ModelMessage, ModelResponse, ToolCallPart from pydantic_ai.models.function import AgentInfo, FunctionModel +from valcore import tracing from valcore.errors import ContractError from valcore.factory import build_output_model from valcore.models import LabelSource, RunKind, RunStatus, ScoreKind from valcore.runner import RunEvent, execute_run from valcore.store import Store, create_engine, init_db +_LOGFIRE_PRESENT = importlib.util.find_spec("logfire") is not None + CATEGORICAL_SCHEMA = {"kind": "categorical", "labels": ["pass", "fail"]} VERSION_FIELDS = { @@ -354,3 +358,227 @@ async def on_event(event: RunEvent) -> None: # started carries the total row count. started = next(e for e in events if e.type == "started") assert started.payload["total"] == 3 + + +# -- Tracing -------------------------------------------------------------------- +# +# execute_run/_score_row wrap their bodies in valcore.tracing.run_span/row_span, which +# are no-op context managers when tracing is unconfigured. These tests prove two +# things: (1) an unconfigured process behaves identically to before this task, and +# (2) with logfire.testing capturing, the span tree, its attributes, and the +# no-behavior-change guarantees (cancellation, per-row errors, only_row_ids) all hold. + + +@pytest.fixture(autouse=True) +def _reset_tracing_configured_guard(monkeypatch: pytest.MonkeyPatch) -> None: + """Reset tracing's module-global idempotency guard before every test in this file. + + The guard is process-global, so a leftover True from another test module (or + from the `traced` fixture below) must never leak into a test that assumes + tracing is unconfigured. + """ + monkeypatch.setattr(tracing, "_configured", False, raising=False) + + +@pytest.fixture +def traced(capfire, monkeypatch: pytest.MonkeyPatch): + """Mark tracing as configured against ``capfire``'s in-memory exporter. + + ``capfire`` configures logfire's test exporter directly, bypassing + ``configure_tracing`` -- so ``tracing._configured`` must be forced to True or + ``run_span``/``row_span`` would (correctly) treat the process as unconfigured + and yield no-op spans instead of exercising the real span tree. + """ + monkeypatch.setattr(tracing, "_configured", True) + return capfire + + +def test_tracing_unconfigured_by_default() -> None: + assert tracing._configured is False + + +@pytest.mark.anyio +async def test_run_behavior_unchanged_with_tracing_unconfigured(store: Store) -> None: + """The happy-path scenario, run with tracing unconfigured, to prove identical + results and metrics -- i.e. wrapping the engine in spans changed no behavior.""" + assert tracing._configured is False + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=3) + + result = await execute_run(store, run.id, agent=constant_agent(version)) + + assert result.status is RunStatus.COMPLETED + results = store.list_results(run.id) + assert len(results) == 5 + by_row = {r.row_id: r for r in results} + rows = store.list_rows(dataset.id) + expected = [True, False, True, False, True] + for row, exp in zip(rows, expected, strict=True): + assert by_row[row.id].agreement is exp + assert by_row[row.id].score_value == "pass" + assert by_row[row.id].error is None + assert result.metrics is not None + assert result.metrics["n"] == 5 + assert result.metrics["accuracy"] == pytest.approx(3 / 5) + + +@pytest.mark.skipif(not _LOGFIRE_PRESENT, reason="logfire extra not installed") +class TestRunnerSpanTree: + """With tracing configured, execute_run/_score_row must produce a real span tree.""" + + @pytest.mark.anyio + async def test_run_span_wraps_one_row_span_per_row(self, store: Store, traced) -> None: + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + await execute_run(store, run.id, agent=constant_agent(version)) + + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + row_spans = [s for s in spans if s["name"] == "valcore.score_row"] + assert len(run_spans) == 1 + assert len(row_spans) == 3 + for row_data in row_spans: + assert row_data["parent"] == run_spans[0]["context"] + + @pytest.mark.anyio + async def test_run_span_carries_run_id_kind_status_and_metrics_keys( + self, store: Store, traced + ) -> None: + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=3) + + result = await execute_run(store, run.id, agent=constant_agent(version)) + + assert result.metrics is not None + spans = traced.exporter.exported_spans_as_dict(parse_json_attributes=True) + run_data = next(s for s in spans if s["name"] == "valcore.run") + attrs = run_data["attributes"] + assert attrs["run_id"] == run.id + assert attrs["kind"] == RunKind.VALIDATION.value + assert attrs["status"] == result.status.value + for key, value in result.metrics.items(): + assert attrs[key] == value + + @pytest.mark.anyio + async def test_cancelled_run_still_closes_span_and_reports_cancelled( + self, store: Store, traced + ) -> None: + version = make_version(store) + dataset = make_dataset(store, ["pass"] * 5) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + async def on_event(event: RunEvent) -> None: + if event.type == "row": + store.request_cancel(run.id) + + result = await execute_run(store, run.id, agent=constant_agent(version), on_event=on_event) + + assert result.status is RunStatus.CANCELLED + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + assert run_spans[0]["attributes"]["status"] == RunStatus.CANCELLED.value + # Fewer row spans than the dataset has rows: cancellation actually took effect. + row_spans = [s for s in spans if s["name"] == "valcore.score_row"] + assert 0 < len(row_spans) < 5 + + @pytest.mark.anyio + async def test_all_rows_erroring_still_closes_span_and_reports_completed_with_errors( + self, store: Store, traced + ) -> None: + version = make_version(store) + dataset = make_dataset(store, ["pass", "pass", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + def always_fail(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: + raise RuntimeError("always fails") + + agent = Agent(FunctionModel(always_fail), output_type=build_output_model(version)) + + result = await execute_run(store, run.id, agent=agent) + + assert result.status is RunStatus.COMPLETED_WITH_ERRORS + # No successful rows means no metrics -- the span-attribute code must not + # choke iterating a metrics dict that never got computed. + assert result.metrics is None + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + assert run_spans[0]["attributes"]["status"] == RunStatus.COMPLETED_WITH_ERRORS.value + row_spans = [s for s in spans if s["name"] == "valcore.score_row"] + assert len(row_spans) == 3 + + @pytest.mark.anyio + async def test_only_row_ids_emits_row_spans_only_for_requested_rows( + self, store: Store, traced + ) -> None: + version = make_version(store) + dataset = make_dataset(store, ["pass", "fail", "pass", "fail", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=2) + + await execute_run(store, run.id, agent=constant_agent(version)) + traced.exporter.clear() + + rows = store.list_rows(dataset.id) + retry_ids = [rows[0].id, rows[2].id] + await execute_run(store, run.id, agent=constant_agent(version), only_row_ids=retry_ids) + + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + row_spans = [s for s in spans if s["name"] == "valcore.score_row"] + assert len(run_spans) == 1 + assert len(row_spans) == 2 + assert {s["attributes"]["row_id"] for s in row_spans} == set(retry_ids) + + @pytest.mark.anyio + async def test_row_span_closes_and_carries_idx_on_the_exception_path( + self, store: Store, traced + ) -> None: + """A row that raises must still close its span with the right ``idx`` -- + the ``except Exception`` branch in ``_score_row`` runs inside the + ``with tracing.row_span(row):`` block, so an unclosed or attribute-less + span here would mean the span wraps only the try body, not the whole + function.""" + version = make_version(store) + dataset = make_dataset(store, ["pass", "pass", "pass"]) + run = store.create_run(RunKind.VALIDATION, version.id, dataset.id, concurrency=1) + + def always_fail(messages: list[ModelMessage], info: AgentInfo) -> ModelResponse: + raise RuntimeError("always fails") + + agent = Agent(FunctionModel(always_fail), output_type=build_output_model(version)) + await execute_run(store, run.id, agent=agent) + + spans = traced.exporter.exported_spans_as_dict() + row_spans = [s for s in spans if s["name"] == "valcore.score_row"] + assert len(row_spans) == 3 + for row_data in row_spans: + assert row_data["end_time"] is not None + assert {s["attributes"]["idx"] for s in row_spans} == {0, 1, 2} + + @pytest.mark.anyio + async def test_eval_kind_run_span_closes_with_no_agreement_evaluator( + self, store: Store, traced + ) -> None: + """An EVAL-kind run has no labels to compare against, so no metrics are + computed -- the run span must still open, close, and report a clean + ``status`` attribute rather than skipping tracing for this run kind.""" + version = make_version(store) + dataset = make_dataset(store, [None, None, None]) + run = store.create_run(RunKind.EVAL, version.id, dataset.id, concurrency=2) + result = await execute_run(store, run.id, agent=constant_agent(version)) + + assert result.status is RunStatus.COMPLETED + assert result.metrics is None + spans = traced.exporter.exported_spans_as_dict() + run_spans = [s for s in spans if s["name"] == "valcore.run"] + assert len(run_spans) == 1 + assert run_spans[0]["end_time"] is not None + assert run_spans[0]["attributes"]["kind"] == RunKind.EVAL.value + assert run_spans[0]["attributes"]["status"] == RunStatus.COMPLETED.value diff --git a/tests/test_spec.py b/tests/test_spec.py index a7a8c22..a3780bf 100644 --- a/tests/test_spec.py +++ b/tests/test_spec.py @@ -6,7 +6,10 @@ renderers and the importer depend on, before any of that code exists. """ +import json + import pytest +from pydantic import TypeAdapter from pydantic_ai.agent.spec import AgentSpec from pydantic_evals import Dataset as EvalsDataset from pydantic_evals.dataset import Case @@ -481,3 +484,132 @@ def test_spec_to_version_fields_reconstructs_a_valid_version() -> None: # And the rebuilt version re-encodes to the identical schema. assert output_fields_to_schema(rebuilt) == output_fields_to_schema(original) + + +# --- OutputT generic derived from the dataset's label schema ------------------ + + +def _resolved_output_type(evals: EvalsDataset) -> object: + """Pull the concrete ``OutputT`` pydantic resolved for a ``dataset_to_evals`` result. + + Pydantic records the arguments a generic model was parameterized with on the class, not + the instance, so the schema a hosted push would infer is read the same way here. + """ + return evals.__class__.__pydantic_generic_metadata__["args"][1] + + +def test_output_type_schema_is_label_enum_for_categorical_dataset() -> None: + dataset = VDataset( + name="d", + columns=["question", "answer"], + label_schema={"kind": "categorical", "labels": ["a", "b"]}, + ) + evals = dataset_to_evals(dataset, make_rows(), []) + schema = TypeAdapter(_resolved_output_type(evals)).json_schema() + assert schema == {"enum": ["a", "b"], "type": "string"} + + +def test_output_type_schema_is_number_for_numeric_dataset() -> None: + dataset = VDataset(name="d", columns=["question", "answer"], label_schema={"kind": "numeric"}) + evals = dataset_to_evals(dataset, make_rows(), []) + schema = TypeAdapter(_resolved_output_type(evals)).json_schema() + assert schema == {"type": "number"} + + +def test_output_type_schema_is_string_for_empty_label_schema() -> None: + dataset = VDataset(name="d", columns=["question", "answer"], label_schema={}) + evals = dataset_to_evals(dataset, make_rows(), []) + schema = TypeAdapter(_resolved_output_type(evals)).json_schema() + assert schema == {"type": "string"} + + +@pytest.mark.parametrize( + "label_schema", + [ + pytest.param({"kind": "categorical", "labels": ["a", "b"]}, id="categorical"), + pytest.param({"kind": "numeric"}, id="numeric"), + pytest.param({}, id="empty"), + ], +) +def test_output_type_schema_is_never_the_empty_schema(label_schema: dict) -> None: + # The bug this fixes: `object` infers to `{}`, which must never come back for any label kind. + dataset = VDataset(name="d", columns=["q"], label_schema=label_schema) + evals = dataset_to_evals(dataset, make_rows(), []) + assert TypeAdapter(_resolved_output_type(evals)).json_schema() != {} + + +# --- byte identity of the serialized dataset ----------------------------------- + + +def test_dataset_to_evals_serialization_is_byte_identical_to_object_generic() -> None: + """The generics fix is schema-only: the serialized dataset bytes must not change. + + Computes the ``[dict, object, dict]`` baseline from the same cases the fixed + implementation produces, rather than hardcoding a blob, so the comparison tracks the + real row mapping instead of a frozen snapshot of it. + """ + dataset = VDataset( + name="refusal-quality", + columns=["question", "answer"], + label_schema={"kind": "categorical", "labels": ["refusal", "partial", "answer"]}, + ) + rows = make_rows() + evaluators: list[dict] = [] + + fixed = dataset_to_evals(dataset, rows, evaluators) + baseline = EvalsDataset[dict, object, dict]( + name=dataset.name, cases=fixed.cases, evaluators=evaluators + ) + + fixed_bytes = json.dumps(fixed.model_dump(mode="json", by_alias=True), sort_keys=True) + baseline_bytes = json.dumps(baseline.model_dump(mode="json", by_alias=True), sort_keys=True) + assert fixed_bytes == baseline_bytes + + +# --- import path is unaffected by the OutputT change --------------------------- + + +def test_from_dict_still_parses_dataset_to_evals_output() -> None: + """The importer always reads back through the plain ``object`` generic (see ``config_io``); + the fix must not disturb that path. + """ + dataset = VDataset( + name="refusal-quality", + columns=["question", "answer"], + label_schema={"kind": "categorical", "labels": ["refusal", "partial", "answer"]}, + ) + evals = dataset_to_evals(dataset, make_rows(), []) + dumped = evals.model_dump(mode="json", by_alias=True) + + reloaded = EvalsDataset[dict, object, dict].from_dict(dumped) + assert reloaded.name == "refusal-quality" + assert reloaded.cases[0].inputs == {"question": "Q1", "answer": "A1"} + assert reloaded.cases[0].expected_output == "refusal" + + +def test_label_outside_categorical_set_still_serializes_and_reimports() -> None: + """Pins that ``OutputT`` is descriptive metadata only, never a validating constraint. + + ``expected_output="nope"`` against a declared ``["refusal", "answer"]`` label set must not + raise: hand-labeled data with a stray value must still export and re-import. + """ + dataset = VDataset( + name="d", + columns=["question", "answer"], + label_schema={"kind": "categorical", "labels": ["refusal", "answer"]}, + ) + rows = [ + DatasetRow( + dataset_id="d1", + idx=0, + data={"question": "Q1", "answer": "A1"}, + label={"value": "nope"}, + ) + ] + + evals = dataset_to_evals(dataset, rows, []) + assert evals.cases[0].expected_output == "nope" + + dumped = evals.model_dump(mode="json", by_alias=True) + reloaded = EvalsDataset[dict, object, dict].from_dict(dumped) + assert reloaded.cases[0].expected_output == "nope" diff --git a/tests/test_store.py b/tests/test_store.py index 8da2abb..ce7ee16 100644 --- a/tests/test_store.py +++ b/tests/test_store.py @@ -4,6 +4,7 @@ from pathlib import Path import pytest +from sqlmodel import select from valcore.errors import ( ContractError, @@ -16,6 +17,7 @@ DatasetGeneration, DatasetRow, EvaluatorVersion, + ExperimentRun, LabelSource, Run, RunKind, @@ -889,3 +891,103 @@ def test_init_db_adds_the_generation_table_to_an_existing_database(tmp_path) -> assert store.get_generation(dataset.id) is None store.set_generation(dataset.id, count=4, instructions="works") assert store.get_generation(dataset.id).count == 4 + + +# -- Experiment runs ----------------------------------------------------------- + + +def test_init_db_adds_the_experiment_run_table_to_an_existing_database(tmp_path) -> None: + """A database written before ``ExperimentRun`` existed gains it on the next start. + + ``ExperimentRun`` is a separate table rather than a ``Run`` column precisely because + ``init_db`` is a bare ``create_all``: it adds missing tables but never missing columns, + so a new table reaches an existing database while a new ``Run`` field would not. + """ + engine = create_engine(tmp_path / "existing.db") + init_db(engine) + store = Store(engine) + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + + # Simulate the older schema: the table simply is not there. + ExperimentRun.__table__.drop(engine) + + init_db(engine) + + # Existing data is untouched, and the new table is usable. + assert store.get_run(run.id).id == run.id + assert store.get_experiment(run.id) is None + store.set_experiment(run.id, experiment_name="exp1", case_count=2) + assert store.get_experiment(run.id).experiment_name == "exp1" + + +def test_experiment_absent_for_a_run_with_no_row(store: Store) -> None: + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + + assert store.get_experiment(run.id) is None + + +def test_set_experiment_then_get_experiment_round_trips(store: Store) -> None: + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + + store.set_experiment(run.id, experiment_name="nightly-eval", case_count=42) + + stored = store.get_experiment(run.id) + assert stored is not None + assert stored.run_id == run.id + assert stored.experiment_name == "nightly-eval" + assert stored.case_count == 42 + + +def test_set_experiment_replaces_rather_than_accumulates(store: Store) -> None: + """A second ``set_experiment`` call replaces the row rather than adding a second one.""" + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + + store.set_experiment(run.id, experiment_name="exp1", case_count=1) + store.set_experiment(run.id, experiment_name="exp2", case_count=2) + + stored = store.get_experiment(run.id) + assert stored.experiment_name == "exp2" + assert stored.case_count == 2 + + with session_scope(store.engine) as session: + rows = session.exec(select(ExperimentRun).where(ExperimentRun.run_id == run.id)).all() + assert len(rows) == 1 + + +def test_get_experiment_missing_run_raises(store: Store) -> None: + with pytest.raises(NotFoundError): + store.get_experiment("nope") + + +def test_set_experiment_missing_run_raises(store: Store) -> None: + with pytest.raises(NotFoundError): + store.set_experiment("nope", experiment_name="exp1", case_count=1) + + +def test_request_cancel_works_for_a_run_with_no_experiment_row(store: Store) -> None: + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + + cancelled = store.request_cancel(run.id) + + assert cancelled.cancel_requested is True + assert store.get_run(run.id).cancel_requested is True + + +def test_request_cancel_raises_for_an_experiment_run(store: Store) -> None: + version_id, dataset_id = _make_run_prereqs(store) + run = store.create_run(RunKind.EVAL, version_id, dataset_id, concurrency=1) + store.set_experiment(run.id, experiment_name="exp1", case_count=5) + + with pytest.raises(ContractError) as excinfo: + store.request_cancel(run.id) + + message = str(excinfo.value).lower() + assert "experiment" in message + assert "cannot be cancelled" in message + # Cancellation must not have been flagged despite the raised error. + assert store.get_run(run.id).cancel_requested is False diff --git a/tests/test_tracing.py b/tests/test_tracing.py new file mode 100644 index 0000000..782d067 --- /dev/null +++ b/tests/test_tracing.py @@ -0,0 +1,440 @@ +"""Tests for the Logfire tracing module: configuration, span shape, and warnings.""" + +import importlib.util +import warnings +from pathlib import Path + +import pytest + +from valcore import tracing +from valcore.config import FileConfig +from valcore.models import Dataset, DatasetRow, EvaluatorVersion, Run, RunKind, RunStatus, ScoreKind + +_LOGFIRE_PRESENT = importlib.util.find_spec("logfire") is not None + + +@pytest.fixture(autouse=True) +def _reset_tracing_state(monkeypatch: pytest.MonkeyPatch) -> None: + """Reset the idempotency guard and any exported Logfire env vars for each test. + + The guard is process-global (module-level), so without a reset the second test in + the file would silently observe the first test's "already configured" state. + """ + monkeypatch.setattr(tracing, "_configured", False, raising=False) + monkeypatch.delenv("LOGFIRE_TOKEN", raising=False) + + +def make_version(**overrides: object) -> EvaluatorVersion: + """Build a valid categorical EvaluatorVersion, applying any field overrides.""" + base: dict[str, object] = { + "evaluator_id": "ev1", + "version_name": "my eval", + "model": "gateway/anthropic:claude-sonnet-5", + "instructions": "You are an evaluator.", + "prompt_template": "Rate the answer to {question}.", + "required_columns": ["question"], + "output_fields": [ + { + "name": "verdict", + "type": "enum", + "description": "The verdict.", + "enum_values": ["pass", "fail"], + } + ], + "score_field": "verdict", + "score_kind": ScoreKind.CATEGORICAL, + "score_labels": ["pass", "fail"], + } + base.update(overrides) + return EvaluatorVersion.model_validate(base) + + +def make_dataset(**overrides: object) -> Dataset: + """Build a minimal Dataset, applying any field overrides.""" + base: dict[str, object] = {"name": "my dataset", "columns": ["question"]} + base.update(overrides) + return Dataset.model_validate(base) + + +def make_row(dataset_id: str, idx: int = 0, **overrides: object) -> DatasetRow: + """Build a minimal DatasetRow, applying any field overrides.""" + base: dict[str, object] = {"dataset_id": dataset_id, "idx": idx, "data": {"question": "hi"}} + base.update(overrides) + return DatasetRow.model_validate(base) + + +def make_run(version_id: str, dataset_id: str, **overrides: object) -> Run: + """Build a minimal Run, applying any field overrides.""" + base: dict[str, object] = { + "kind": RunKind.EVAL, + "version_id": version_id, + "dataset_id": dataset_id, + "status": RunStatus.RUNNING, + "concurrency": 4, + } + base.update(overrides) + return Run.model_validate(base) + + +class TestConfigureTracingSilentWithoutToken: + """No token configured must be a fully silent, working no-op path.""" + + def test_emits_no_warning(self) -> None: + cfg = FileConfig() + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracing.configure_tracing(cfg) + assert len(caught) == 0 + + def test_run_span_yields_without_raising(self) -> None: + cfg = FileConfig() + tracing.configure_tracing(cfg) + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + with tracing.run_span(run, version, dataset, row_count=1) as span: + assert span is not None + + def test_row_span_yields_without_raising(self) -> None: + cfg = FileConfig() + tracing.configure_tracing(cfg) + dataset = make_dataset() + row = make_row(dataset.id) + with tracing.row_span(row) as span: + assert span is not None + + def test_run_span_yields_even_without_configure_call(self) -> None: + """Callers need no conditionals: an unconfigured process still yields a span.""" + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + with tracing.run_span(run, version, dataset, row_count=1): + pass + + def test_row_span_yields_even_without_configure_call(self) -> None: + dataset = make_dataset() + row = make_row(dataset.id) + with tracing.row_span(row): + pass + + def test_run_span_does_not_call_logfire_span_when_unconfigured( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + """Calling logfire.span before configure() emits LogfireNotConfiguredWarning. + + Unconfigured spans must never reach logfire.span at all, not merely avoid + raising -- otherwise a process with the real package installed but no + configured token would print that warning on every run. + """ + calls: list[object] = [] + monkeypatch.setattr(tracing.logfire, "span", lambda *a, **k: calls.append((a, k))) + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + with tracing.run_span(run, version, dataset, row_count=1): + pass + assert calls == [] + assert len(caught) == 0 + + def test_row_span_does_not_call_logfire_span_when_unconfigured( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + calls: list[object] = [] + monkeypatch.setattr(tracing.logfire, "span", lambda *a, **k: calls.append((a, k))) + dataset = make_dataset() + row = make_row(dataset.id) + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + with tracing.row_span(row): + pass + assert calls == [] + assert len(caught) == 0 + + +class TestConfigureTracingTokenWithoutLogfire: + """A configured token with the ``logfire`` extra absent must warn exactly once.""" + + def _simulate_logfire_absent(self, monkeypatch: pytest.MonkeyPatch) -> None: + original_find_spec = importlib.util.find_spec + + def fake_find_spec(name: str, *args: object, **kwargs: object) -> object: + if name == "logfire": + return None + return original_find_spec(name, *args, **kwargs) + + monkeypatch.setattr(importlib.util, "find_spec", fake_find_spec) + + def test_warns_once_naming_the_extra(self, monkeypatch: pytest.MonkeyPatch) -> None: + self._simulate_logfire_absent(monkeypatch) + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: None) + cfg = FileConfig(logfire_token="lf-write-token") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracing.configure_tracing(cfg) + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + assert len(user_warnings) == 1 + assert "valcore[logfire]" in str(user_warnings[0].message) + + def test_does_not_warn_again_on_second_call(self, monkeypatch: pytest.MonkeyPatch) -> None: + self._simulate_logfire_absent(monkeypatch) + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: None) + cfg = FileConfig(logfire_token="lf-write-token") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracing.configure_tracing(cfg) + tracing.configure_tracing(cfg) + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + assert len(user_warnings) == 1 + + +@pytest.mark.skipif(not _LOGFIRE_PRESENT, reason="logfire extra not installed") +class TestConfigureTracingTokenWithLogfirePresent: + """With the real extra installed, a configured token must never warn.""" + + def test_no_warning_when_logfire_present(self, monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: None) + cfg = FileConfig(logfire_token="lf-write-token") + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracing.configure_tracing(cfg) + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + assert len(user_warnings) == 0 + + +class TestConfigureTracingIdempotent: + """configure_tracing must be safe to call twice without reconfiguring Logfire.""" + + def test_second_call_does_not_reconfigure(self, monkeypatch: pytest.MonkeyPatch) -> None: + calls: list[dict[str, object]] = [] + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: calls.append(kwargs)) + cfg = FileConfig() + tracing.configure_tracing(cfg) + tracing.configure_tracing(cfg) + assert len(calls) == 1 + + def test_configure_called_with_expected_arguments( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + calls: list[dict[str, object]] = [] + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: calls.append(kwargs)) + tracing.configure_tracing(FileConfig()) + assert len(calls) == 1 + assert calls[0]["send_to_logfire"] == "if-token-present" + assert calls[0]["service_name"] == "valcore" + assert calls[0]["console"] is False + + def test_applies_token_to_environment_before_configuring( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + monkeypatch.setattr(tracing.logfire, "configure", lambda **kwargs: None) + cfg = FileConfig(logfire_token="lf-from-config") + tracing.configure_tracing(cfg) + import os + + assert os.environ["LOGFIRE_TOKEN"] == "lf-from-config" + + def test_retries_after_a_failed_configure_call(self, monkeypatch: pytest.MonkeyPatch) -> None: + """A raising logfire.configure must not strand the module as permanently configured.""" + calls: list[dict[str, object]] = [] + + def failing_configure(**kwargs: object) -> None: + raise RuntimeError("boom") + + monkeypatch.setattr(tracing.logfire, "configure", failing_configure) + cfg = FileConfig() + with pytest.raises(RuntimeError): + tracing.configure_tracing(cfg) + assert tracing._configured is False + + def succeeding_configure(**kwargs: object) -> None: + calls.append(kwargs) + + monkeypatch.setattr(tracing.logfire, "configure", succeeding_configure) + tracing.configure_tracing(cfg) + assert tracing._configured is True + assert len(calls) == 1 + + +@pytest.mark.skipif(not _LOGFIRE_PRESENT, reason="logfire extra not installed") +class TestSpanTreeShape: + """The run span must be the parent of each row span, carrying the documented attributes.""" + + @pytest.fixture(autouse=True) + def _mark_configured(self, capfire, monkeypatch: pytest.MonkeyPatch) -> None: + """The capfire fixture configures logfire's test exporter directly, bypassing + configure_tracing -- so this module's own _configured guard must be set to + match, or run_span/row_span would (correctly) treat the process as unconfigured + and yield no-op spans instead of exercising the real span tree. + """ + monkeypatch.setattr(tracing, "_configured", True) + + def test_run_span_is_parent_of_row_span(self, capfire) -> None: + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + row = make_row(dataset.id, idx=0) + + with tracing.run_span(run, version, dataset, row_count=1) as span: + with tracing.row_span(row): + pass + span.set_attribute("status", "completed") + span.set_attribute("accuracy", 0.92) + + spans = {s["name"]: s for s in capfire.exporter.exported_spans_as_dict()} + assert "valcore.run" in spans + assert "valcore.score_row" in spans + + run_data = spans["valcore.run"] + row_data = spans["valcore.score_row"] + assert row_data["parent"] == run_data["context"] + + def test_run_span_carries_documented_attributes(self, capfire) -> None: + version = make_version(version_name="accuracy-check") + dataset = make_dataset(name="golden-set") + run = make_run(version.id, dataset.id, concurrency=7) + + with tracing.run_span(run, version, dataset, row_count=5) as span: + span.set_attribute("status", "completed") + + spans = {s["name"]: s for s in capfire.exporter.exported_spans_as_dict()} + attrs = spans["valcore.run"]["attributes"] + assert attrs["run_id"] == run.id + assert attrs["kind"] == RunKind.EVAL.value + assert attrs["version_id"] == version.id + assert attrs["version_name"] == "accuracy-check" + assert attrs["dataset_id"] == dataset.id + assert attrs["dataset_name"] == "golden-set" + assert attrs["row_count"] == 5 + assert attrs["concurrency"] == 7 + assert attrs["status"] == "completed" + + def test_run_span_carries_metrics_keys_set_before_close(self, capfire) -> None: + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + metrics = {"accuracy": 0.92, "agreement_rate": 0.87} + + with tracing.run_span(run, version, dataset, row_count=1) as span: + span.set_attribute("status", "completed") + for key, value in metrics.items(): + span.set_attribute(key, value) + + spans = {s["name"]: s for s in capfire.exporter.exported_spans_as_dict()} + attrs = spans["valcore.run"]["attributes"] + assert attrs["accuracy"] == 0.92 + assert attrs["agreement_rate"] == 0.87 + + def test_row_span_carries_row_id_and_idx(self, capfire) -> None: + dataset = make_dataset() + row = make_row(dataset.id, idx=3) + + with tracing.row_span(row): + pass + + spans = {s["name"]: s for s in capfire.exporter.exported_spans_as_dict()} + attrs = spans["valcore.score_row"]["attributes"] + assert attrs["row_id"] == row.id + assert attrs["idx"] == 3 + + def test_multiple_row_spans_share_the_same_run_parent(self, capfire) -> None: + version = make_version() + dataset = make_dataset() + run = make_run(version.id, dataset.id) + rows = [make_row(dataset.id, idx=i) for i in range(3)] + + with tracing.run_span(run, version, dataset, row_count=len(rows)) as span: + for row in rows: + with tracing.row_span(row): + pass + span.set_attribute("status", "completed") + + exported = capfire.exporter.exported_spans_as_dict() + run_data = next(s for s in exported if s["name"] == "valcore.run") + row_spans = [s for s in exported if s["name"] == "valcore.score_row"] + assert len(row_spans) == 3 + for row_data in row_spans: + assert row_data["parent"] == run_data["context"] + + +class TestNoLocalAgentInstrumentation: + """valcore must never instrument an agent locally -- the Gateway already reports.""" + + def test_no_source_file_references_instrument_pydantic_ai_or_instrument_all(self) -> None: + src_root = Path(__file__).resolve().parent.parent / "src" / "valcore" + offenders = [] + for path in src_root.rglob("*.py"): + text = path.read_text() + if "instrument_pydantic_ai" in text or "instrument_all" in text: + offenders.append(str(path)) + assert offenders == [], f"Found forbidden client-side instrumentation in: {offenders}" + + +class TestNoTokenStaysSilentRegardlessOfLogfirePresence: + """The opted-out path (no token) must never warn, even if logfire looks absent.""" + + def test_no_token_no_warning_when_logfire_simulated_absent( + self, monkeypatch: pytest.MonkeyPatch + ) -> None: + original_find_spec = importlib.util.find_spec + + def fake_find_spec(name: str, *args: object, **kwargs: object) -> object: + if name == "logfire": + return None + return original_find_spec(name, *args, **kwargs) + + monkeypatch.setattr(importlib.util, "find_spec", fake_find_spec) + cfg = FileConfig() + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + tracing.configure_tracing(cfg) + user_warnings = [w for w in caught if issubclass(w.category, UserWarning)] + assert len(user_warnings) == 0 + + +class TestSourceDoesNotImportLogfireDirectlyOrSwallowImportError: + """tracing.py must import the shim only, and never guard it with try/except ImportError.""" + + def _source_text(self) -> str: + module_path = Path(__file__).resolve().parent.parent / "src" / "valcore" / "tracing.py" + return module_path.read_text() + + def test_does_not_import_real_logfire_directly(self) -> None: + text = self._source_text() + for line in text.splitlines(): + stripped = line.strip() + assert not stripped.startswith("import logfire\n"), ( + "tracing.py must not import the real 'logfire' package directly" + ) + assert stripped != "import logfire", ( + "tracing.py must not import the real 'logfire' package directly" + ) + assert not stripped.startswith("from logfire "), ( + "tracing.py must not import from the real 'logfire' package directly" + ) + assert "import logfire_api as logfire" in text + + def test_does_not_use_try_except_import_error(self) -> None: + """Docstring prose may mention ImportError; only actual try/except blocks are forbidden.""" + import ast + + tree = ast.parse(self._source_text()) + offenders = [ + node + for node in ast.walk(tree) + if isinstance(node, ast.Try) + for handler in node.handlers + if handler.type is not None + and ( + (isinstance(handler.type, ast.Name) and handler.type.id == "ImportError") + or ( + isinstance(handler.type, ast.Tuple) + and any( + isinstance(elt, ast.Name) and elt.id == "ImportError" + for elt in handler.type.elts + ) + ) + ) + ] + assert offenders == [] diff --git a/uv.lock b/uv.lock index 3f2e226..eab4f13 100644 --- a/uv.lock +++ b/uv.lock @@ -77,6 +77,15 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/12/f6/5b8ec087cd9cfa9449491ec83f76fb6b7006b4dff57d2ba8aaab330fe8e4/argcomplete-3.7.0-py3-none-any.whl", hash = "sha256:d8f0f22d2a8a7caa383be1e22b6caf1ecaf0ebd10d8f83cc125e36540c95830c", size = 42575, upload-time = "2026-06-30T22:28:20.547Z" }, ] +[[package]] +name = "asgiref" +version = "3.12.1" +source = { registry = "https://pypi.org/simple" } +sdist = { url = "https://files.pythonhosted.org/packages/e6/26/3b59f2bdae5f640389becb1f673cded775287f5fc4f816309d9ca9a3f93d/asgiref-3.12.1.tar.gz", hash = "sha256:59dcb51c272ad209d59bed5708a64a333083e86017d7fcdd67498eeab7784340", size = 42378, upload-time = "2026-07-14T09:56:18.087Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/c0/1b/54f4ad77cd8a584fa70746c47df988e002cf1ee1eba43364d46f87803647/asgiref-3.12.1-py3-none-any.whl", hash = "sha256:fe386d1c2bff7259ea95929266d12a8cf9a8b5a1c2598402967d8792e7a7c094", size = 25478, upload-time = "2026-07-14T09:56:16.926Z" }, +] + [[package]] name = "attrs" version = "26.1.0" @@ -1130,6 +1139,9 @@ wheels = [ ] [package.optional-dependencies] +fastapi = [ + { name = "opentelemetry-instrumentation-fastapi" }, +] httpx = [ { name = "opentelemetry-instrumentation-httpx" }, ] @@ -1283,6 +1295,38 @@ wheels = [ { url = "https://files.pythonhosted.org/packages/40/7b/85eab1215f72adf0e68d3dc4a679b9bff993fa679ff34cd8dd378e2659fd/opentelemetry_instrumentation-0.65b0-py3-none-any.whl", hash = "sha256:ea967a72b9939b5fcfdad572753b4306c59dcb99e3f382d95dae04286805e137", size = 36717, upload-time = "2026-07-16T15:24:51.424Z" }, ] +[[package]] +name = "opentelemetry-instrumentation-asgi" +version = "0.65b0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "asgiref" }, + { name = "opentelemetry-api" }, + { name = "opentelemetry-instrumentation" }, + { name = "opentelemetry-semantic-conventions" }, + { name = "opentelemetry-util-http" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/17/83/8e8e83b7ac285281687c7be2fd305213ccccbb8c0a2dd4fb45a8ccaf12c7/opentelemetry_instrumentation_asgi-0.65b0.tar.gz", hash = "sha256:892bca67c56522ffa85a8a83cf934d7b50b3be2132e45cbee705825f0a5ba426", size = 26140, upload-time = "2026-07-16T15:25:54.544Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/0b/9c/376962840b619d2d55fe8ee2285f8c70971c090e5fff614516fc654a6f3a/opentelemetry_instrumentation_asgi-0.65b0-py3-none-any.whl", hash = "sha256:3a845a8ebd1c4ef0d8263401e6545f5b219b2feee612090d50f578a87e71fd65", size = 15903, upload-time = "2026-07-16T15:24:57.198Z" }, +] + +[[package]] +name = "opentelemetry-instrumentation-fastapi" +version = "0.65b0" +source = { registry = "https://pypi.org/simple" } +dependencies = [ + { name = "opentelemetry-api" }, + { name = "opentelemetry-instrumentation" }, + { name = "opentelemetry-instrumentation-asgi" }, + { name = "opentelemetry-semantic-conventions" }, + { name = "opentelemetry-util-http" }, +] +sdist = { url = "https://files.pythonhosted.org/packages/30/23/b057f8196d06efdc1b50e3ff11fbc499a7d96b35c87f217eb7885542f4ea/opentelemetry_instrumentation_fastapi-0.65b0.tar.gz", hash = "sha256:10a3a95486036230413a58fe4fdf4a83fa6bba46918407e527476994bd92bd97", size = 26236, upload-time = "2026-07-16T15:26:05.954Z" } +wheels = [ + { url = "https://files.pythonhosted.org/packages/fa/b0/c9b0300d33349ecc3dfd2362516eaffc44877e90970e6a52178ff953fec3/opentelemetry_instrumentation_fastapi-0.65b0-py3-none-any.whl", hash = "sha256:cda2610a0ec1b22d19886f33e4d861e9f5dbb886aeaa3a1263b47aff82c36943", size = 13261, upload-time = "2026-07-16T15:25:12.429Z" }, +] + [[package]] name = "opentelemetry-instrumentation-httpx" version = "0.65b0" @@ -2678,10 +2722,16 @@ dependencies = [ { name = "uvicorn", extra = ["standard"] }, ] +[package.optional-dependencies] +logfire = [ + { name = "logfire", extra = ["fastapi"] }, +] + [package.dev-dependencies] dev = [ { name = "anyio" }, { name = "httpx" }, + { name = "logfire", extra = ["fastapi"] }, { name = "pre-commit" }, { name = "pytest" }, { name = "pytest-asyncio" }, @@ -2693,6 +2743,7 @@ dev = [ requires-dist = [ { name = "click" }, { name = "fastapi" }, + { name = "logfire", extras = ["fastapi"], marker = "extra == 'logfire'", specifier = ">=4.39,<5" }, { name = "pydantic-ai", specifier = ">=2.19,<3" }, { name = "pydantic-ai-harness", extras = ["code-mode"], specifier = ">=0.12,<1" }, { name = "pydantic-settings" }, @@ -2700,11 +2751,13 @@ requires-dist = [ { name = "sqlmodel" }, { name = "uvicorn", extras = ["standard"] }, ] +provides-extras = ["logfire"] [package.metadata.requires-dev] dev = [ { name = "anyio" }, { name = "httpx" }, + { name = "logfire", extras = ["fastapi"], specifier = ">=4.39,<5" }, { name = "pre-commit", specifier = ">=4,<5" }, { name = "pytest" }, { name = "pytest-asyncio" }, diff --git a/web/src/api/client.test.ts b/web/src/api/client.test.ts index 89650c7..b9d7bfe 100644 --- a/web/src/api/client.test.ts +++ b/web/src/api/client.test.ts @@ -1,6 +1,6 @@ import { afterEach, describe, expect, it, vi } from "vitest"; -import { ApiError, api, datasets, evaluators, overview } from "./client"; -import type { Dataset, Overview } from "./types"; +import { ApiError, api, datasets, evaluators, overview, setup } from "./client"; +import type { Dataset, Overview, SetupStatus } from "./types"; function jsonResponse(body: unknown, init: { status?: number } = {}): Response { return new Response(JSON.stringify(body), { @@ -564,6 +564,74 @@ describe("overview client helper", () => { }); }); +// The setup endpoint is read-only: it reports which keys are configured (env or CLI) +// so the UI can gate actions that need the gateway key. There is no POST — keys are +// set only via the CLI — so this also guards against a write method sneaking in. +describe("setup client helper", () => { + it("setup.get GETs /api/setup and returns the parsed SetupStatus with all three keys", async () => { + const body: SetupStatus = { + keys: [ + { + name: "gateway_api_key", + set: true, + required: true, + label: "Pydantic AI Gateway API key", + command: "valcore setup gateway-key ", + purpose: "Required to run evaluators and generate datasets.", + }, + { + name: "logfire_token", + set: false, + required: false, + label: "Logfire write token", + command: "valcore setup logfire-token ", + purpose: "Sends run and row spans to your Logfire project.", + }, + { + name: "logfire_api_key", + set: false, + required: false, + label: "Logfire API key", + command: "valcore setup logfire-api-key ", + purpose: "Pushes datasets to Logfire's hosted store.", + }, + ], + }; + const fetchMock = vi.spyOn(globalThis, "fetch").mockResolvedValue(jsonResponse(body)); + + const result = await setup.get(); + + const [url, init] = fetchMock.mock.calls[0]; + expect(url).toBe("/api/setup"); + // No method means the default GET; setup is strictly read-only. + expect(init?.method ?? "GET").toBe("GET"); + expect(result).toEqual(body); + expect(result.keys).toHaveLength(3); + expect(result.keys.map((k) => k.name)).toEqual([ + "gateway_api_key", + "logfire_token", + "logfire_api_key", + ]); + }); + + it("setup.get surfaces a non-OK response as a rejection", async () => { + vi.spyOn(globalThis, "fetch").mockResolvedValue( + jsonResponse({ error: { type: "Error", message: "setup unavailable" } }, { status: 500 }), + ); + + const error = await setup.get().catch((e) => e); + + expect(error).toBeInstanceOf(ApiError); + if (!(error instanceof ApiError)) throw error; + expect(error.status).toBe(500); + expect(error.message).toBe("setup unavailable"); + }); + + it("exposes no method that issues a POST — keys are set only via the CLI", () => { + expect(Object.keys(setup).sort()).toEqual(["get"]); + }); +}); + // The dataset list items now carry row and label counts. This exercises that the // Dataset type accepts the two new required fields and that a list response round-trips // them unchanged. diff --git a/web/src/api/client.ts b/web/src/api/client.ts index 2575cdc..6f262d6 100644 --- a/web/src/api/client.ts +++ b/web/src/api/client.ts @@ -28,6 +28,7 @@ import type { ResultsPage, Run, RunStreamEvent, + SetupStatus, } from "./types"; export class ApiError extends Error { @@ -209,6 +210,11 @@ export const overview = { get: () => api("/api/overview"), }; +// Read-only: keys are set only via the CLI, so there is no write method here. +export const setup = { + get: () => api("/api/setup"), +}; + export const runs = { list: () => api("/api/runs"), get: (id: string) => api(`/api/runs/${id}`), diff --git a/web/src/api/types.ts b/web/src/api/types.ts index d7d9d09..11f6439 100644 --- a/web/src/api/types.ts +++ b/web/src/api/types.ts @@ -324,3 +324,19 @@ export interface RowsGenerate { label_mix?: LabelMix; label_guidance?: string; } + +// One credential the setup walkthrough checks for. `set` reflects effective +// presence (env or CLI-written config); `command` is the CLI invocation shown +// to the user when it is missing. +export interface SetupKey { + name: "gateway_api_key" | "logfire_token" | "logfire_api_key"; + set: boolean; + required: boolean; + label: string; + command: string; + purpose: string; +} + +export interface SetupStatus { + keys: SetupKey[]; +} diff --git a/web/src/components/DatasetFromEvaluator.test.tsx b/web/src/components/DatasetFromEvaluator.test.tsx index bb0261e..c20a1ae 100644 --- a/web/src/components/DatasetFromEvaluator.test.tsx +++ b/web/src/components/DatasetFromEvaluator.test.tsx @@ -8,13 +8,15 @@ // puts in the payload, and how it reacts to the label toggle — rather than the // editor's internals. -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; import DatasetFromEvaluator from "./DatasetFromEvaluator"; import { ApiError } from "../api/client"; import type { DatasetCreated, EvaluatorVersion } from "../api/types"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; // Hoisted so the mock factory (evaluated during import) can capture the same spy // the tests assert against. @@ -28,6 +30,11 @@ vi.mock("../api/client", async (importOriginal) => { }; }); +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + const navigate = vi.fn(); vi.mock("react-router-dom", async () => { @@ -35,6 +42,26 @@ vi.mock("react-router-dom", async () => { return { ...actual, useNavigate: () => navigate }; }); +const useSetupMock = vi.mocked(useSetup); + +/** Drives the mocked hook straight to a loaded state, skipping loading/error entirely. */ +function mockGatewayReady(gatewayReady: boolean): void { + const result: UseSetupResult = { + status: null, + gatewayReady, + loading: false, + error: null, + refetch: vi.fn(), + }; + useSetupMock.mockReturnValue(result); +} + +beforeEach(() => { + // Every pre-existing test in this file exercises form validity, not gateway gating, so + // the default keeps the key "present" and leaves their assertions undisturbed. + mockGatewayReady(true); +}); + // Stub of the fully-controlled ColumnNotesEditor. It echoes the props that matter // to these tests (the locked columns, the extras, whether adding is allowed) and // exposes two buttons that drive its controlled callbacks, standing in for a user @@ -398,3 +425,63 @@ describe("DatasetFromEvaluator chrome", () => { expect(screen.getByRole("tooltip").textContent).toMatch(/invent|named explicitly/i); }); }); + +describe("DatasetFromEvaluator gateway gating", () => { + it("disables Generate and shows the gateway blocker when the key is missing", async () => { + mockGatewayReady(false); + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Dataset name"), "Support QA"); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + expect(generateMock).not.toHaveBeenCalled(); + }); + + it("governs Generate by the form's own validity alone once the key is present", async () => { + // The unmodified "ready" case: naming the dataset with a present key enables Generate. + mockGatewayReady(true); + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Dataset name"), "Support QA"); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).not.toBeDisabled(); + }); + + it("shows the gateway blocker instead of a form-validity blocker when both apply", async () => { + // The row count also exceeds the cap, but a missing key blocks regardless of what else + // is wrong, and only one instruction is shown at a time. + mockGatewayReady(false); + const user = userEvent.setup(); + renderModal({ maxCount: 50 }); + + await user.type(screen.getByLabelText("Dataset name"), "Support QA"); + const count = screen.getByLabelText("Row count"); + await user.clear(count); + await user.type(count, "999"); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.queryByText(/must be 50 or fewer/i)).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); + + it("keeps the dataset name, instructions, and column editing usable while the key is missing", async () => { + mockGatewayReady(false); + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Dataset name"), "Support QA"); + await user.type(screen.getByLabelText("Instructions"), "half should be edge cases"); + await user.click(screen.getByRole("button", { name: "add extra column" })); + await user.click(screen.getByRole("button", { name: "set answer note" })); + + expect(screen.getByLabelText("Dataset name")).toHaveValue("Support QA"); + expect(screen.getByLabelText("Instructions")).toHaveValue("half should be edge cases"); + expect(screen.getByTestId("extra-column")).toHaveTextContent("context"); + // The form is now internally valid, yet the missing key still blocks the action. + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); +}); diff --git a/web/src/components/DatasetFromEvaluator.tsx b/web/src/components/DatasetFromEvaluator.tsx index 785fae4..4802487 100644 --- a/web/src/components/DatasetFromEvaluator.tsx +++ b/web/src/components/DatasetFromEvaluator.tsx @@ -14,6 +14,7 @@ import { TOTAL_PERCENT, toProportions, totalPercent } from "./labelMix"; import type { LabelMixPercents } from "./labelMix"; import { Button, ErrorBanner, Modal, Spinner } from "./ui"; import { Tooltip } from "./Tooltip"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; type DatasetFromEvaluatorProps = { open: boolean; @@ -33,6 +34,7 @@ export default function DatasetFromEvaluator({ onClose, }: DatasetFromEvaluatorProps) { const navigate = useNavigate(); + const { gatewayReady } = useSetup(); const [name, setName] = useState(""); const [instructions, setInstructions] = useState(""); const [extraColumns, setExtraColumns] = useState([]); @@ -57,7 +59,13 @@ export default function DatasetFromEvaluator({ // key sets by exact equality), and an over-count request would only fail server-side, so // both are blocked here rather than surfaced late. const countExceeds = count > maxCount; - const canSubmit = name.trim() !== "" && !countExceeds && !mixIncomplete; + const canSubmit = name.trim() !== "" && !countExceeds && !mixIncomplete && gatewayReady; + + // One reason at a time, mirroring the neighbouring generate forms' blockers idiom — + // a missing key blocks regardless of what else is wrong, so it takes precedence. + const blockers: string[] = []; + if (!gatewayReady) blockers.push(GATEWAY_BLOCKER); + if (countExceeds) blockers.push(`Row count must be ${maxCount} or fewer.`); async function submit() { if (!canSubmit) { @@ -207,9 +215,7 @@ export default function DatasetFromEvaluator({ /> - {countExceeds && ( -

Row count must be {maxCount} or fewer.

- )} + {blockers.length > 0 &&

{blockers[0]}

} ); diff --git a/web/src/components/DatasetGenerateForm.test.tsx b/web/src/components/DatasetGenerateForm.test.tsx index 2483a34..d50a96e 100644 --- a/web/src/components/DatasetGenerateForm.test.tsx +++ b/web/src/components/DatasetGenerateForm.test.tsx @@ -3,12 +3,14 @@ // the schema the user is editing, and it reaches the API as proportions rather than // percents; `instructions` are omitted when blank so `description` keeps driving the prompt. -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import DatasetGenerateForm from "./DatasetGenerateForm"; import { datasets } from "../api/client"; import type { DatasetCreated } from "../api/types"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; vi.mock("../api/client", async (importOriginal) => { const actual = await importOriginal(); @@ -18,7 +20,31 @@ vi.mock("../api/client", async (importOriginal) => { }; }); +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + const generateMock = vi.mocked(datasets.generate); +const useSetupMock = vi.mocked(useSetup); + +/** Drives the mocked hook straight to a loaded state, skipping loading/error entirely. */ +function mockGatewayReady(gatewayReady: boolean): void { + const result: UseSetupResult = { + status: null, + gatewayReady, + loading: false, + error: null, + refetch: vi.fn(), + }; + useSetupMock.mockReturnValue(result); +} + +beforeEach(() => { + // Every pre-existing test in this file exercises form validity, not gateway gating, so + // the default keeps the key "present" and leaves their assertions undisturbed. + mockGatewayReady(true); +}); function madeCreated(): DatasetCreated { return { @@ -473,3 +499,55 @@ describe("DatasetGenerateForm guidance", () => { expect(screen.getByText(/"label"/)).toBeInTheDocument(); }); }); + +describe("DatasetGenerateForm gateway gating", () => { + it("disables Generate and shows the gateway blocker when the key is missing", () => { + mockGatewayReady(false); + render(); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); + + it("governs Generate by the form's own validity alone once the key is present", async () => { + // The unmodified "ready" case: filling the essentials with a present key enables Generate. + mockGatewayReady(true); + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), "Synth"); + await user.type(screen.getByLabelText("Description"), "support questions"); + await user.type(screen.getByLabelText("Columns (comma separated)"), "question"); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(screen.queryByRole("status")).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).not.toBeDisabled(); + }); + + it("shows the gateway blocker instead of a form-validity blocker when both apply", () => { + // The form is also missing its name, but a missing key blocks regardless of what else + // is missing, and only one instruction is shown at a time. + mockGatewayReady(false); + render(); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.queryByText("Add a name")).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); + + it("keeps name, description, and column fields editable while the key is missing", async () => { + mockGatewayReady(false); + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Name"), "Synth"); + await user.type(screen.getByLabelText("Description"), "support questions"); + await user.type(screen.getByLabelText("Columns (comma separated)"), "question"); + + expect(screen.getByLabelText("Name")).toHaveValue("Synth"); + expect(screen.getByLabelText("Description")).toHaveValue("support questions"); + expect(screen.getByLabelText("Columns (comma separated)")).toHaveValue("question"); + // The form is now internally valid, yet the missing key still blocks the action. + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); +}); diff --git a/web/src/components/DatasetGenerateForm.tsx b/web/src/components/DatasetGenerateForm.tsx index c9e37ef..aa18fe3 100644 --- a/web/src/components/DatasetGenerateForm.tsx +++ b/web/src/components/DatasetGenerateForm.tsx @@ -8,6 +8,7 @@ import type { LabelSchema } from "../api/types"; import { Button, ErrorBanner, Spinner } from "./ui"; import { Tooltip } from "./Tooltip"; import { FormFooter } from "./FormFooter"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; import LabelSchemaEditor from "./LabelSchemaEditor"; import ColumnNotesEditor from "./ColumnNotesEditor"; import LabelMixEditor from "./LabelMixEditor"; @@ -44,6 +45,7 @@ const INSTRUCTIONS_HINT = "from the description alone."; export default function DatasetGenerateForm({ onCreated, initial }: DatasetGenerateFormProps) { + const { gatewayReady } = useSetup(); const [name, setName] = useState(initial?.name ?? ""); const [description, setDescription] = useState(initial?.description ?? ""); const [instructions, setInstructions] = useState(initial?.instructions ?? ""); @@ -72,6 +74,7 @@ export default function DatasetGenerateForm({ onCreated, initial }: DatasetGener // through FormFooter one instruction at a time. The gating is unchanged: the button stays // disabled while any blocker remains, i.e. exactly when the old `canSubmit` was false. const blockers: string[] = []; + if (!gatewayReady) blockers.push(GATEWAY_BLOCKER); if (name.trim() === "") blockers.push("Add a name"); if (description.trim() === "") blockers.push("Add a description"); if (columns.length === 0) blockers.push("Add at least one column"); @@ -259,7 +262,7 @@ export default function DatasetGenerateForm({ onCreated, initial }: DatasetGener } > - diff --git a/web/src/components/EvaluatorFromDataset.test.tsx b/web/src/components/EvaluatorFromDataset.test.tsx index 67edeec..e47ae4d 100644 --- a/web/src/components/EvaluatorFromDataset.test.tsx +++ b/web/src/components/EvaluatorFromDataset.test.tsx @@ -7,11 +7,13 @@ // exercise the real ColumnNotesEditor so the locked-column / no-add-control behaviour is // verified end to end rather than stubbed. -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import EvaluatorFromDataset from "./EvaluatorFromDataset"; import { ApiError, evaluators } from "../api/client"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; import type { Dataset, GeneratedConfig, LabelSchema } from "../api/types"; vi.mock("../api/client", async (importOriginal) => { @@ -29,9 +31,30 @@ vi.mock("../api/client", async (importOriginal) => { }; }); +// Generation needs the gateway key as much as a run does; the hook is mocked directly +// (rather than driving it through `../api/client`'s `setup.get`) so each test can set +// `gatewayReady` without re-exercising useSetup's own fetch/loading machinery, which has +// its own dedicated suite in useSetup.test.tsx. +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + const generateMock = vi.mocked(evaluators.generate); const createMock = vi.mocked(evaluators.create); const createVersionMock = vi.mocked(evaluators.createVersion); +const useSetupMock = vi.mocked(useSetup); + +function makeSetupResult(overrides: Partial = {}): UseSetupResult { + return { + status: null, + gatewayReady: true, + loading: false, + error: null, + refetch: vi.fn(), + ...overrides, + }; +} const LABELLED_SCHEMA: LabelSchema = { kind: "categorical", @@ -113,6 +136,10 @@ afterEach(() => { vi.clearAllMocks(); }); +beforeEach(() => { + useSetupMock.mockReturnValue(makeSetupResult()); +}); + describe("EvaluatorFromDataset", () => { it("renders the dataset's columns locked with no control for adding columns", () => { renderModal({ dataset: madeDataset({ columns: ["question", "answer"] }) }); @@ -288,3 +315,62 @@ describe("EvaluatorFromDataset chrome", () => { expect(onClose).toHaveBeenCalled(); }); }); + +// -- Gateway gating ----------------------------------------------------------- +// Generation needs the Pydantic AI Gateway key as much as running a version does, so it +// is gated the same way: the shared GATEWAY_BLOCKER text and a disabled primary action +// when the key is not set, and no change at all to today's behavior once it is. + +describe("EvaluatorFromDataset gateway gating", () => { + it("disables Generate and shows the shared gateway blocker when the gateway key is unset", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + const user = userEvent.setup(); + renderModal(); + + await user.type(screen.getByLabelText("Criteria"), "Judge answer quality."); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Generate evaluator" })).toBeDisabled(); + expect(generateMock).not.toHaveBeenCalled(); + }); + + it("does not call generate if Generate is somehow invoked while the gateway is blocked", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + renderModal(); + + // Disabled buttons swallow user-event clicks by design; assert directly on + // generateMock so this test does not depend on that browser behaviour. + expect(generateMock).not.toHaveBeenCalled(); + }); + + it("shows no gateway blocker and governs Generate only by criteria validity when the gateway is ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + const user = userEvent.setup(); + renderModal(); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(screen.getByRole("button", { name: "Generate evaluator" })).toBeDisabled(); + + await user.type(screen.getByLabelText("Criteria"), "Judge answer quality."); + expect(screen.getByRole("button", { name: "Generate evaluator" })).not.toBeDisabled(); + }); + + it("re-enables Generate once gatewayReady flips true with criteria already filled", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + const user = userEvent.setup(); + const { rerender } = render( + , + ); + + await user.type(screen.getByLabelText("Criteria"), "Judge answer quality."); + expect(screen.getByRole("button", { name: "Generate evaluator" })).toBeDisabled(); + + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + rerender( + , + ); + + expect(screen.getByRole("button", { name: "Generate evaluator" })).not.toBeDisabled(); + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + }); +}); diff --git a/web/src/components/EvaluatorFromDataset.tsx b/web/src/components/EvaluatorFromDataset.tsx index cca4a87..506be27 100644 --- a/web/src/components/EvaluatorFromDataset.tsx +++ b/web/src/components/EvaluatorFromDataset.tsx @@ -12,6 +12,7 @@ import { evaluators } from "../api/client"; import type { Dataset, GeneratedConfig, LabelSchema } from "../api/types"; import { Button, ErrorBanner, Modal, TextArea } from "./ui"; import { ColumnNotesEditor } from "./ColumnNotesEditor"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; type EvaluatorFromDatasetProps = { open: boolean; @@ -36,10 +37,11 @@ export function EvaluatorFromDataset({ const [notes, setNotes] = useState>({}); const [error, setError] = useState(null); const [submitting, setSubmitting] = useState(false); + const { gatewayReady } = useSetup(); const schema = dataset.label_schema; const hasLabelSpace = declaresLabelSpace(schema); - const canSubmit = criteria.trim() !== "" && !submitting; + const canSubmit = criteria.trim() !== "" && !submitting && gatewayReady; const submit = async () => { if (!canSubmit) return; @@ -72,6 +74,7 @@ export function EvaluatorFromDataset({ onClose={onClose} footer={
+ {!gatewayReady && {GATEWAY_BLOCKER}} diff --git a/web/src/components/GenerateMoreRows.test.tsx b/web/src/components/GenerateMoreRows.test.tsx index 3df9ce4..2a15dfc 100644 --- a/web/src/components/GenerateMoreRows.test.tsx +++ b/web/src/components/GenerateMoreRows.test.tsx @@ -1,12 +1,14 @@ // Tests for the top-up modal. Coverage focuses on the prefill (stored settings become the // starting point) and on shape being fixed by the dataset rather than the form. -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import GenerateMoreRows from "./GenerateMoreRows"; import { datasets } from "../api/client"; import type { Dataset, DatasetGeneration, DatasetRow } from "../api/types"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; vi.mock("../api/client", async (importOriginal) => { const actual = await importOriginal(); @@ -16,7 +18,31 @@ vi.mock("../api/client", async (importOriginal) => { }; }); +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + const generateRowsMock = vi.mocked(datasets.generateRows); +const useSetupMock = vi.mocked(useSetup); + +/** Drives the mocked hook straight to a loaded state, skipping loading/error entirely. */ +function mockGatewayReady(gatewayReady: boolean): void { + const result: UseSetupResult = { + status: null, + gatewayReady, + loading: false, + error: null, + refetch: vi.fn(), + }; + useSetupMock.mockReturnValue(result); +} + +beforeEach(() => { + // Every pre-existing test in this file exercises form validity, not gateway gating, so + // the default keeps the key "present" and leaves their assertions undisturbed. + mockGatewayReady(true); +}); afterEach(() => { cleanup(); @@ -297,3 +323,57 @@ describe("GenerateMoreRows chrome", () => { expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); }); }); + +describe("GenerateMoreRows gateway gating", () => { + it("disables Generate and shows the gateway blocker when the key is missing", () => { + mockGatewayReady(false); + renderModal(); + + const blocker = screen.getByRole("status"); + expect(blocker.textContent).toBe(GATEWAY_BLOCKER); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); + + it("governs Generate by the form's own validity alone once the key is present", () => { + // The unmodified "ready" case: a satisfiable form with a present key carries no status + // region and the primary action is live. + mockGatewayReady(true); + renderModal(); + + expect(screen.queryByRole("status")).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).not.toBeDisabled(); + }); + + it("shows the gateway blocker instead of the row-count blocker when both apply", async () => { + // The row count also exceeds the cap, but a missing key blocks regardless of what else + // is wrong, and FormFooter shows only one instruction at a time. + mockGatewayReady(false); + const user = userEvent.setup(); + renderModal({ maxCount: 50 }); + + const count = screen.getByLabelText("Rows to add"); + await user.clear(count); + await user.type(count, "51"); + + const blocker = screen.getByRole("status"); + expect(blocker.textContent).toBe(GATEWAY_BLOCKER); + expect(screen.queryByText(/must be 50 or fewer/i)).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); + + it("keeps Rows to add and Instructions editable while the key is missing", async () => { + mockGatewayReady(false); + const user = userEvent.setup(); + renderModal({ generation: null }); + + const count = screen.getByLabelText("Rows to add"); + await user.clear(count); + await user.type(count, "5"); + await user.type(screen.getByLabelText("Instructions"), "be subtle"); + + expect(count).toHaveValue(5); + expect(screen.getByLabelText("Instructions")).toHaveValue("be subtle"); + // Still blocked by the missing key even though the form itself is now valid. + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + }); +}); diff --git a/web/src/components/GenerateMoreRows.tsx b/web/src/components/GenerateMoreRows.tsx index 6d00f0d..05ede81 100644 --- a/web/src/components/GenerateMoreRows.tsx +++ b/web/src/components/GenerateMoreRows.tsx @@ -12,6 +12,7 @@ import { TOTAL_PERCENT, fromProportions, toProportions, totalPercent } from "./l import type { LabelMixPercents } from "./labelMix"; import { Button, ErrorBanner, Modal, Spinner } from "./ui"; import { FormFooter } from "./FormFooter"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; type GenerateMoreRowsProps = { open: boolean; @@ -32,6 +33,7 @@ export default function GenerateMoreRows({ onGenerated, onClose, }: GenerateMoreRowsProps) { + const { gatewayReady } = useSetup(); const [count, setCount] = useState(DEFAULT_COUNT); const [instructions, setInstructions] = useState(""); const [notes, setNotes] = useState>({}); @@ -67,6 +69,7 @@ export default function GenerateMoreRows({ // One reason at a time, so a blocked Generate says why instead of sitting silently // disabled. Order mirrors the fields top to bottom. const blockers: string[] = []; + if (!gatewayReady) blockers.push(GATEWAY_BLOCKER); if (count < 1) blockers.push("Add at least one row."); if (countExceeds) blockers.push(`Rows to add must be ${maxCount} or fewer.`); if (mixIncomplete) blockers.push("The label mix must total 100%."); @@ -109,7 +112,7 @@ export default function GenerateMoreRows({ - diff --git a/web/src/components/RefinePanel.test.tsx b/web/src/components/RefinePanel.test.tsx index b80559b..93faf96 100644 --- a/web/src/components/RefinePanel.test.tsx +++ b/web/src/components/RefinePanel.test.tsx @@ -1,8 +1,10 @@ import { render, screen } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { RefinePanel } from "./RefinePanel"; import { evaluators } from "../api/client"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; import type { GeneratedConfig } from "../api/types"; vi.mock("../api/client", () => ({ @@ -11,6 +13,27 @@ vi.mock("../api/client", () => ({ }, })); +// Refining calls the model through the gateway, so it is gated the same way generation +// and runs are; the hook is mocked directly rather than driven through a fetch, matching +// the pattern used by the other gated forms' test suites. +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + +const useSetupMock = vi.mocked(useSetup); + +function makeSetupResult(overrides: Partial = {}): UseSetupResult { + return { + status: null, + gatewayReady: true, + loading: false, + error: null, + refetch: vi.fn(), + ...overrides, + }; +} + const config: GeneratedConfig = { name: "Judge", version_name: "v1", @@ -42,6 +65,10 @@ afterEach(() => { vi.clearAllMocks(); }); +beforeEach(() => { + useSetupMock.mockReturnValue(makeSetupResult()); +}); + describe("RefinePanel", () => { it("does nothing when the instruction is empty", async () => { const user = userEvent.setup(); @@ -87,3 +114,42 @@ describe("RefinePanel", () => { expect(onApply).toHaveBeenCalledWith({ instructions: "Be stricter." }); }); }); + +// -- Gateway gating ----------------------------------------------------------- +// Refining calls a model through the gateway, so it needs the key exactly as generation +// and run-launching do. + +describe("RefinePanel gateway gating", () => { + it("disables Refine and shows the shared gateway blocker when the gateway key is unset", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + const user = userEvent.setup(); + render(); + + await user.type(screen.getByLabelText("Refine instruction"), "make it stricter"); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Refine" })).toBeDisabled(); + expect(evaluators.refine).not.toHaveBeenCalled(); + }); + + it("shows no gateway blocker and keeps existing behavior when the gateway is ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + vi.mocked(evaluators.refine).mockResolvedValue({ + config: { ...config, instructions: "Be stricter." }, + changed_fields: ["instructions"], + summary: "", + }); + const user = userEvent.setup(); + render(); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(screen.getByRole("button", { name: "Refine" })).not.toBeDisabled(); + + await user.type(screen.getByLabelText("Refine instruction"), "make it stricter"); + await user.click(screen.getByRole("button", { name: "Refine" })); + + expect(evaluators.refine).toHaveBeenCalledWith( + expect.objectContaining({ config, instruction: "make it stricter" }), + ); + }); +}); diff --git a/web/src/components/RefinePanel.tsx b/web/src/components/RefinePanel.tsx index 3c6da25..7b18af9 100644 --- a/web/src/components/RefinePanel.tsx +++ b/web/src/components/RefinePanel.tsx @@ -7,6 +7,7 @@ import { evaluators } from "../api/client"; import type { GeneratedConfig } from "../api/types"; import { ConfigDiff } from "./ConfigDiff"; import { Button, ErrorBanner, Spinner, TextArea } from "./ui"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; type RefinePanelProps = { config: GeneratedConfig; @@ -18,6 +19,7 @@ export function RefinePanel({ config, onApply }: RefinePanelProps) { const [refining, setRefining] = useState(false); const [error, setError] = useState(null); const [diff, setDiff] = useState<{ config: GeneratedConfig; changed: string[] } | null>(null); + const { gatewayReady } = useSetup(); const handleRefine = async () => { if (instruction.trim() === "") { @@ -45,7 +47,8 @@ export function RefinePanel({ config, onApply }: RefinePanelProps) { value={instruction} onChange={(event) => setInstruction(event.target.value)} /> - {diff && ( diff --git a/web/src/components/RunLauncher.test.tsx b/web/src/components/RunLauncher.test.tsx new file mode 100644 index 0000000..37ec953 --- /dev/null +++ b/web/src/components/RunLauncher.test.tsx @@ -0,0 +1,300 @@ +// Tests for RunLauncher: pick an evaluator, one of its versions, and a dataset, choose +// the run kind and concurrency, then Start. RunLauncher owns its own API traffic (it is +// stubbed in RunsPage.test.tsx), so this suite exercises that traffic directly — loading +// the evaluator/dataset lists, resolving versions for the selected evaluator, falling +// back off "validation" for an unlabeled dataset, submitting, and the shared gateway gate +// that also covers generation and refinement. + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, render, screen, waitFor } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import RunLauncher from "./RunLauncher"; +import { ApiError, datasets, evaluators, runs } from "../api/client"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import type { UseSetupResult } from "./useSetup"; +import type { Dataset, DatasetStats, Evaluator, EvaluatorVersion, Run } from "../api/types"; + +vi.mock("../api/client", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + evaluators: { ...actual.evaluators, list: vi.fn(), get: vi.fn() }, + datasets: { ...actual.datasets, list: vi.fn(), stats: vi.fn() }, + runs: { ...actual.runs, create: vi.fn() }, + }; +}); + +// Launching a run needs the gateway key exactly as generation and refinement do; the +// hook is mocked directly so each test controls gatewayReady without re-exercising +// useSetup's own fetch machinery, which has its own dedicated suite. +vi.mock("./useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + +const evaluatorsListMock = vi.mocked(evaluators.list); +const evaluatorsGetMock = vi.mocked(evaluators.get); +const datasetsListMock = vi.mocked(datasets.list); +const datasetsStatsMock = vi.mocked(datasets.stats); +const runsCreateMock = vi.mocked(runs.create); +const useSetupMock = vi.mocked(useSetup); + +function makeSetupResult(overrides: Partial = {}): UseSetupResult { + return { + status: null, + gatewayReady: true, + loading: false, + error: null, + refetch: vi.fn(), + ...overrides, + }; +} + +function makeEvaluator(overrides: Partial = {}): Evaluator { + return { + id: "ev-1", + created_at: "2026-01-01T00:00:00Z", + name: "My evaluator", + description: "", + active_version_id: "ver-1", + ...overrides, + }; +} + +function makeVersion(overrides: Partial = {}): EvaluatorVersion { + return { + id: "ver-1", + created_at: "2026-01-01T00:00:00Z", + evaluator_id: "ev-1", + version_name: "v1", + notes: "", + frozen: false, + model: "model-a", + instructions: "Judge it.", + prompt_template: "{answer}", + required_columns: ["answer"], + output_fields: [], + score_field: "verdict", + score_kind: "categorical", + score_labels: ["pass", "fail"], + score_minimum: null, + score_maximum: null, + capabilities: [], + tools: [], + ...overrides, + }; +} + +function makeDataset(overrides: Partial = {}): Dataset { + return { + id: "ds-1", + created_at: "2026-01-01T00:00:00Z", + name: "My dataset", + description: "", + columns: ["answer"], + label_schema: {}, + row_count: 10, + labeled_count: 10, + ...overrides, + }; +} + +function makeStats(overrides: Partial = {}): DatasetStats { + return { + total: 10, + labeled: 10, + unlabeled: 0, + label_distribution: {}, + ...overrides, + }; +} + +function makeRun(overrides: Partial = {}): Run { + return { + id: "run-1", + created_at: "2026-01-01T00:00:00Z", + kind: "eval", + version_id: "ver-1", + dataset_id: "ds-1", + status: "pending", + concurrency: 8, + started_at: null, + finished_at: null, + metrics: null, + error: null, + cancel_requested: false, + ...overrides, + }; +} + +function renderLauncher(onStarted: (run: Run) => void = vi.fn()) { + render(); + return { onStarted }; +} + +async function selectFullRun(user: ReturnType) { + await user.selectOptions(await screen.findByLabelText("Evaluator"), "ev-1"); + await waitFor(() => expect(screen.getByLabelText("Version")).not.toBeDisabled()); + await user.selectOptions(screen.getByLabelText("Dataset"), "ds-1"); +} + +beforeEach(() => { + useSetupMock.mockReturnValue(makeSetupResult()); + evaluatorsListMock.mockResolvedValue([makeEvaluator()]); + datasetsListMock.mockResolvedValue([makeDataset()]); + evaluatorsGetMock.mockResolvedValue({ + ...makeEvaluator(), + versions: [makeVersion()], + } as unknown as Evaluator); + datasetsStatsMock.mockResolvedValue(makeStats()); +}); + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); +}); + +describe("RunLauncher", () => { + it("renders the evaluator, version, dataset, run kind, and concurrency fields", async () => { + renderLauncher(); + + expect(await screen.findByLabelText("Evaluator")).toBeInTheDocument(); + expect(screen.getByLabelText("Version")).toBeInTheDocument(); + expect(screen.getByLabelText("Dataset")).toBeInTheDocument(); + expect(screen.getByLabelText("Run kind")).toBeInTheDocument(); + expect(screen.getByLabelText("Concurrency")).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Start" })).toBeInTheDocument(); + }); + + it("loads the evaluator and dataset lists on mount", async () => { + renderLauncher(); + + await waitFor(() => expect(evaluatorsListMock).toHaveBeenCalled()); + expect(datasetsListMock).toHaveBeenCalled(); + }); + + it("disables Start until a version and a dataset are selected", async () => { + renderLauncher(); + + expect(await screen.findByRole("button", { name: "Start" })).toBeDisabled(); + + const user = userEvent.setup(); + await selectFullRun(user); + + expect(screen.getByRole("button", { name: "Start" })).not.toBeDisabled(); + }); + + it("selecting an evaluator loads its versions and preselects the active version", async () => { + evaluatorsGetMock.mockResolvedValue({ + ...makeEvaluator({ active_version_id: "ver-1" }), + versions: [makeVersion({ id: "ver-1", version_name: "v1" }), makeVersion({ id: "ver-2", version_name: "v2" })], + } as unknown as Evaluator); + const user = userEvent.setup(); + renderLauncher(); + + await user.selectOptions(await screen.findByLabelText("Evaluator"), "ev-1"); + + await waitFor(() => expect(screen.getByLabelText("Version")).not.toBeDisabled()); + expect((screen.getByLabelText("Version") as HTMLSelectElement).value).toBe("ver-1"); + }); + + it("falls back off validation to eval when the selected dataset has unlabeled rows", async () => { + datasetsStatsMock.mockResolvedValue(makeStats({ unlabeled: 3, labeled: 7 })); + const user = userEvent.setup(); + renderLauncher(); + + await selectFullRun(user); + await user.selectOptions(screen.getByLabelText("Run kind"), "validation"); + + await waitFor(() => + expect((screen.getByLabelText("Run kind") as HTMLSelectElement).value).toBe("eval"), + ); + expect(screen.getByText(/unlabeled/i)).toBeInTheDocument(); + }); + + it("creates the run with the selected fields and hands it to onStarted", async () => { + runsCreateMock.mockResolvedValue(makeRun()); + const user = userEvent.setup(); + const { onStarted } = renderLauncher(); + + await selectFullRun(user); + const concurrency = screen.getByLabelText("Concurrency"); + await user.clear(concurrency); + await user.type(concurrency, "4"); + await user.click(screen.getByRole("button", { name: "Start" })); + + await waitFor(() => + expect(runsCreateMock).toHaveBeenCalledWith( + expect.objectContaining({ + kind: "eval", + version_id: "ver-1", + dataset_id: "ds-1", + concurrency: 4, + }), + ), + ); + expect(onStarted).toHaveBeenCalledWith(makeRun()); + }); + + it("surfaces a server error and re-enables Start without calling onStarted", async () => { + runsCreateMock.mockRejectedValue(new ApiError("Run failed", "ContractError", 422)); + const user = userEvent.setup(); + const { onStarted } = renderLauncher(); + + await selectFullRun(user); + await user.click(screen.getByRole("button", { name: "Start" })); + + expect(await screen.findByText(/Run failed/i)).toBeInTheDocument(); + expect(onStarted).not.toHaveBeenCalled(); + expect(screen.getByRole("button", { name: "Start" })).not.toBeDisabled(); + }); +}); + +// -- Gateway gating ----------------------------------------------------------- +// A run needs the Pydantic AI Gateway key as much as generation does: launching one +// dispatches the judge agent through the gateway for every row. + +describe("RunLauncher gateway gating", () => { + it("disables Start and shows the shared gateway blocker when the gateway key is unset", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + const user = userEvent.setup(); + renderLauncher(); + + await selectFullRun(user); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Start" })).toBeDisabled(); + expect(runsCreateMock).not.toHaveBeenCalled(); + }); + + it("shows no gateway blocker and governs Start only by its own validity when the gateway is ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + runsCreateMock.mockResolvedValue(makeRun()); + const user = userEvent.setup(); + const { onStarted } = renderLauncher(); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(await screen.findByRole("button", { name: "Start" })).toBeDisabled(); + + await selectFullRun(user); + expect(screen.getByRole("button", { name: "Start" })).not.toBeDisabled(); + + await user.click(screen.getByRole("button", { name: "Start" })); + await waitFor(() => expect(onStarted).toHaveBeenCalled()); + }); + + it("re-enables Start once gatewayReady flips true with valid selections already made", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + const user = userEvent.setup(); + const { rerender } = render(); + + await selectFullRun(user); + expect(screen.getByRole("button", { name: "Start" })).toBeDisabled(); + + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + rerender(); + + expect(screen.getByRole("button", { name: "Start" })).not.toBeDisabled(); + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + }); +}); diff --git a/web/src/components/RunLauncher.tsx b/web/src/components/RunLauncher.tsx index 76f083d..473dc34 100644 --- a/web/src/components/RunLauncher.tsx +++ b/web/src/components/RunLauncher.tsx @@ -7,6 +7,7 @@ import { useEffect, useMemo, useState } from "react"; import { datasets, evaluators, runs } from "../api/client"; import type { Dataset, DatasetStats, Evaluator, EvaluatorVersion, Run, RunKind } from "../api/types"; import { Button, ErrorBanner, Select, Spinner } from "./ui"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; type EvaluatorWithVersions = Evaluator & { versions: EvaluatorVersion[] }; @@ -28,6 +29,7 @@ export default function RunLauncher({ onStarted }: Props) { const [error, setError] = useState(null); const [submitting, setSubmitting] = useState(false); + const { gatewayReady } = useSetup(); useEffect(() => { evaluators.list().then(setEvaluatorList).catch(setError); @@ -77,8 +79,8 @@ export default function RunLauncher({ onStarted }: Props) { }, [validationDisabled, kind]); const canStart = useMemo( - () => versionId !== "" && datasetId !== "" && concurrency > 0 && !submitting, - [versionId, datasetId, concurrency, submitting], + () => versionId !== "" && datasetId !== "" && concurrency > 0 && !submitting && gatewayReady, + [versionId, datasetId, concurrency, submitting, gatewayReady], ); async function start() { @@ -183,6 +185,7 @@ export default function RunLauncher({ onStarted }: Props) {
+ {!gatewayReady && {GATEWAY_BLOCKER}} diff --git a/web/src/components/VersionEditor.test.tsx b/web/src/components/VersionEditor.test.tsx index a32bf62..a3d0923 100644 --- a/web/src/components/VersionEditor.test.tsx +++ b/web/src/components/VersionEditor.test.tsx @@ -13,6 +13,11 @@ vi.mock("../api/client", () => ({ copyVersion: vi.fn(), refine: vi.fn(), }, + // RefinePanel (rendered in the rail) reads useSetup, which calls this; a ready + // gateway keeps this suite's existing behavior unchanged. + setup: { + get: vi.fn().mockResolvedValue({ keys: [] }), + }, })); const config: AppConfig = { diff --git a/web/src/components/useSetup.test.tsx b/web/src/components/useSetup.test.tsx new file mode 100644 index 0000000..eb09a14 --- /dev/null +++ b/web/src/components/useSetup.test.tsx @@ -0,0 +1,231 @@ +// Tests for useSetup, the one hook that every gateway-gated action (generation, runs) reads to +// decide whether to render disabled with the shared GATEWAY_BLOCKER hint. The central contract +// is the don't-strand-the-user guarantee: gatewayReady defaults to true and stays true through a +// transient loading state or a rejected fetch, since the hook is a helpful hint, not the actual +// enforcement (the server-side guard is what really blocks the request). These tests render the +// hook inside a probe component and assert via role/text queries, matching this project's +// convention of never querying by CSS class. The stale-response guard test mirrors the +// equivalent case in ExportModal.test.tsx, which exercises the same cancelled-flag pattern. + +import { afterEach, describe, expect, it, vi } from "vitest"; +import { act, cleanup, render, screen } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; +import { GATEWAY_BLOCKER, useSetup } from "./useSetup"; +import { setup } from "../api/client"; +import type { SetupKey, SetupStatus } from "../api/types"; + +vi.mock("../api/client", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + setup: { ...actual.setup, get: vi.fn() }, + }; +}); + +const setupGet = vi.mocked(setup.get); + +// A promise whose resolution is driven by the test, used to interleave in-flight fetches and +// to observe the hook's state while a request is still pending. +function deferred() { + let resolve!: (value: T) => void; + let reject!: (reason: unknown) => void; + const promise = new Promise((res, rej) => { + resolve = res; + reject = rej; + }); + return { promise, resolve, reject }; +} + +// Builds a SetupStatus with all three known keys, overriding only the `set` flag for each so a +// test can flip just the one bit it cares about. +function makeStatus(overrides: Partial> = {}): SetupStatus { + const defaults: Record = { + gateway_api_key: true, + logfire_token: false, + logfire_api_key: false, + }; + const set = { ...defaults, ...overrides }; + const names: SetupKey["name"][] = ["gateway_api_key", "logfire_token", "logfire_api_key"]; + return { + keys: names.map((name) => ({ + name, + set: set[name], + required: name === "gateway_api_key", + label: name, + command: `valcore config set ${name} ...`, + purpose: `used for ${name}`, + })), + }; +} + +// Renders the hook's return value as plain text/role nodes a test can query the way this +// project's component tests already do, rather than reaching for renderHook internals. +function Probe() { + const { status, gatewayReady, loading, error, refetch } = useSetup(); + return ( +
+

{loading ? "loading" : "idle"}

+

{gatewayReady ? "gateway ready" : "gateway blocked"}

+

{error !== null ? `error:${String(error)}` : "no error"}

+

{status ? `keys:${status.keys.length}` : "no status"}

+ +
+ ); +} + +afterEach(() => { + cleanup(); + vi.clearAllMocks(); + vi.restoreAllMocks(); +}); + +describe("useSetup", () => { + it("reports gatewayReady true once loaded when the gateway key is set", async () => { + setupGet.mockResolvedValue(makeStatus({ gateway_api_key: true })); + render(); + + expect(await screen.findByText("gateway ready")).toBeInTheDocument(); + expect(screen.queryByText("gateway blocked")).toBeNull(); + expect(screen.getByText("keys:3")).toBeInTheDocument(); + }); + + it("reports gatewayReady false once loaded when the gateway key is unset", async () => { + setupGet.mockResolvedValue(makeStatus({ gateway_api_key: false })); + render(); + + expect(await screen.findByText("gateway blocked")).toBeInTheDocument(); + expect(screen.queryByText("gateway ready")).toBeNull(); + }); + + it("keeps gatewayReady true while the initial fetch is still in flight", async () => { + const pending = deferred(); + setupGet.mockReturnValue(pending.promise); + render(); + + expect(screen.getByRole("status")).toHaveTextContent("loading"); + // Not yet loaded, and the gateway key could turn out to be unset — the hook must not strand + // the user with disabled buttons during this window. + expect(screen.getByText("gateway ready")).toBeInTheDocument(); + + pending.resolve(makeStatus({ gateway_api_key: false })); + // Only once the fetch resolves does the real (false) value take over. + expect(await screen.findByText("gateway blocked")).toBeInTheDocument(); + }); + + it("keeps gatewayReady true after the initial fetch rejects", async () => { + setupGet.mockRejectedValue(new Error("network down")); + render(); + + await screen.findByText(/error:/); + expect(screen.getByRole("status")).toHaveTextContent("idle"); + expect(screen.getByText("gateway ready")).toBeInTheDocument(); + expect(screen.queryByText("gateway blocked")).toBeNull(); + }); + + it("keeps gatewayReady true while a refetch is in flight, even after a loaded unset key", async () => { + const user = userEvent.setup(); + const pending = deferred(); + setupGet.mockResolvedValueOnce(makeStatus({ gateway_api_key: false })).mockReturnValueOnce(pending.promise); + render(); + + // The initial load establishes a real (false) status — this is what exposed the bug: a + // naive `gatewayKey?.set !== false` read of stale status ignores the in-flight refetch. + await screen.findByText("gateway blocked"); + + await user.click(screen.getByRole("button", { name: "Refetch" })); + expect(screen.getByRole("status")).toHaveTextContent("loading"); + expect(screen.getByText("gateway ready")).toBeInTheDocument(); + expect(screen.queryByText("gateway blocked")).toBeNull(); + + await act(async () => { + pending.resolve(makeStatus({ gateway_api_key: false })); + await Promise.resolve(); + }); + expect(await screen.findByText("gateway blocked")).toBeInTheDocument(); + }); + + it("keeps gatewayReady true after a refetch rejects, even after a loaded unset key", async () => { + const user = userEvent.setup(); + setupGet + .mockResolvedValueOnce(makeStatus({ gateway_api_key: false })) + .mockRejectedValueOnce(new Error("network down")); + render(); + + await screen.findByText("gateway blocked"); + + await user.click(screen.getByRole("button", { name: "Refetch" })); + + await screen.findByText(/error:/); + expect(screen.getByRole("status")).toHaveTextContent("idle"); + expect(screen.getByText("gateway ready")).toBeInTheDocument(); + expect(screen.queryByText("gateway blocked")).toBeNull(); + }); + + it("refetch issues a second request and reflects the new value", async () => { + const user = userEvent.setup(); + setupGet + .mockResolvedValueOnce(makeStatus({ gateway_api_key: false })) + .mockResolvedValueOnce(makeStatus({ gateway_api_key: true })); + render(); + + await screen.findByText("gateway blocked"); + await user.click(screen.getByRole("button", { name: "Refetch" })); + + expect(await screen.findByText("gateway ready")).toBeInTheDocument(); + expect(setupGet).toHaveBeenCalledTimes(2); + }); + + it("clears a stale error once a refetch succeeds", async () => { + const user = userEvent.setup(); + setupGet + .mockRejectedValueOnce(new Error("network down")) + .mockResolvedValueOnce(makeStatus({ gateway_api_key: true })); + render(); + + await screen.findByText(/error:/); + await user.click(screen.getByRole("button", { name: "Refetch" })); + + expect(await screen.findByText("no error")).toBeInTheDocument(); + expect(screen.getByText("gateway ready")).toBeInTheDocument(); + }); + + it("keeps the later response in state when an earlier in-flight refetch resolves last", async () => { + const user = userEvent.setup(); + const first = deferred(); + const second = deferred(); + setupGet + .mockResolvedValueOnce(makeStatus({ gateway_api_key: true })) // initial mount fetch + .mockReturnValueOnce(first.promise) // first refetch, stale + .mockReturnValueOnce(second.promise); // second refetch, supersedes the first + + render(); + await screen.findByText("gateway ready"); + + await user.click(screen.getByRole("button", { name: "Refetch" })); + await user.click(screen.getByRole("button", { name: "Refetch" })); + + // The newer (second) request settles first, then the stale (first) one. + second.resolve(makeStatus({ gateway_api_key: false })); + expect(await screen.findByText("gateway blocked")).toBeInTheDocument(); + + // Resolve and flush the stale promise inside act() so its (wrongly) resulting render, if + // any, commits before the assertion below runs — awaiting bare microtasks outside act() + // lets React defer that commit past the assertion and hide the bug. + await act(async () => { + first.resolve(makeStatus({ gateway_api_key: true })); + await Promise.resolve(); + await Promise.resolve(); + }); + + expect(screen.getByText("gateway blocked")).toBeInTheDocument(); + expect(screen.queryByText("gateway ready")).toBeNull(); + }); + + it("exposes GATEWAY_BLOCKER as a non-empty string mentioning the gateway key", () => { + expect(typeof GATEWAY_BLOCKER).toBe("string"); + expect(GATEWAY_BLOCKER.length).toBeGreaterThan(0); + expect(GATEWAY_BLOCKER.toLowerCase()).toContain("gateway"); + }); +}); diff --git a/web/src/components/useSetup.ts b/web/src/components/useSetup.ts new file mode 100644 index 0000000..30375b3 --- /dev/null +++ b/web/src/components/useSetup.ts @@ -0,0 +1,71 @@ +// The one hook every gateway-gated action (generation, runs) reads to decide whether to render +// disabled, and the shared string those call sites show when it does. Fetching setup status is a +// hint for the UI, not the enforcement — the server-side guard is what actually blocks the +// request — so a transient fetch failure or a slow initial load must never strand the user with +// every button disabled. That is why `gatewayReady` defaults to (and stays) true except in the +// one case where the loaded status says the key is actually unset. + +import { useCallback, useEffect, useState } from "react"; +import { setup } from "../api/client"; +import type { SetupStatus } from "../api/types"; + +export const GATEWAY_BLOCKER = + "Set the Pydantic AI Gateway key to generate or run — see Setup on the Overview page."; + +export interface UseSetupResult { + status: SetupStatus | null; + gatewayReady: boolean; + loading: boolean; + error: unknown; + refetch: () => void; +} + +export function useSetup(): UseSetupResult { + const [status, setStatus] = useState(null); + const [loading, setLoading] = useState(true); + const [error, setError] = useState(null); + // Bumping this triggers the fetch effect on demand, driving both the initial load and refetch + // through the same cancelled-flag-guarded effect rather than duplicating the fetch logic. + const [version, setVersion] = useState(0); + + useEffect(() => { + let cancelled = false; + setLoading(true); + + setup + .get() + .then((result) => { + // Guard against a stale in-flight response overwriting a newer one: a superseded fetch + // has already had `cancelled` flipped by its cleanup before this resolves. + if (!cancelled) { + setStatus(result); + setError(null); + } + }) + .catch((err) => { + if (!cancelled) { + setError(err); + } + }) + .finally(() => { + if (!cancelled) { + setLoading(false); + } + }); + + return () => { + cancelled = true; + }; + }, [version]); + + const refetch = useCallback(() => { + setVersion((current) => current + 1); + }, []); + + // Only a loaded, successful response reporting the key unset may disable gated actions — + // loading, an error, or a not-yet-loaded status must all resolve to true. + const gatewayKey = status?.keys.find((key) => key.name === "gateway_api_key"); + const gatewayReady = loading || error !== null || gatewayKey?.set !== false; + + return { status, gatewayReady, loading, error, refetch }; +} diff --git a/web/src/pages/EvaluatorsPage.test.tsx b/web/src/pages/EvaluatorsPage.test.tsx index d6a7f71..92233d8 100644 --- a/web/src/pages/EvaluatorsPage.test.tsx +++ b/web/src/pages/EvaluatorsPage.test.tsx @@ -1,9 +1,11 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; import { cleanup, render, screen, waitFor } from "@testing-library/react"; import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; import EvaluatorsPage from "./EvaluatorsPage"; import { api, evaluators } from "../api/client"; +import { GATEWAY_BLOCKER, useSetup } from "../components/useSetup"; +import type { UseSetupResult } from "../components/useSetup"; import type { Evaluator, GeneratedConfig } from "../api/types"; const navigate = vi.fn(); @@ -27,6 +29,27 @@ vi.mock("../api/client", async () => { }; }); +// Only Generate (criteria mode) calls the model through the gateway; scratch-mode +// Create must stay usable regardless. The hook is mocked directly so each test can set +// gatewayReady without re-exercising useSetup's own fetch machinery. +vi.mock("../components/useSetup", async (importOriginal) => { + const actual = await importOriginal(); + return { ...actual, useSetup: vi.fn() }; +}); + +const useSetupMock = vi.mocked(useSetup); + +function makeSetupResult(overrides: Partial = {}): UseSetupResult { + return { + status: null, + gatewayReady: true, + loading: false, + error: null, + refetch: vi.fn(), + ...overrides, + }; +} + const config = { models: ["model-a", "model-b"], tools: [], capabilities: [] }; function makeEvaluator(overrides: Partial = {}): Evaluator { @@ -72,6 +95,10 @@ afterEach(() => { vi.clearAllMocks(); }); +beforeEach(() => { + useSetupMock.mockReturnValue(makeSetupResult()); +}); + describe("EvaluatorsPage: new-evaluator modal", () => { it("defaults to From scratch and shows name + description, not criteria", async () => { vi.mocked(evaluators.list).mockResolvedValue([]); @@ -285,3 +312,95 @@ describe("EvaluatorsPage: create modal guidance", () => { expect(screen.queryByRole("button", { name: "Create" })).toBeNull(); }); }); + +// -- Gateway gating ----------------------------------------------------------- +// Only the criteria-mode Generate action calls a model through the gateway; scratch-mode +// authoring (Create) must stay fully usable even when the gateway key is unset. + +describe("EvaluatorsPage: gateway gating", () => { + async function openModal() { + const user = userEvent.setup(); + renderPage(); + await user.click(await screen.findByRole("button", { name: "New evaluator" })); + return user; + } + + it("still allows creating an evaluator in scratch mode when the gateway is not ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + vi.mocked(evaluators.list).mockResolvedValue([]); + vi.mocked(api).mockResolvedValue(config); + vi.mocked(evaluators.create).mockResolvedValue(makeEvaluator({ id: "e-scratch" })); + const user = await openModal(); + + // Scratch mode (the default) is untouched by the gateway gate. + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + + await user.type(screen.getByLabelText("Evaluator name"), "My evaluator"); + const create = screen.getByRole("button", { name: "Create" }); + expect(create).not.toBeDisabled(); + + await user.click(create); + + await waitFor(() => + expect(evaluators.create).toHaveBeenCalledWith( + expect.objectContaining({ name: "My evaluator" }), + ), + ); + expect(navigate).toHaveBeenCalledWith("/evaluators/e-scratch"); + }); + + it("disables Generate and shows the shared gateway blocker in criteria mode when not ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + vi.mocked(evaluators.list).mockResolvedValue([]); + vi.mocked(api).mockResolvedValue(config); + const user = await openModal(); + + await user.click(screen.getByRole("tab", { name: "From criteria" })); + await user.type(screen.getByLabelText("Evaluator name"), "Criteria eval"); + await user.type(screen.getByLabelText("Criteria"), "A good answer is concise."); + + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + expect(evaluators.generate).not.toHaveBeenCalled(); + }); + + it("shows no gateway blocker in scratch mode even when the gateway is not ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + vi.mocked(evaluators.list).mockResolvedValue([]); + vi.mocked(api).mockResolvedValue(config); + await openModal(); + + // Scratch mode is the default tab; no gateway blocker should leak in from criteria mode. + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + }); + + it("switching from a blocked criteria mode back to scratch clears the gateway blocker", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: false })); + vi.mocked(evaluators.list).mockResolvedValue([]); + vi.mocked(api).mockResolvedValue(config); + const user = await openModal(); + + await user.click(screen.getByRole("tab", { name: "From criteria" })); + expect(screen.getByText(GATEWAY_BLOCKER)).toBeInTheDocument(); + + await user.click(screen.getByRole("tab", { name: "From scratch" })); + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + }); + + it("governs Generate only by its own validity (name + criteria) once the gateway is ready", async () => { + useSetupMock.mockReturnValue(makeSetupResult({ gatewayReady: true })); + vi.mocked(evaluators.list).mockResolvedValue([]); + vi.mocked(api).mockResolvedValue(config); + const user = await openModal(); + + await user.click(screen.getByRole("tab", { name: "From criteria" })); + + expect(screen.queryByText(GATEWAY_BLOCKER)).toBeNull(); + expect(screen.getByRole("button", { name: "Generate" })).toBeDisabled(); + + await user.type(screen.getByLabelText("Evaluator name"), "Criteria eval"); + await user.type(screen.getByLabelText("Criteria"), "A good answer is concise."); + + expect(screen.getByRole("button", { name: "Generate" })).not.toBeDisabled(); + }); +}); diff --git a/web/src/pages/EvaluatorsPage.tsx b/web/src/pages/EvaluatorsPage.tsx index ea89c95..fd64c30 100644 --- a/web/src/pages/EvaluatorsPage.tsx +++ b/web/src/pages/EvaluatorsPage.tsx @@ -16,6 +16,7 @@ import { EmptyState } from "../components/EmptyState"; import { FormFooter } from "../components/FormFooter"; import { Tooltip } from "../components/Tooltip"; import { EvaluatorIcon } from "../components/icons"; +import { GATEWAY_BLOCKER, useSetup } from "../components/useSetup"; type EvaluatorRow = Evaluator & { active_version: { version_name: string } | null; @@ -57,6 +58,7 @@ function EvaluatorsList() { const [name, setName] = useState(""); const [description, setDescription] = useState(""); const [criteria, setCriteria] = useState(""); + const { gatewayReady } = useSetup(); useEffect(() => { evaluators @@ -68,8 +70,15 @@ function EvaluatorsList() { }, []); // One reason at a time, so a blocked primary action says why instead of sitting - // silently disabled. Order mirrors the fields top to bottom. + // silently disabled. Order mirrors the fields top to bottom, except the gateway + // blocker: it always leads when it applies, since no amount of filling in the form + // resolves it. const blockers: string[] = []; + // Only the criteria mode calls a model through the gateway; scratch-mode authoring + // stays fully usable regardless of gateway key presence. + if (mode === "criteria" && !gatewayReady) { + blockers.push(GATEWAY_BLOCKER); + } if (name.trim() === "") { blockers.push("Add a name"); } diff --git a/web/src/pages/OverviewPage.test.tsx b/web/src/pages/OverviewPage.test.tsx index db4027e..d466b99 100644 --- a/web/src/pages/OverviewPage.test.tsx +++ b/web/src/pages/OverviewPage.test.tsx @@ -1,21 +1,24 @@ -import { afterEach, describe, expect, it, vi } from "vitest"; -import { cleanup, render, screen } from "@testing-library/react"; +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { cleanup, render, screen, waitFor, within } from "@testing-library/react"; +import userEvent from "@testing-library/user-event"; import { MemoryRouter } from "react-router-dom"; import OverviewPage from "./OverviewPage"; -import { overview } from "../api/client"; -import type { Overview } from "../api/types"; +import { overview, setup } from "../api/client"; +import type { Overview, SetupKey, SetupStatus } from "../api/types"; -// Only the overview endpoint is exercised here; the page makes exactly one -// request on mount. Keep every other client member intact so the module loads. +// The overview and setup endpoints are exercised here; the page makes one request to each on +// mount. Keep every other client member intact so the module loads. vi.mock("../api/client", async (importOriginal) => { const actual = await importOriginal(); return { ...actual, overview: { ...actual.overview, get: vi.fn() }, + setup: { ...actual.setup, get: vi.fn() }, }; }); const getMock = vi.mocked(overview.get); +const setupGet = vi.mocked(setup.get); function makeOverview(overrides: Partial = {}): Overview { return { @@ -36,6 +39,44 @@ function makeOverview(overrides: Partial = {}): Overview { }; } +// Builds a SetupStatus with all three known keys, overriding only the `set` flag for each so a +// test can flip just the one bit it cares about. Mirrors the helper in useSetup.test.tsx, but +// keeps its own copy of the per-key label/command/purpose so this suite can assert on them +// without importing test fixtures across modules. +function makeSetupStatus(overrides: Partial> = {}): SetupStatus { + const defaults: Record = { + gateway_api_key: true, + logfire_token: true, + logfire_api_key: true, + }; + const set = { ...defaults, ...overrides }; + const fixed: Record> = { + gateway_api_key: { + name: "gateway_api_key", + required: true, + label: "Pydantic AI Gateway key", + command: "valcore config set gateway_api_key ", + purpose: "Required to generate datasets and run evaluators.", + }, + logfire_token: { + name: "logfire_token", + required: false, + label: "Logfire write token", + command: "valcore config set logfire_token ", + purpose: "Sends run and row spans to Logfire for tracing.", + }, + logfire_api_key: { + name: "logfire_api_key", + required: false, + label: "Logfire API key", + command: "valcore config set logfire_api_key ", + purpose: "Pushes datasets to Logfire's hosted dataset store.", + }, + }; + const names: SetupKey["name"][] = ["gateway_api_key", "logfire_token", "logfire_api_key"]; + return { keys: names.map((name) => ({ ...fixed[name], set: set[name] })) }; +} + function renderPage() { return render( @@ -44,6 +85,12 @@ function renderPage() { ); } +beforeEach(() => { + // A harmless default (all keys set, card collapsed) so every pre-existing test that doesn't + // care about setup state still gets a resolved promise instead of an unhandled rejection. + setupGet.mockResolvedValue(makeSetupStatus()); +}); + afterEach(() => { cleanup(); vi.clearAllMocks(); @@ -200,3 +247,138 @@ describe("OverviewPage", () => { expect(screen.queryByText("Best accuracy")).toBeNull(); }); }); + +// The setup card sits above the stats and is driven entirely by useSetup's own fetch — it must +// neither block nor be blocked by the overview fetch. Every test here gives the overview request +// a resolved value so the stat-card assertions in the shared-rendering tests have something to +// find, and drives setup state through the same api/client mock the rest of this suite uses. +describe("OverviewPage setup card", () => { + it("renders expanded with all three commands, marking gateway required and Logfire optional, when the gateway key is unset", async () => { + getMock.mockResolvedValue(makeOverview()); + const status = makeSetupStatus({ + gateway_api_key: false, + logfire_token: false, + logfire_api_key: false, + }); + setupGet.mockResolvedValue(status); + + renderPage(); + await screen.findByText("Overview"); + + const rows = await screen.findAllByRole("listitem"); + expect(rows).toHaveLength(3); + + for (const key of status.keys) { + const row = rows.find((candidate) => within(candidate).queryByText(key.label)); + expect(row).toBeTruthy(); + within(row as HTMLElement).getByText(key.command); + within(row as HTMLElement).getByText(key.required ? "Required" : "Optional"); + } + }); + + it("collapses to a summary line and shows no commands when all keys are set", async () => { + getMock.mockResolvedValue(makeOverview()); + const status = makeSetupStatus(); + setupGet.mockResolvedValue(status); + + renderPage(); + await screen.findByText("Overview"); + + expect(await screen.findByText(/all setup keys are configured/i)).toBeInTheDocument(); + expect(screen.queryByRole("listitem")).toBeNull(); + for (const key of status.keys) { + expect(screen.queryByText(key.command)).toBeNull(); + } + }); + + it("copies the right command for the right key when several are shown", async () => { + const user = userEvent.setup(); + getMock.mockResolvedValue(makeOverview()); + const status = makeSetupStatus({ + gateway_api_key: false, + logfire_token: false, + logfire_api_key: true, + }); + setupGet.mockResolvedValue(status); + + renderPage(); + + // userEvent.setup() installs its own clipboard stub, so the spy must be installed after it + // runs, matching ExportModal.test.tsx's copy-testing convention. + const writeText = vi.fn(() => Promise.resolve()); + Object.defineProperty(navigator, "clipboard", { + value: { writeText }, + configurable: true, + writable: true, + }); + + const rows = await screen.findAllByRole("listitem"); + const gatewayRow = rows.find((row) => within(row).queryByText("Pydantic AI Gateway key")); + const logfireApiKeyRow = rows.find((row) => within(row).queryByText("Logfire API key")); + expect(gatewayRow).toBeTruthy(); + expect(logfireApiKeyRow).toBeTruthy(); + + await user.click(within(gatewayRow as HTMLElement).getByRole("button", { name: "Copy" })); + expect(writeText).toHaveBeenLastCalledWith(status.keys[0].command); + + await user.click(within(logfireApiKeyRow as HTMLElement).getByRole("button", { name: "Copy" })); + expect(writeText).toHaveBeenLastCalledWith(status.keys[2].command); + }); + + it("Recheck triggers a second fetch and updates the card from expanded to collapsed", async () => { + const user = userEvent.setup(); + getMock.mockResolvedValue(makeOverview()); + setupGet + .mockResolvedValueOnce(makeSetupStatus({ gateway_api_key: false })) + .mockResolvedValueOnce(makeSetupStatus()); + + renderPage(); + + await screen.findAllByRole("listitem"); + await user.click(screen.getByRole("button", { name: "Recheck" })); + + await waitFor(() => expect(screen.queryByRole("listitem")).toBeNull()); + expect(await screen.findByText(/all setup keys are configured/i)).toBeInTheDocument(); + expect(setupGet).toHaveBeenCalledTimes(2); + }); + + it("renders the existing stat cards unchanged while the setup card is expanded", async () => { + getMock.mockResolvedValue(makeOverview()); + setupGet.mockResolvedValue(makeSetupStatus({ gateway_api_key: false })); + + renderPage(); + await screen.findAllByRole("listitem"); + + expect(screen.getByText("Evaluators")).toBeTruthy(); + expect(screen.getByText("7")).toBeTruthy(); + expect(screen.getByText("Datasets")).toBeTruthy(); + expect(screen.getByText(/5 of 40 labeled/i)).toBeTruthy(); + expect(screen.getByText("Best accuracy")).toBeTruthy(); + expect(screen.getByText("91%")).toBeTruthy(); + }); + + it("renders the existing stat cards unchanged while the setup card is collapsed", async () => { + getMock.mockResolvedValue(makeOverview()); + setupGet.mockResolvedValue(makeSetupStatus()); + + renderPage(); + await screen.findByText(/all setup keys are configured/i); + + expect(screen.getByText("Evaluators")).toBeTruthy(); + expect(screen.getByText("7")).toBeTruthy(); + expect(screen.getByText("Datasets")).toBeTruthy(); + expect(screen.getByText("Best accuracy")).toBeTruthy(); + expect(screen.getByText("91%")).toBeTruthy(); + }); + + it("does not blank the page when the setup fetch rejects — stats still render", async () => { + getMock.mockResolvedValue(makeOverview()); + setupGet.mockRejectedValue(new Error("setup unavailable")); + + renderPage(); + + expect(await screen.findByText("Best accuracy")).toBeTruthy(); + expect(screen.getByText("91%")).toBeTruthy(); + expect(screen.getByText("Evaluators")).toBeTruthy(); + }); +}); diff --git a/web/src/pages/OverviewPage.tsx b/web/src/pages/OverviewPage.tsx index 1dc967e..fbf8a8c 100644 --- a/web/src/pages/OverviewPage.tsx +++ b/web/src/pages/OverviewPage.tsx @@ -5,10 +5,11 @@ import { useEffect, useState } from "react"; import { Link } from "react-router-dom"; import { overview } from "../api/client"; -import type { Overview } from "../api/types"; +import type { Overview, SetupKey } from "../api/types"; import { EmptyState } from "../components/EmptyState"; import { PageHeader } from "../components/PageHeader"; -import { ErrorBanner, Spinner } from "../components/ui"; +import { useSetup } from "../components/useSetup"; +import { Button, ErrorBanner, Spinner } from "../components/ui"; // Both accuracy fields are 0..1 floats or null. Null is a genuine "no measurement" // state, not zero — render an em dash so it never reads as 0% or NaN%. @@ -16,6 +17,64 @@ function formatAccuracy(value: number | null): string { return value === null ? "—" : `${Math.round(value * 100)}%`; } +// Renders one key's row: label, required/optional badge, purpose, and its command with a Copy +// button. The command is always shown while the card is expanded — even for a key that is +// already set — so a user who sets the gateway key but skips Logfire still sees exactly what to +// run for the optional keys without re-expanding anything. +function SetupKeyRow({ item }: { item: SetupKey }): JSX.Element { + const copy = () => { + void navigator.clipboard.writeText(item.command); + }; + + return ( +
  • + {item.label} + + {item.required ? "Required" : "Optional"} + + {item.set ? "Set" : "Not set"} + {item.purpose} +
    + {item.command} + +
    +
    • + ); +} + +// The setup walkthrough. Expanded (one row per key) whenever anything is unset, collapsed to a +// quiet summary once everything is configured — incomplete setup is itself the trigger, so it +// reappears on its own if a user clears their config. A still-loading or errored fetch renders +// nothing here; the rest of the page never waits on it. +function SetupCard(): JSX.Element | null { + const { status, refetch } = useSetup(); + + if (status === null) { + return null; + } + + const allSet = status.keys.every((key) => key.set); + + return ( +
    + {allSet ? ( +
    All setup keys are configured.
    + ) : ( +
      + {status.keys.map((key) => ( + + ))} +
    + )} + +
    + ); +} + export default function OverviewPage(): JSX.Element { const [data, setData] = useState(null); const [error, setError] = useState(null); @@ -54,6 +113,7 @@ export default function OverviewPage(): JSX.Element { return (
    +
    + +
    {data.evaluator_count}
    diff --git a/web/src/styles.css b/web/src/styles.css index 4762d33..dfb8982 100644 --- a/web/src/styles.css +++ b/web/src/styles.css @@ -1584,3 +1584,56 @@ kbd { .tick-ok { color: var(--success); } + +/* -- Setup card --------------------------------------------------------------- */ + +.setup-card { + display: flex; + flex-direction: column; + gap: var(--space-3); + padding: var(--space-4); + background: var(--panel); + border: 1px solid var(--border); + border-radius: var(--radius-lg); +} + +/* Label and status share a row; the command wraps to its own line beneath via the + 100% flex-basis on `.setup-command`. */ +.setup-key { + display: flex; + align-items: baseline; + flex-wrap: wrap; + gap: var(--space-2); + padding-bottom: var(--space-3); + border-bottom: 1px solid var(--border); +} + +.setup-key:last-child { + padding-bottom: 0; + border-bottom: none; +} + +.setup-key-required { + color: var(--accent); + font-size: var(--text-sm); + font-weight: 600; +} + +.setup-key-optional { + color: var(--muted); + font-size: var(--text-sm); +} + +.setup-command { + flex: 1 0 100%; + font-family: ui-monospace, SFMono-Regular, Menlo, monospace; + font-size: var(--text-sm); + background: var(--panel-sunk); + border-radius: var(--radius); + padding: var(--space-1) var(--space-2); +} + +.setup-summary { + color: var(--muted); + font-size: var(--text-sm); +} diff --git a/web/src/styles.test.ts b/web/src/styles.test.ts index 3dfe598..c37afd7 100644 --- a/web/src/styles.test.ts +++ b/web/src/styles.test.ts @@ -119,6 +119,13 @@ const CONTRACT_CLASSES = [ "export-file", "export-file-name", "export-file-actions", + // Setup card. + "setup-card", + "setup-key", + "setup-key-required", + "setup-key-optional", + "setup-command", + "setup-summary", ]; // Every design token the contract publishes on :root. Later tasks reference these in the