From 764617ab0c1ab965e33a2cf323f03cb8c05749a9 Mon Sep 17 00:00:00 2001 From: Luca Belli <129434630+sator-labs@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:27:41 -0700 Subject: [PATCH 1/2] feat: add unified vera judge command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Mirrors `vera generate`: same five-step spine (register, run, resolve_configs, _from_cli/_from_config, _execute), and `resolve_input` is reused unchanged -- the per-command parameterization of the config-or-flags rule needed no modification to serve a second command. `ResolvedTarget`'s rubric fields, validated since #191 and unused until now, needed no new manifest code. Three changes were not a mirror. `RunConfig` becomes multi-section: `generation` and `judging` are both optional with at least one required. `to_dict` omits absent sections rather than emitting null, because its output doubles as input config and each command rejects top-level keys it does not own -- a null section would make `generate --print` emit something `generate` itself refuses. The existing byte-identical generate round-trip is covered by a test. `generation_persona_sets` becomes `targets_from_config`, owning only the two rules both commands share: the `target: "all"` fan-out and the target/explicit-fields mutual exclusion. Projecting a resolved target onto the fields a command needs stays in the command module, which avoids two near-copies of the exclusivity logic. Provider parameters return to the CLI as `--judge-params`, plus `--user-params` and `--chatbot-params` for generate, which silently lost the capability in #191 -- `ModelSpec.from_shorthand` always produced empty params, leaving `--config` the only way to set them. CHANGELOG v1.2 documents `-jep` in its migration instructions, so this was a live regression. Parameters are supplied per role, as the legacy scripts accepted, and stored per model, so `--print` shows what each model will use. Judge-specific behavior, per the decisions in #193: - `--target all` is rejected until Phase 4 adds `evaluations//`; until then N rubric runs would share one output folder and be distinguishable only by timestamp. - `--conversations` takes exactly one folder; judge separately and combine with `vera pool`. - No `--resume` and no single-conversation mode; both stay in legacy `judge.py`. - `--sample` doubles as the debug cap, limiting conversations for judge and personas per file for generate, so `InvocationConfig` stays uniform. `JudgingConfig` rejects two inputs the domain cannot honor: repeated model names, which would silently collapse because judge models are keyed by name, and differing per-model provider parameters, because `run_judging` takes one parameter dict for the whole run. Breaking change, noted in CHANGELOG.md and architecture.md: `vera judge` drops legacy `judge.py`'s fallback of writing evaluations to `evaluations/` relative to the working directory when the input is a flat transcript folder rather than a generation run. It errors and requires `-o/--output`. Old flat folders remain readable with an explicit `-o`. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 4 + docs/architecture.md | 2 +- utils/config_schema.py | 146 +++++++++++- vera.py | 2 + vera_cli/README.md | 3 +- vera_cli/config.py | 22 ++ vera_cli/generate.py | 68 +++++- vera_cli/judge.py | 498 +++++++++++++++++++++++++++++++++++++++++ vera_cli/targets.py | 105 ++++----- 9 files changed, 781 insertions(+), 69 deletions(-) create mode 100644 vera_cli/judge.py diff --git a/CHANGELOG.md b/CHANGELOG.md index fb915bb36..49ee7f3b7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] +### Breaking / migration + +- **`vera judge` requires an explicit output location for flat conversation folders** — Legacy [`judge.py`](judge.py) falls back to writing evaluations into `evaluations/` *relative to the working directory* when `--folder` points at a flat folder of `.txt` transcripts rather than a generation run. `vera judge` does not carry that fallback: it errors and asks for `-o/--output`. The default for a generation run is unchanged and still lands beside the transcripts, at `/evaluations/`. Reading old flat-layout conversations continues to work — pass `-o` to say where the results go. The fallback was dropped because it detached evaluations from the conversations that produced them, and because the same relative path means different directories depending on the input form (CLI paths resolve against the working directory, config paths against the repository root). Legacy `judge.py` keeps the old behavior until it is removed. + ## [v1.2.0](https://github.com/SpringCare/VERA-MH/releases/tag/v1.2.0) \- 2026-07-16 ### Breaking / migration diff --git a/docs/architecture.md b/docs/architecture.md index d176ddba5..2336bd955 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -446,7 +446,7 @@ uv run pytest -m "not live" | **0 — De-risk multi-rubric** | Prove the target-manifest format on both generation and judging before the unified CLI replaces legacy scripts | Treat every manifest as one complete target containing a rubric, personas, and both prompt sets. Legacy judge code may consume only its rubric fields and legacy generation code only its persona fields, but both read the same complete manifest. Preserve the existing explicit persona/rubric paths so Phase 1 can offer both whole-target and component-level selection | `pytest -m "not live"` green; one complete fixture target drives both legacy generation and judging; incomplete manifests fail validation; explicit persona/rubric selection remains covered | | **S — Storage abstraction** *(orthogonal — see note above)* | Decouple "what path/key to use" from "how to persist bytes," so a future non-local backend (S3, etc.) is a new implementation, not a rewrite | New `storage/` package: `StorageBackend` (ABC) with `write(key, bytes)` / `read(key)` / `exists(key)`, plus `LocalFilesystemStorage` as the default implementation — mirrors the `LLMInterface`/`QueueProtocol` idiom (interface and implementations live together in one concern-scoped package). The backend knows nothing about run semantics; `utils/naming.py` still builds keys/paths, `storage/` just persists what it's given. All domain/`workers/` code that currently touches the filesystem directly switches to calling through `StorageBackend` instead | `pytest -m "not live"` green; no domain or `workers/` code calls `open()`/`pathlib` file-write directly for run artifacts — everything routes through `StorageBackend`; `LocalFilesystemStorage` is behaviorally identical to today's direct-filesystem writes | | **O — Adopt OpenSpec** *(orthogonal — see note above)* | Turn "consider an OpenSpec change if the team adopts that workflow" from a maybe into an actual, exercised requirement | Populate `openspec/changes/` with a real OpenSpec change document the next time a large multi-file feature lands (the existing ESCALATE trigger: new judge dimensions, pipeline CLI changes). Currently `openspec/` is empty scaffolding — this phase is "done" only once a real change has actually gone through it, not just once the config exists | A qualifying multi-file feature has shipped with a real OpenSpec change document under `openspec/changes/`, and the ESCALATE section's language is updated from "if the team adopts" to a firm MUST for future qualifying changes | -| **1 — New CLI + config** | `vera.py` fully replaces the top-level scripts | Add whole-target selection through `--target` and top-level config `target`. Preserve explicit component selection through `--personas ` and `--rubric `, so callers can combine the persona side of one target with the rubric side of another. Resolve target manifests before print or dispatch, then call existing domain behavior directly. Ship `-u`/`-j`/`--sample` and the informal config shape; delete `generate.py`/`run_pipeline.py` at the end of the phase. **`judge.py` is the one exception and outlives this phase:** `vera judge` ships without `--resume`, because the resume contract is deferred (see the Deferred resume contract note above), so `judge.py` is retained *solely* as the resume entry point until `vera resume` exists. It is not a general escape hatch — no new work targets it, and it is deleted the moment `vera resume` lands. Judging output also keeps the existing `/evaluations/j_*` layout in this phase; Phase 3 renames it and Phase 4 adds the `/` segment | `pytest -m "not live"` green; `vera.py` is the only documented entry point for everything except resume; target and explicit-component paths have structural parity tests; `--config`, `--target`, `--personas`, `--rubric`, `-u`, `-j`, and `--sample` are functional; `generate.py` and `run_pipeline.py` are gone | +| **1 — New CLI + config** | `vera.py` fully replaces the top-level scripts | Add whole-target selection through `--target` and top-level config `target`. Preserve explicit component selection through `--personas ` and `--rubric `, so callers can combine the persona side of one target with the rubric side of another. Resolve target manifests before print or dispatch, then call existing domain behavior directly. Ship `-u`/`-j`/`--sample` and the informal config shape; delete `generate.py`/`run_pipeline.py` at the end of the phase. **`judge.py` is the one exception and outlives this phase:** `vera judge` ships without `--resume`, because the resume contract is deferred (see the Deferred resume contract note above), so `judge.py` is retained *solely* as the resume entry point until `vera resume` exists. It is not a general escape hatch — no new work targets it, and it is deleted the moment `vera resume` lands. Judging output also keeps the existing `/evaluations/j_*` layout in this phase; Phase 3 renames it and Phase 4 adds the `/` segment. **Acknowledged compatibility break:** `vera judge` drops legacy `judge.py`'s fallback of writing evaluations to `evaluations/` relative to the working directory when the input is a flat transcript folder rather than a generation run — it errors and requires `-o/--output` instead. Accepted because that fallback detached evaluations from the conversations that produced them, and because a bare relative path resolves against the working directory on the CLI but against the repository root in a config. Reading old flat-layout conversations still works with an explicit `-o`, satisfying the read-old-data guarantee above | `pytest -m "not live"` green; `vera.py` is the only documented entry point for everything except resume; target and explicit-component paths have structural parity tests; `--config`, `--target`, `--personas`, `--rubric`, `-u`, `-j`, and `--sample` are functional; `generate.py` and `run_pipeline.py` are gone | | **2 — Scoring split** | Extract `score/` | `score.py`/`score_viz.py`/`pool.py` move out of `judge/` into `score/`; pure move, no new behavior. `vera.py` (the only entry point since Phase 1) gets its imports updated directly — no shim needed, since the legacy root scripts no longer exist. A minimal import-linter contract is added covering only the `judge/` ⊥ `score/` boundary this phase creates | `pytest -m "not live"` green; the `judge/` ⊥ `score/` import-linter contract passes; no code imports `judge.score`/`judge.pool` | | **3 — Traceability & naming** | Harden Phase 1's config shape; add persistence; swap in the new naming scheme | `utils/config_schema.py` formalizes Phase 1's informal `config.json` shape into a stable interface (design doc required for future changes) — `judging.rubrics` stays a list (continuing the list-from-day-one approach already used since Phase 0/1), still length-1-only in practice until Phase 4. Adds the persisted artifacts: `config.json` written to disk + `state.json` + `.sha256` sidecar; `utils/naming.py` (the naming/layout module, already tracked today implementing the legacy `p_`/`a_` scheme) is **rewritten** for the `c_`/`u_`/`j_` scheme, retiring the `p_*`/`j_*` layout Phase 1 kept. Existing `output/` run folders under the old layout are left alone — no migration script, only new runs use the new layout. **Acknowledged compatibility break:** anything outside `vera.py` that parses the old `p_*`/`j_*` pattern directly (`spring_scripts/`, `distribute_files.py`, `score_comparison.py`, notebooks, human-review tooling) breaks the moment new runs use `c_*`/`u_*`/`j_*` instead — accepted, since the vast majority of real usage goes through the CLI, not direct path-parsing. **This break is about auto-discovery, not about reading old data at all:** `vera judge --conversations ` and `vera score -r ` keep working against existing old-layout output, since both take an explicit path and read files by their own format, never by re-deriving meaning from the parent folder's naming pattern. What genuinely doesn't carry over is `vera resume` on an old run — `config.json`/`.sha256`/`state.json` didn't exist under the old layout, so there's nothing for `resume` to read regardless of naming scheme. Import-linter contract extended to cover `utils/` as a leaf | `pytest -m "not live"` green; a run's `config.json` round-trips through `vera resume`; `utils/` leaf-layer import-linter contract passes | | **4 — Multi-rubric support** | Support multiple rubrics per run | Built on Phase 3's `config.json`/naming: `judging.rubrics[]` now supports length > 1 (the list shape has existed since Phase 0 — this phase lifts the length-1 restriction, it doesn't introduce the list); per-rubric judge-model overrides; per-rubric `evaluations//` folder separation, one folder per rubric-providing target (the segment is named for the target the rubric came from, not for the rubric file, so it matches `c_`; with `--rubric` that may differ from the run's own target). Also enables `vera judge --target all`, deferred from Phase 1 because judging output could not be attributed to a rubric before this segment existed | `pytest -m "not live"` green; a config with 2+ rubrics produces separated `evaluations//` output for each; **and** a pre-existing length-1 `judging.rubrics` config from Phase 1-3 still produces identical behavior — backward compatibility with the single-rubric case is verified, not just the new multi-rubric case; `vera judge --target all` no longer errors and its output is attributable per target | diff --git a/utils/config_schema.py b/utils/config_schema.py index 3b5beef9a..706c31d16 100644 --- a/utils/config_schema.py +++ b/utils/config_schema.py @@ -78,6 +78,42 @@ def to_dict(self) -> dict[str, Any]: return {"name": self.name, "repeats": self.repeats, **self.extra_params} +@dataclasses.dataclass(frozen=True) +class RubricFiles: + """The three resolved files that make up one rubric. + + They travel together because a rubric is not usable without all three, and + they are named as files rather than as a manifest path because this is the + resolved form — nothing downstream re-reads a manifest to find them. + """ + + rubric_file: str + rubric_prompt_beginning_file: str + question_prompt_file: str + + def __post_init__(self) -> None: + for field in dataclasses.fields(self): + value = getattr(self, field.name) + if not isinstance(value, str) or not value: + raise ValueError(f"judging.rubrics {field.name} must be a path") + + @classmethod + def from_dict(cls, data: dict[str, Any]) -> "RubricFiles": + names = {field.name for field in dataclasses.fields(cls)} + missing = sorted(names.difference(data)) + if missing: + raise ValueError( + f"rubric is missing required field(s): {', '.join(missing)}" + ) + unknown = sorted(set(data).difference(names)) + if unknown: + raise ValueError(f"rubric has unknown field(s): {', '.join(unknown)}") + return cls(**data) + + def to_dict(self) -> dict[str, Any]: + return dataclasses.asdict(self) + + @dataclasses.dataclass(frozen=True) class GenerationConfig: """What run to perform — the run-defining half of a `RunConfig`. @@ -192,19 +228,117 @@ def to_dict(self) -> dict[str, Any]: return {"debug": self.debug, "sample": self.sample} +@dataclasses.dataclass(frozen=True) +class JudgingConfig: + """What judging run to perform — the run-defining half for `vera judge`. + + The counterpart of `GenerationConfig`, and subject to the same rule: every + field here is part of the run's identity, so all of them must come from one + input form. + + `rubrics` is list-shaped from day one per AD-20 while only length 1 is + accepted, so lifting the multi-rubric restriction later is not a schema + break. Each entry holds the three resolved rubric files rather than a + manifest path, because the resolved form names concrete files. + `conversations` is list-shaped for the same reason and likewise length 1. + """ + + models: list[ModelSpec] + conversations: list[str] + rubrics: list[RubricFiles] + output: str + max_concurrent: int | None + per_judge: bool + + def __post_init__(self) -> None: + if not self.models: + raise ValueError("judging.models must contain at least one model") + names = [model.name for model in self.models] + if len(set(names)) != len(names): + raise ValueError( + "judging.models must not repeat a model name; use repeats to run " + "several instances of the same model" + ) + # The judging domain takes one provider-parameter dict for the whole run, + # so per-model parameters cannot be honored yet. Reject them rather than + # silently applying one model's parameters to all. Drop this check when + # the domain accepts per-model parameters. + distinct_params = { + tuple(sorted(model.extra_params.items())) for model in self.models + } + if len(distinct_params) > 1: + raise ValueError( + "judging.models must all use the same provider parameters; " + "per-model judge parameters are not supported yet" + ) + if len(self.conversations) != 1: + raise ValueError( + "judging.conversations must contain exactly one folder; judge " + "each folder separately and combine the results with vera pool" + ) + if not all(isinstance(folder, str) and folder for folder in self.conversations): + raise ValueError("judging.conversations entries must be non-empty paths") + if len(self.rubrics) != 1: + raise ValueError( + "judging.rubrics must contain exactly one rubric; multi-rubric " + "support is not implemented yet" + ) + if not self.output: + raise ValueError("judging.output cannot be empty") + if self.max_concurrent is not None: + if isinstance(self.max_concurrent, bool) or not isinstance( + self.max_concurrent, int + ): + raise ValueError("judging.max_concurrent must be null or an integer") + if self.max_concurrent < 0: + raise ValueError( + "judging.max_concurrent must be null, 0, or a positive integer" + ) + if not isinstance(self.per_judge, bool): + raise ValueError("judging.per_judge must be a boolean") + + def to_dict(self) -> dict[str, Any]: + return { + "models": [model.to_dict() for model in self.models], + "conversations": list(self.conversations), + "rubrics": [rubric.to_dict() for rubric in self.rubrics], + "output": self.output, + "max_concurrent": self.max_concurrent, + "per_judge": self.per_judge, + } + + @dataclasses.dataclass(frozen=True) class RunConfig: - """One fully resolved `vera generate` run, ready to execute. + """One fully resolved `vera` run, ready to execute. + + Holds one section per command taking part in the run. `generate` populates + `generation`, `judge` populates `judging`, and a later `pipeline` populates + both; at least one is required. `--target all` resolves to one `RunConfig` per target; every other input resolves to exactly one. """ invocation: InvocationConfig - generation: GenerationConfig + generation: GenerationConfig | None = None + judging: JudgingConfig | None = None + + def __post_init__(self) -> None: + if self.generation is None and self.judging is None: + raise ValueError("a run must define generation, judging, or both") def to_dict(self) -> dict[str, Any]: - return { - "invocation": self.invocation.to_dict(), - "generation": self.generation.to_dict(), - } + """Serialize, omitting sections this run does not define. + + Absent sections are left out rather than emitted as null. That keeps the + output a valid input config: a command rejects top-level fields it does + not own, so a `null` judging section would make `vera generate --print` + emit something `vera generate` itself refuses. + """ + config: dict[str, Any] = {"invocation": self.invocation.to_dict()} + if self.generation is not None: + config["generation"] = self.generation.to_dict() + if self.judging is not None: + config["judging"] = self.judging.to_dict() + return config diff --git a/vera.py b/vera.py index 64aca55a7..22858d590 100644 --- a/vera.py +++ b/vera.py @@ -8,6 +8,7 @@ from vera_cli.config import ConfigError from vera_cli.generate import register as register_generate +from vera_cli.judge import register as register_judge def build_parser() -> argparse.ArgumentParser: @@ -16,6 +17,7 @@ def build_parser() -> argparse.ArgumentParser: # Each command owns its flags and attaches its handler to its subparser. register_generate(subparsers) + register_judge(subparsers) return parser diff --git a/vera_cli/README.md b/vera_cli/README.md index 20a21f33a..75bb7fc98 100644 --- a/vera_cli/README.md +++ b/vera_cli/README.md @@ -4,7 +4,7 @@ This package is the argument-and-config layer behind `vera.py`. It resolves user input into canonical values and then calls domain functions. It contains no generation, judging, or scoring logic. -Currently implemented: `generate`. `judge`, `score`, `pool`, `pipeline`, and +Currently implemented: `generate` and `judge`. `score`, `pool`, `pipeline`, and `resume` are specified in [../docs/architecture.md](../docs/architecture.md) but not built yet. @@ -14,6 +14,7 @@ not built yet. |---|---| | `../vera.py` | Root parser and dispatcher. Registers each command, routes to its handler, turns `ConfigError` into a standard CLI error. | | `generate.py` | The `generate` command: flags, input resolution, and the call into the generation domain. The reference implementation for new commands. | +| `judge.py` | The `judge` command, same five steps. Differs from legacy `judge.py` in three ways, all recorded in [../docs/vera-cli-use-cases.md](../docs/vera-cli-use-cases.md): no `--resume`, no single-conversation mode, and no implicit working-directory output. | | `config.py` | Shared input handling: loading config JSON, the config-or-flags rule, path resolution, resolved-run rendering. | | `targets.py` | Target discovery and manifest validation — turning a target name into concrete, verified file paths. | | `../utils/config_schema.py` | *Not a CLI module.* Shared canonical types (`RunConfig` and friends) in the leaf `utils/` layer, so domain packages may consume them too. Validation and serialization only; no parsing, no defaults. | diff --git a/vera_cli/config.py b/vera_cli/config.py index 033469cee..d395140a1 100644 --- a/vera_cli/config.py +++ b/vera_cli/config.py @@ -2,6 +2,7 @@ from __future__ import annotations +import dataclasses import json import os import shlex @@ -89,6 +90,27 @@ def model_from_config(value: Any, *, field: str) -> ModelSpec: return ModelSpec.from_dict(value) +def models_from_cli( + tokens: list[str], role_params: dict[str, Any] | None +) -> list[ModelSpec]: + """Build `ModelSpec`s from CLI `name[:repeats]` tokens plus role parameters. + + Provider parameters are supplied per *role* on the command line (one + `--*-params` flag covering every model of that role), matching what the + legacy scripts accepted. The resolved form stays per-model: each `ModelSpec` + gets its own copy, so `--print` shows exactly what each model will use and a + printed config can then be edited per model. + + Per-model differentiation is a config-only capability; the CLI shorthand has + no room to express it. + """ + params = dict(role_params or {}) + return [ + dataclasses.replace(ModelSpec.from_shorthand(token), extra_params=dict(params)) + for token in tokens + ] + + def models_from_config(value: Any, *, field: str) -> list[ModelSpec]: """Build a `ModelSpec` list from a config array of objects.""" if not isinstance(value, list) or not all(isinstance(item, dict) for item in value): diff --git a/vera_cli/generate.py b/vera_cli/generate.py index c4c4ed2dd..383d611f2 100644 --- a/vera_cli/generate.py +++ b/vera_cli/generate.py @@ -22,10 +22,12 @@ from generate import run_for_user_models from utils.config_schema import GenerationConfig, InvocationConfig, ModelSpec, RunConfig from utils.debug import set_debug +from utils.utils import parse_key_value_list from .config import ( ConfigError, model_from_config, + models_from_cli, models_from_config, path_from_root, print_resolved_config, @@ -34,10 +36,12 @@ resolve_input, ) from .targets import ( - generation_persona_sets, + config_path, + config_paths, load_target, resolve_target_manifest, target_manifest_paths, + targets_from_config, ) # CLI behavior defaults. They live here, beside the flag definitions, rather than @@ -81,8 +85,9 @@ # on the command line its contents are spelled as individual flags. # # `invocation` is always allowed and is added by `resolve_input`. `judging` is -# deliberately absent: until `vera judge` exists there is nothing to do with it, -# and accepting a key this command ignores is worse than rejecting it. +# deliberately absent: `vera judge` owns that section, and accepting a key this +# command would ignore is worse than rejecting it. A later `pipeline` accepts +# both. ALLOWED_CONFIG_FIELDS = {"generation", "target"} @@ -170,6 +175,20 @@ def register(subparsers: argparse._SubParsersAction) -> None: "(default: one session, using the chatbot's own session type)" ), ) + parser.add_argument( + "--user-params", + type=parse_key_value_list, + default=argparse.SUPPRESS, + metavar="k=v[,k=v...]", + help="Provider parameters applied to every -u model (default: none)", + ) + parser.add_argument( + "--chatbot-params", + type=parse_key_value_list, + default=argparse.SUPPRESS, + metavar="k=v[,k=v...]", + help="Provider parameters applied to the -c model (default: none)", + ) parser.add_argument("--config", help="JSON path or '-' for stdin") parser.add_argument( "--sample", @@ -277,8 +296,10 @@ def _from_cli( return [ _run_config( invocation, - chatbot=ModelSpec.from_shorthand(chatbot), - users=[ModelSpec.from_shorthand(user) for user in users], + chatbot=models_from_cli([chatbot], getattr(args, "chatbot_params", None))[ + 0 + ], + users=models_from_cli(users, getattr(args, "user_params", None)), personas=resolved.personas, persona_context_template=resolved.persona_context_template, turns=_value(args, "turns"), @@ -330,6 +351,34 @@ def _from_config( raise ConfigError("generation.output must be a path string") behavior["output"] = path_from_root(behavior["output"]) + targets = targets_from_config( + config, + generation, + explicit_fields=("personas", "persona_context_template"), + section_name="generation", + ) + if targets is not None: + persona_sets = [ + (target.personas, target.persona_context_template) for target in targets + ] + else: + persona_sets = [ + ( + config_paths( + required(generation, "personas", section="generation config"), + field="generation.personas", + ), + config_path( + required( + generation, + "persona_context_template", + section="generation config", + ), + field="generation.persona_context_template", + ), + ) + ] + return [ _run_config( invocation, @@ -339,7 +388,7 @@ def _from_config( persona_context_template=context, **behavior, ) - for personas, context in generation_persona_sets(config, generation) + for personas, context in persona_sets ] @@ -413,6 +462,7 @@ async def _execute(run_configs: list[RunConfig]) -> None: second-granularity timestamps, so concurrent starts would collide. """ for run_config in run_configs: - await run_for_user_models( - run_config.generation, max_personas=run_config.invocation.sample - ) + generation = run_config.generation + if generation is None: # pragma: no cover - resolve_configs always sets it + raise ConfigError("generate produced a run with no generation section") + await run_for_user_models(generation, max_personas=run_config.invocation.sample) diff --git a/vera_cli/judge.py b/vera_cli/judge.py new file mode 100644 index 000000000..9d40ab91b --- /dev/null +++ b/vera_cli/judge.py @@ -0,0 +1,498 @@ +"""The ``vera judge`` command: flags, resolution, and the workflow call. + +Structured exactly like `vera_cli/generate.py`, which is the reference +implementation of the command contract in `vera_cli/README.md`: + +1. `register` declares the flags and attaches `run` as the subparser's handler. +2. `run` is the entry point `vera.py` dispatches to. +3. `resolve_configs` picks one input form and produces canonical `RunConfig`s. +4. `_execute` hands each resolved run to the judging domain. + +Nothing below step 3 reads an `argparse.Namespace`, and nothing above it touches +the judging domain. + +Two deliberate differences from legacy `judge.py`, both recorded in +docs/vera-cli-use-cases.md: + +- No `--resume`. The resume contract is deferred, so resuming stays available + only through legacy `judge.py` until `vera resume` exists. +- No single-conversation mode. Judge a folder containing one conversation. +""" + +from __future__ import annotations + +import argparse +import asyncio +from pathlib import Path +from typing import Any + +from judge import run_judging +from utils.config_schema import ( + InvocationConfig, + JudgingConfig, + ModelSpec, + RubricFiles, + RunConfig, +) +from utils.conversation_layout import resolve_conversation_input +from utils.debug import set_debug +from utils.utils import parse_key_value_list + +from .config import ( + ConfigError, + models_from_cli, + models_from_config, + path_from_root, + print_resolved_config, + render_invocation, + required, + resolve_input, +) +from .targets import ( + config_path, + load_target, + resolve_target_manifest, + targets_from_config, +) + +# CLI behavior defaults, applied during resolution by `_value` rather than by the +# parser — see `vera_cli/generate.py` for why the parser cannot hold them. +# +# `output` has no static default: it is derived from the conversations folder, +# landing beside the transcripts it evaluates. `-h` says so. +DEFAULTS: dict[str, Any] = { + "output": None, + "max_concurrent": None, + "per_judge": False, +} + +# Flags that do not define the run. Note `--sample` doubles as the debug cap for +# both commands: it caps personas per file for `generate` and conversations +# loaded for `judge`, so `InvocationConfig` stays uniform across commands rather +# than growing a second per-command cap. +INVOCATION_ONLY_FLAGS = frozenset({"config", "sample", "debug", "print_only"}) + +# Top-level config keys `judge` accepts. `generation` is absent for the same +# reason `generate` rejects `judging`: a key this command would ignore is worse +# rejected than accepted. A later `pipeline` accepts both. +ALLOWED_CONFIG_FIELDS = {"judging", "target"} + + +def register(subparsers: argparse._SubParsersAction) -> None: + """Register ``judge`` with the root parser. + + Uses the same `argparse.SUPPRESS` convention as `generate`: run-defining + flags are absent from the namespace unless the user passed them, which is + what makes the config-or-flags rule enforceable. + + Note `-c` is deliberately *not* accepted. Judging is decoupled from chatbot + selection by design, and in legacy `judge.py` `-c` meant `--conversation`, + which this command does not have. + """ + parser = subparsers.add_parser("judge", help="Evaluate conversations") + parser.add_argument( + "-j", + "--judge", + nargs="+", + metavar="[:]", + default=argparse.SUPPRESS, + help="Judge model(s) and how many instances of each to run", + ) + parser.add_argument( + "--conversations", + nargs="+", + metavar="", + default=argparse.SUPPRESS, + help=( + "Conversation run folder to judge (exactly one; judge folders " + "separately and combine with 'vera pool')" + ), + ) + target = parser.add_mutually_exclusive_group() + target.add_argument( + "--target", + default=argparse.SUPPRESS, + help="Complete target name or manifest path supplying the rubric", + ) + target.add_argument( + "--rubric", + default=argparse.SUPPRESS, + help="Target name or manifest path whose rubric and prompts should be used", + ) + parser.add_argument( + "-o", + "--output", + default=argparse.SUPPRESS, + help=( + "Parent directory for the evaluation run folder " + "(default: /evaluations/)" + ), + ) + parser.add_argument( + "--max-concurrent", + type=int, + default=argparse.SUPPRESS, + help="Maximum concurrent judge workers (default: unlimited)", + ) + parser.add_argument( + "--per-judge", + action="store_true", + default=argparse.SUPPRESS, + help=( + "Apply --max-concurrent per judge model rather than across all " + "(default: across all)" + ), + ) + parser.add_argument( + "--judge-params", + type=parse_key_value_list, + default=argparse.SUPPRESS, + metavar="k=v[,k=v...]", + help="Provider parameters applied to every -j model (default: none)", + ) + parser.add_argument("--config", help="JSON path or '-' for stdin") + parser.add_argument( + "--sample", + type=int, + default=argparse.SUPPRESS, + help="Debug-only cap on conversations judged", + ) + parser.add_argument( + "-d", + "--debug", + action="store_true", + default=argparse.SUPPRESS, + help="Enable debug logging", + ) + parser.add_argument( + "--print", + action="store_true", + dest="print_only", + help="Print the resolved invocation without executing it", + ) + + parser.set_defaults(handler=run) + + +def run(args: argparse.Namespace) -> int: + """Resolve the requested run(s) and execute them. + + Resolution happens up front and completely, so an invalid target, missing + rubric file, or underivable output location fails before any model is called. + """ + run_configs = resolve_configs(args) + if args.print_only: + for run_config in run_configs: + print(render_invocation(run_config, command="judge")) + return 0 + + if any(config.invocation.debug for config in run_configs): + set_debug(True) + for run_config in run_configs: + print_resolved_config(run_config) + asyncio.run(_execute(run_configs)) + return 0 + + +def resolve_configs(args: argparse.Namespace) -> list[RunConfig]: + """Resolve either config JSON or CLI flags into canonical runs. + + Always returns exactly one `RunConfig`. Unlike `generate`, `--target all` is + rejected rather than fanned out — see `_target_selection`. + """ + try: + config, invocation = resolve_input( + args, + invocation_only_flags=INVOCATION_ONLY_FLAGS, + allowed_config_fields=ALLOWED_CONFIG_FIELDS, + ) + return ( + _from_config(config, invocation) + if config is not None + else _from_cli(args, invocation) + ) + except ConfigError: + raise + except (TypeError, ValueError) as error: + raise ConfigError(f"invalid judging config: {error}") from error + + +def _reject_target_all(selection: str) -> str: + """Reject `all` for judging, which cannot yet attribute its output. + + Judging every target means evaluating the same conversations under N rubrics. + That resolves cleanly, but every run would land in the same + `/evaluations/` distinguishable only by timestamp, because the judge run + folder name encodes the judge model and time, not the rubric. Erroring beats + writing output nobody can attribute. Lifted in Phase 4, which adds the + `evaluations//` segment (see docs/architecture.md). + """ + if selection.casefold() == "all": + raise ConfigError( + "judge does not support --target all yet: evaluations for different " + "rubrics would share one output folder and could not be told apart. " + "Judge one target at a time." + ) + return selection + + +def _rubric_from_target(selection: str) -> RubricFiles: + """Resolve a target name or manifest path to its three rubric files.""" + target = load_target(resolve_target_manifest(_reject_target_all(selection))) + return RubricFiles( + rubric_file=target.rubric, + rubric_prompt_beginning_file=target.rubric_prompt_beginning, + question_prompt_file=target.question_prompt, + ) + + +def _output_root(conversations: str, output: str | None) -> str: + """Decide where the evaluation run folder goes. + + Defaults beside the transcripts being judged, at + `/evaluations/`, so the output records what produced it. + + When the input is not a recognizable generation run — a legacy flat folder of + `.txt` files — there is nothing to derive from, and `-o` is required. Legacy + `judge.py` instead wrote to `evaluations/` relative to the working directory; + that silently detached results from their input and is not carried over. See + the breaking-change note in CHANGELOG.md. + """ + if output is not None: + return str(Path(output).resolve()) + + _, generation_run, _ = resolve_conversation_input(conversations) + if generation_run is None: + raise ConfigError( + f"cannot derive an output location from {conversations}: it is not a " + "generation run folder. Pass -o/--output to say where evaluations " + "should go." + ) + return str((Path(generation_run) / "evaluations").resolve()) + + +def _from_cli( + args: argparse.Namespace, invocation: InvocationConfig +) -> list[RunConfig]: + """Resolve CLI flags into one canonical run, applying CLI defaults. + + `--target` and `--rubric` differ only in intent, exactly as `--target` and + `--personas` do for `generate`: the first names a whole bundle, the second + names the rubric component explicitly. Both resolve through the same manifest + to the same three files. + """ + models: list[str] | None = getattr(args, "judge", None) + conversations: list[str] | None = getattr(args, "conversations", None) + target: str | None = getattr(args, "target", None) + rubric: str | None = getattr(args, "rubric", None) + + # The parser cannot enforce these: a config may supply the same values, and + # the flags use SUPPRESS so absence is indistinguishable from a default. The + # target/rubric group enforces "not both" but cannot require one. + if not models: + raise ConfigError("judge requires at least one -j/--judge model") + if not conversations: + raise ConfigError("judge requires --conversations") + if target: + rubric_files = _rubric_from_target(target) + elif rubric: + rubric_files = _rubric_from_target(rubric) + else: + raise ConfigError("judge requires --target or --rubric") + + folders = [str(Path(folder).resolve()) for folder in conversations] + return [ + _run_config( + invocation, + models=models_from_cli(models, getattr(args, "judge_params", None)), + conversations=folders, + rubrics=[rubric_files], + output=_output_root(folders[0], _value(args, "output")), + max_concurrent=_value(args, "max_concurrent"), + per_judge=_value(args, "per_judge"), + ) + ] + + +def _from_config( + config: dict[str, Any], invocation: InvocationConfig +) -> list[RunConfig]: + """Resolve a config object into one canonical run. + + Every behavior field is required rather than defaulted, matching `generate`: + a stored config is a complete, reproducible description of a run, so a value + it does not state is an error rather than something this code fills in. + """ + value = config.get("judging") + if not isinstance(value, dict): + raise ConfigError("judge requires a judging config object") + judging = dict(value) + models = models_from_config( + required(judging, "models", section="judging config"), + field="judging.models", + ) + conversations = [ + config_dir(folder, field="judging.conversations") + for folder in _string_list( + required(judging, "conversations", section="judging config"), + field="judging.conversations", + ) + ] + + targets = targets_from_config( + config, + judging, + explicit_fields=("rubrics",), + section_name="judging", + ) + if targets is not None: + if len(targets) != 1: + raise ConfigError( + "judge does not support target 'all' yet: evaluations for " + "different rubrics would share one output folder" + ) + rubrics = [ + RubricFiles( + rubric_file=targets[0].rubric, + rubric_prompt_beginning_file=targets[0].rubric_prompt_beginning, + question_prompt_file=targets[0].question_prompt, + ) + ] + else: + rubrics = _rubrics_from_config( + required(judging, "rubrics", section="judging config") + ) + + output = required(judging, "output", section="judging config") + if not isinstance(output, str) or not output: + raise ConfigError("judging.output must be a path string") + + return [ + _run_config( + invocation, + models=models, + conversations=conversations, + rubrics=rubrics, + output=path_from_root(output), + max_concurrent=required( + judging, "max_concurrent", section="judging config" + ), + per_judge=required(judging, "per_judge", section="judging config"), + ) + ] + + +def _string_list(value: Any, *, field: str) -> list[str]: + """Validate a config value is a non-empty list of non-empty strings.""" + if ( + not isinstance(value, list) + or not value + or not all(isinstance(item, str) and item for item in value) + ): + raise ConfigError(f"{field} must be a non-empty list of paths") + return value + + +def config_dir(value: str, *, field: str) -> str: + """Resolve a config-supplied directory against the repository root. + + The path-list helpers in `targets` verify *files*; a conversations folder is + a directory, so it gets its own check. + """ + resolved = Path(path_from_root(value)) + if not resolved.is_dir(): + raise ConfigError(f"{field} does not exist or is not a directory: {resolved}") + return str(resolved) + + +def _rubrics_from_config(value: Any) -> list[RubricFiles]: + """Build `RubricFiles` from explicit config entries, resolving each path.""" + if not isinstance(value, list) or not value: + raise ConfigError("judging.rubrics must be a non-empty list of objects") + rubrics = [] + for entry in value: + if not isinstance(entry, dict): + raise ConfigError("judging.rubrics entries must be objects") + files = RubricFiles.from_dict(entry) + rubrics.append( + RubricFiles( + **{ + field: config_path( + getattr(files, field), field=f"judging.rubrics.{field}" + ) + for field in ( + "rubric_file", + "rubric_prompt_beginning_file", + "question_prompt_file", + ) + } + ) + ) + return rubrics + + +def _value(args: argparse.Namespace, field: str) -> Any: + """Read a run-defining flag, falling back to its CLI default.""" + return getattr(args, field, DEFAULTS[field]) + + +def _run_config( + invocation: InvocationConfig, + *, + models: list[ModelSpec], + conversations: list[str], + rubrics: list[RubricFiles], + output: str, + max_concurrent: int | None, + per_judge: bool, +) -> RunConfig: + """Assemble and validate one canonical `RunConfig` holding a judging section. + + Fields are named rather than forwarded as opaque keywords so this signature + states what a judging run consists of. Type enforcement happens at runtime in + `JudgingConfig.__post_init__`. + """ + return RunConfig( + invocation=invocation, + judging=JudgingConfig( + models=models, + conversations=conversations, + rubrics=rubrics, + output=output, + max_concurrent=max_concurrent, + per_judge=per_judge, + ), + ) + + +async def _execute(run_configs: list[RunConfig]) -> None: + """Hand each resolved run to the judging domain.""" + for run_config in run_configs: + judging = run_config.judging + if judging is None: # pragma: no cover - resolve_configs always sets it + raise ConfigError("judge produced a run with no judging section") + rubric = judging.rubrics[0] + + # Discovery of the transcripts directory is idempotent, so deriving it + # here keeps the resolved config stating the folder the user named rather + # than an internal subdirectory. + transcripts_dir, _, folder_name = resolve_conversation_input( + judging.conversations[0] + ) + await run_judging( + judge_models={model.name: model.repeats for model in judging.models}, + rubric_file=rubric.rubric_file, + rubric_prompt_beginning_file=rubric.rubric_prompt_beginning_file, + question_prompt_file=rubric.question_prompt_file, + transcripts_dir=transcripts_dir, + conversation_folder_name=folder_name, + limit=run_config.invocation.sample, + output_root=judging.output, + output_folder=None, + judge_model_extra_params=dict(judging.models[0].extra_params), + max_concurrent=judging.max_concurrent, + per_judge=judging.per_judge, + verbose_workers=False, + verbose=True, + resume=False, + ) diff --git a/vera_cli/targets.py b/vera_cli/targets.py index 39151ce41..92b8c687b 100644 --- a/vera_cli/targets.py +++ b/vera_cli/targets.py @@ -15,7 +15,7 @@ import json from pathlib import Path -from .config import ROOT, ConfigError, existing_file, path_from_root, required +from .config import ROOT, ConfigError, existing_file, path_from_root # A manifest must describe a *complete* target: enough for both generation and # judging. Partial bundles are rejected at resolution time rather than failing @@ -148,61 +148,62 @@ def resolve_file(field: str, path: object) -> str: ) -def generation_persona_sets( - config: dict[str, object], generation: dict[str, object] -) -> list[tuple[list[str], str]]: - """Resolve a config's persona inputs into one `(persona files, context)` pair - per run. +def targets_from_config( + config: dict[str, object], + section: dict[str, object], + *, + explicit_fields: tuple[str, ...], + section_name: str, +) -> list[ResolvedTarget] | None: + """Resolve a config's top-level `target`, or report that it has none. - Config states persona inputs one of two ways, and this collapses both to the - same output: + Returns one `ResolvedTarget` per selected target, or `None` when the config + states its components explicitly instead, in which case the caller reads + those fields itself. - - a top-level `target` name, whose manifest supplies the persona files and - context template. `target: "all"` selects every target, which is the only - case that returns more than one pair. - - explicit `generation.personas` and `generation.persona_context_template` - paths, which always yield exactly one pair. + This owns the two rules every command shares, so they cannot drift: - The two are mutually exclusive: a target already determines these values, so - also naming them explicitly is a contradiction rather than an override. + - `target: "all"` selects every discovered target, and is the only input that + yields more than one. Anything else yields exactly one. + - A target and the explicit fields it would supply are mutually exclusive. A + target already determines those values, so naming them too is a + contradiction rather than an override. + + Projecting a `ResolvedTarget` onto the fields a command needs is left to the + caller: `generate` takes personas and the context template, `judge` takes the + rubric and its prompts. Those differ per command; the rules above do not. """ target = config.get("target") - if target is not None: - if not isinstance(target, str) or not target: - raise ConfigError("target must be a non-empty string") - overlap = {"personas", "persona_context_template"}.intersection(generation) - if overlap: - raise ConfigError( - "target is mutually exclusive with explicit generation fields: " - f"{', '.join(sorted(overlap))}" - ) - return [ - (resolved.personas, resolved.persona_context_template) - for resolved in ( - load_target(manifest) for manifest in target_manifest_paths(target) - ) - ] - - personas = required(generation, "personas", section="generation config") - context = required( - generation, "persona_context_template", section="generation config" - ) + if target is None: + return None + if not isinstance(target, str) or not target: + raise ConfigError("target must be a non-empty string") + overlap = set(explicit_fields).intersection(section) + if overlap: + raise ConfigError( + f"target is mutually exclusive with explicit {section_name} fields: " + f"{', '.join(sorted(overlap))}" + ) + return [load_target(manifest) for manifest in target_manifest_paths(target)] + + +def config_path(value: object, *, field: str) -> str: + """Resolve one explicit config path against the repository root, verifying it. + + Shared by every command's explicit-component branch, so "a config path is + repo-relative and must exist" is stated once. + """ + if not isinstance(value, str) or not value: + raise ConfigError(f"{field} must be a path") + return existing_file(path_from_root(value), field=field) + + +def config_paths(value: object, *, field: str) -> list[str]: + """Resolve a non-empty list of explicit config paths, verifying each.""" if ( - not isinstance(personas, list) - or not personas - or not all(isinstance(persona, str) and persona for persona in personas) + not isinstance(value, list) + or not value + or not all(isinstance(item, str) and item for item in value) ): - raise ConfigError("generation.personas must be a non-empty list of paths") - if not isinstance(context, str) or not context: - raise ConfigError("generation.persona_context_template must be a path") - return [ - ( - [ - existing_file(path_from_root(persona), field="generation.personas") - for persona in personas - ], - existing_file( - path_from_root(context), field="generation.persona_context_template" - ), - ) - ] + raise ConfigError(f"{field} must be a non-empty list of paths") + return [config_path(item, field=field) for item in value] From 867f1ca30015101b20b76ec2e6c5b1cc528d03f7 Mon Sep 17 00:00:00 2001 From: Luca Belli <129434630+sator-labs@users.noreply.github.com> Date: Thu, 13 Aug 2026 17:27:54 -0700 Subject: [PATCH 2/2] test: cover the vera judge command MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Twenty tests mirroring tests/unit/test_vera_cli.py, focused on the decisions that are easy to regress rather than on restating the parser. Behavior parity: `--target` and `--rubric` resolve to identical configs, and output defaults beside the conversation run. The judge-specific rejections each get a test, since every one of them exists to prevent silently wrong output: `--target all`, more than one conversations folder, repeated judge model names, differing per-model provider parameters, and a flat folder with no `-o` -- plus the matching case showing an explicit `-o` still works against a flat folder. Two tests guard the multi-section schema. One asserts a judging run omits the `generation` key entirely rather than emitting null, since `to_dict` output doubles as input config. The other round-trips a resolved run through `VERA_RUN_CONFIG` and compares against the original object. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 5 (1M context) --- tests/unit/test_vera_judge.py | 369 ++++++++++++++++++++++++++++++++++ 1 file changed, 369 insertions(+) create mode 100644 tests/unit/test_vera_judge.py diff --git a/tests/unit/test_vera_judge.py b/tests/unit/test_vera_judge.py new file mode 100644 index 000000000..da7e2467d --- /dev/null +++ b/tests/unit/test_vera_judge.py @@ -0,0 +1,369 @@ +"""Tests for the ``vera judge`` command.""" + +from __future__ import annotations + +import json +from pathlib import Path +from unittest.mock import AsyncMock, patch + +import pytest + +import vera +from vera_cli import config as cli_config +from vera_cli import judge + + +@pytest.fixture(autouse=True) +def clear_env_config(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv(cli_config.VERA_RUN_CONFIG_ENV, raising=False) + + +def _generation_run(tmp_path: Path, *, conversations: int = 1) -> Path: + """Create a folder shaped like a generation run, which judge derives output from.""" + run = tmp_path / "p_user__a_bot__t5__r1" + transcripts = run / "conversations" + transcripts.mkdir(parents=True) + for index in range(conversations): + (transcripts / f"c{index}.txt").write_text("transcript", encoding="utf-8") + return run + + +def _flat_folder(tmp_path: Path) -> Path: + """Create a legacy flat folder of transcripts, with no derivable run root.""" + folder = tmp_path / "flat" + folder.mkdir(parents=True) + (folder / "c.txt").write_text("transcript", encoding="utf-8") + return folder + + +def _write_config(tmp_path: Path, data: dict) -> Path: + path = tmp_path / "run.json" + path.write_text(json.dumps(data), encoding="utf-8") + return path + + +def _judging_config(tmp_path: Path, **overrides: object) -> dict: + judging: dict[str, object] = { + "models": [{"name": "gpt-4o", "repeats": 1}], + "conversations": [str(_generation_run(tmp_path))], + "rubrics": [ + { + "rubric_file": "data/SI/rubric.tsv", + "rubric_prompt_beginning_file": "data/SI/rubric_prompt_beginning.txt", + "question_prompt_file": "data/SI/question_prompt.txt", + } + ], + "output": "output", + "max_concurrent": None, + "per_judge": False, + } + judging.update(overrides) + return {"judging": judging} + + +def test_judge_is_registered_and_has_help() -> None: + parser = vera.build_parser() + with pytest.raises(SystemExit) as exit_info: + parser.parse_args(["judge", "--help"]) + assert exit_info.value.code == 0 + + +def test_target_and_rubric_resolve_the_same_rubric(tmp_path: Path) -> None: + """`--target` and `--rubric` differ in intent only, like generate's --personas.""" + run = _generation_run(tmp_path) + parser = vera.build_parser() + base = ["judge", "-j", "gpt-4o", "--conversations", str(run)] + + from_target = judge.resolve_configs(parser.parse_args([*base, "--target", "SI"])) + from_rubric = judge.resolve_configs(parser.parse_args([*base, "--rubric", "SI"])) + + assert from_target == from_rubric + + +def test_output_defaults_beside_the_conversation_run(tmp_path: Path) -> None: + run = _generation_run(tmp_path) + args = vera.build_parser().parse_args( + ["judge", "-j", "gpt-4o", "--conversations", str(run), "--target", "SI"] + ) + + judging = judge.resolve_configs(args)[0].judging + assert judging is not None + assert judging.output == str((run / "evaluations").resolve()) + + +def test_flat_folder_requires_explicit_output(tmp_path: Path) -> None: + """The legacy cwd-relative `evaluations/` fallback is gone; -o is required.""" + folder = _flat_folder(tmp_path) + + with pytest.raises(SystemExit) as error: + vera.main( + ["judge", "-j", "gpt-4o", "--conversations", str(folder), "--target", "SI"] + ) + + assert error.value.code == 2 + + +def test_flat_folder_works_with_explicit_output(tmp_path: Path) -> None: + folder = _flat_folder(tmp_path) + destination = tmp_path / "evals" + args = vera.build_parser().parse_args( + [ + "judge", + "-j", + "gpt-4o", + "--conversations", + str(folder), + "--target", + "SI", + "-o", + str(destination), + ] + ) + + judging = judge.resolve_configs(args)[0].judging + assert judging is not None + assert judging.output == str(destination.resolve()) + + +def test_target_all_is_rejected(tmp_path: Path) -> None: + """Deferred to Phase 4: N rubrics would share one output folder.""" + run = _generation_run(tmp_path) + + with pytest.raises(SystemExit) as error: + vera.main( + ["judge", "-j", "gpt-4o", "--conversations", str(run), "--target", "all"] + ) + + assert error.value.code == 2 + + +def test_multiple_conversation_folders_are_rejected(tmp_path: Path) -> None: + """Exactly one folder; judge separately and combine with vera pool.""" + first = _generation_run(tmp_path / "a") + second = _generation_run(tmp_path / "b") + + with pytest.raises(SystemExit) as error: + vera.main( + [ + "judge", + "-j", + "gpt-4o", + "--conversations", + str(first), + str(second), + "--target", + "SI", + ] + ) + + assert error.value.code == 2 + + +def test_judge_params_apply_to_every_model(tmp_path: Path) -> None: + run = _generation_run(tmp_path) + args = vera.build_parser().parse_args( + [ + "judge", + "-j", + "gpt-4o:2", + "claude:1", + "--conversations", + str(run), + "--target", + "SI", + "--judge-params", + "temperature=0,max_tokens=500", + ] + ) + + judging = judge.resolve_configs(args)[0].judging + assert judging is not None + assert [model.extra_params for model in judging.models] == [ + {"temperature": 0, "max_tokens": 500}, + {"temperature": 0, "max_tokens": 500}, + ] + + +def test_repeated_model_name_is_rejected(tmp_path: Path) -> None: + """The domain keys judge models by name, so duplicates would silently collapse.""" + run = _generation_run(tmp_path) + + with pytest.raises(SystemExit) as error: + vera.main( + [ + "judge", + "-j", + "gpt-4o", + "gpt-4o", + "--conversations", + str(run), + "--target", + "SI", + ] + ) + + assert error.value.code == 2 + + +def test_per_model_judge_params_are_rejected(tmp_path: Path) -> None: + """The domain takes one params dict, so differing per-model params must error.""" + config_data = _judging_config( + tmp_path, + models=[ + {"name": "gpt-4o", "repeats": 1, "temperature": 0}, + {"name": "claude", "repeats": 1, "temperature": 1}, + ], + ) + + with pytest.raises(SystemExit) as error: + vera.main(["judge", "--config", str(_write_config(tmp_path, config_data))]) + + assert error.value.code == 2 + + +def test_judge_requires_target_or_rubric(tmp_path: Path) -> None: + run = _generation_run(tmp_path) + + with pytest.raises(SystemExit) as error: + vera.main(["judge", "-j", "gpt-4o", "--conversations", str(run)]) + + assert error.value.code == 2 + + +def test_judge_requires_conversations() -> None: + with pytest.raises(SystemExit) as error: + vera.main(["judge", "-j", "gpt-4o", "--target", "SI"]) + + assert error.value.code == 2 + + +def test_config_rejects_generation_section(tmp_path: Path) -> None: + """`judge` rejects a generation block rather than silently ignoring it.""" + config_data = _judging_config(tmp_path) + config_data["generation"] = {"chatbot": {"name": "x", "repeats": 1}} + + with pytest.raises(SystemExit) as error: + vera.main(["judge", "--config", str(_write_config(tmp_path, config_data))]) + + assert error.value.code == 2 + + +def test_config_rejects_run_defining_cli_flag(tmp_path: Path) -> None: + config = _write_config(tmp_path, _judging_config(tmp_path)) + + with pytest.raises(SystemExit) as error: + vera.main(["judge", "--config", str(config), "-j", "gpt-4o"]) + + assert error.value.code == 2 + + +def test_config_target_rejects_explicit_rubrics(tmp_path: Path) -> None: + config_data = _judging_config(tmp_path) + config_data["target"] = "SI" + + with pytest.raises(SystemExit) as error: + vera.main(["judge", "--config", str(_write_config(tmp_path, config_data))]) + + assert error.value.code == 2 + + +def test_config_paths_resolve_from_repository_root(tmp_path: Path) -> None: + config = _write_config(tmp_path, _judging_config(tmp_path)) + args = vera.build_parser().parse_args(["judge", "--config", str(config)]) + + judging = judge.resolve_configs(args)[0].judging + assert judging is not None + assert judging.rubrics[0].rubric_file == str( + (cli_config.ROOT / "data/SI/rubric.tsv").resolve() + ) + assert judging.output == str((cli_config.ROOT / "output").resolve()) + + +def test_resolved_run_omits_the_generation_section(tmp_path: Path) -> None: + """A judging run must not emit `generation: null`. + + `to_dict` output doubles as input config, and `generate` rejects unknown + top-level fields, so a null section would make `--print` emit something the + other command refuses. + """ + run = _generation_run(tmp_path) + args = vera.build_parser().parse_args( + ["judge", "-j", "gpt-4o", "--conversations", str(run), "--target", "SI"] + ) + + resolved = judge.resolve_configs(args)[0].to_dict() + assert "generation" not in resolved + assert set(resolved) == {"invocation", "judging"} + + +def test_print_round_trips_through_the_environment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch +) -> None: + """`--print` emits a config that reproduces the same resolved run.""" + run = _generation_run(tmp_path) + parser = vera.build_parser() + first = judge.resolve_configs( + parser.parse_args( + ["judge", "-j", "gpt-4o:2", "--conversations", str(run), "--target", "SI"] + ) + )[0] + + monkeypatch.setenv(cli_config.VERA_RUN_CONFIG_ENV, json.dumps(first.to_dict())) + replayed = judge.resolve_configs(parser.parse_args(["judge"])) + + assert replayed == [first] + + +def test_sample_caps_conversations_judged(tmp_path: Path) -> None: + """`--sample` is the shared debug cap; for judge it limits conversations.""" + run = _generation_run(tmp_path, conversations=3) + with patch.object(judge, "run_judging", new_callable=AsyncMock) as run_judging: + vera.main( + [ + "judge", + "-j", + "gpt-4o", + "--conversations", + str(run), + "--target", + "SI", + "--sample", + "2", + ] + ) + + assert run_judging.await_count == 1 + assert run_judging.await_args is not None + assert run_judging.await_args.kwargs["limit"] == 2 + + +def test_execution_forwards_resolved_values(tmp_path: Path) -> None: + run = _generation_run(tmp_path) + with patch.object(judge, "run_judging", new_callable=AsyncMock) as run_judging: + result = vera.main( + [ + "judge", + "-j", + "gpt-4o:3", + "--conversations", + str(run), + "--target", + "SI", + "--max-concurrent", + "4", + "--per-judge", + ] + ) + + assert result == 0 + kwargs = run_judging.await_args.kwargs + assert kwargs["judge_models"] == {"gpt-4o": 3} + assert kwargs["max_concurrent"] == 4 + assert kwargs["per_judge"] is True + assert kwargs["output_root"] == str((run / "evaluations").resolve()) + assert kwargs["output_folder"] is None + assert kwargs["resume"] is False + assert kwargs["transcripts_dir"] == str(run / "conversations") + assert kwargs["rubric_file"] == str( + (cli_config.ROOT / "data/SI/rubric.tsv").resolve() + )