diff --git a/AGENTS.md b/AGENTS.md index aef480b3..c41f9b6a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -35,9 +35,9 @@ cp .env.example .env # Add API keys (ANTHROPIC_API_KEY, OPENAI_API_KEY, et | **CLI** | `vera.py`, `vera_cli/`, `utils/config_schema.py` | Thin entry point, command adapters, and shared command/config wiring | | **Generation** | `generate/` | Conversation simulation, turns, personas | | **Judging** | `judge/` | Rubric scoring, TSV output, question navigation | -| **Scoring** | `score/` | Score aggregation and visualizations from `results.csv` | +| **Scoring** | `score/` | Score aggregation, pooling, and visualizations from `results.csv` | | **LLM providers** | `llm_clients/`, `llm_clients/llm_factory.py` | New models, custom HTTP/API providers | -| **Pipeline helpers** | `scripts/` | Pooling and automation until absorbed into `vera pool` | +| **Pipeline helpers** | `scripts/` | Batch and reporting helpers | | **Data** | `data/` (personas, rubrics) | Evaluation inputs (committed) | | **Output** | `output/` (gitignored) | Generated transcripts, evaluations, logs | | **Config** | `utils/model_config_loader.py`, `llm_clients/config.py` | Model name resolution, API keys | diff --git a/CHANGELOG.md b/CHANGELOG.md index 73abd4ff..77f43b09 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,20 +4,26 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), ## [Unreleased] -### Documentation and development workflow - -- **Wider maintainer review**—[`.github/CODEOWNERS`](.github/CODEOWNERS) now also covers `score/`, `vera_cli/`, `vera.py`, `utils/`, `scripts/`, `.github/`, and `docs/roadmap.md`, and [`AGENTS.md`](AGENTS.md) asks agents not to change public signatures or unrelated files unless the task calls for it. ### Breaking / migration +- **Pool script moved to [`legacy/`](legacy/)**—`scripts/pool_vera_scores.py` is now `legacy/pool_vera_scores.py`; run it as `uv run python -m legacy.pool_vera_scores`. Its flags and defaults are unchanged. Prefer `vera pool`. - **`generate.main` is now `generate.run`**—The generation entry module follows the package naming convention. `from generate import run_generation` is unchanged; update direct imports and `patch()` targets that name `generate.main`. +### Runtime, CLI, and pipeline + +- **`vera pool`**—Merges two or more judge evaluations into one scored result (`vera pool --evaluations `), replacing the pool script. Pooling now lives in [`score/pool.py`](score/pool.py) (`from score import run_pooling`). Like `vera score`, it skips the risk breakdown unless `--personas` is given. + ### Testing and CI - **Pyright gate**—CI now fails on type errors in `generate/`, `score/`, `utils/`, `vera_cli/`, and `vera.py`, which have none today. The full-repo pyright run still reports without failing. +### Documentation and development workflow + +- **Wider maintainer review**—[`.github/CODEOWNERS`](.github/CODEOWNERS) now also covers `score/`, `vera_cli/`, `vera.py`, `utils/`, `scripts/`, `.github/`, and `docs/roadmap.md`, and [`AGENTS.md`](AGENTS.md) asks agents not to change public signatures or unrelated files unless the task calls for it. + ### Fixed -- **Recommended pipeline pooling**—[`legacy/run_recommended_vera_pipeline.sh`](legacy/run_recommended_vera_pipeline.sh) finds [`scripts/pool_vera_scores.py`](scripts/pool_vera_scores.py) again. It looked for the pool script next to itself, which broke its final pooling step when the shell script moved to `legacy/`. +- **Recommended pipeline pooling**—[`legacy/run_recommended_vera_pipeline.sh`](legacy/run_recommended_vera_pipeline.sh) runs its pooling step again: it looked for the pool script next to itself after moving to `legacy/`, where the script was not yet. It now runs `python -m legacy.pool_vera_scores`. ## [v2.0.3](https://github.com/SpringCare/VERA-MH/releases/tag/v2.0.3) \- 2026-10-09 diff --git a/README.md b/README.md index 9290f51a..b0b4bfb8 100644 --- a/README.md +++ b/README.md @@ -61,7 +61,8 @@ jq '.generation.chatbot.name = ""' configs/recommended-SI.json This produces one evaluation per user model. The headline score is the **pooled** result across both: ```bash -uv run python scripts/pool_vera_scores.py +uv run python vera.py pool --evaluations \ + --personas data/SI/personas.tsv ``` `vera pipeline` prints both evaluation folders when it finishes. How the score is computed and what the output files contain is covered in [docs/scoring.md](docs/scoring.md). diff --git a/docs/ARCHITECTURE-SPINE.md b/docs/ARCHITECTURE-SPINE.md index 3a14dc22..ea1a5b98 100644 --- a/docs/ARCHITECTURE-SPINE.md +++ b/docs/ARCHITECTURE-SPINE.md @@ -368,7 +368,7 @@ tests/ # permanent tests - **Operational/deployment envelope.** Future service or scheduled deployment is possible but not committed, and this architecture is explicitly not designed around it yet. Can wait because it would affect the `workers/` queue backend choice (e.g. pulling `SQSQueue` forward) and would need its own operations doc — premature before a concrete deployment requirement exists. - **Multi-chatbot (multi-provider) support within a single `vera pipeline` invocation.** Stays single-chatbot-per-invocation for now, with cross-chatbot comparison done by looping the CLI externally. Flagged as a possible future addition, not ruled out permanently. Can wait because the external-loop pattern already satisfies every known use case, including batch generate's cross-provider handling. - **Per-provider concurrency limits.** The shared `workers/` pool enables real parallel fan-out (e.g. `-u gpt:5 sonnet:5`) with no cap on per-provider concurrency yet — only per-call retry/backoff exists today. Flagged as a real requirement for the phase that lands `workers/` unification, not yet designed. Can wait until that phase, since no code path exercises pool-level fan-out at meaningful scale before then. -- **Fate of `scripts/pool_vera_scores.py`.** Whether to fold its logic into `score/pool.py` or delete it outright once `vera pool` fully supersedes it is unresolved. Can wait until the substantial-refactor phase, when `score/` is the only pooling entry point and the script's remaining usage (if any) becomes clear. +- **Fate of `scripts/pool_vera_scores.py`.** Resolved: its logic moved into `score/pool.py` behind `vera pool`, and the script remains only as the legacy wrapper `legacy/pool_vera_scores.py`, removed with the rest of `legacy/`. - **Fate of `judge/`'s legacy score-tooling siblings.** `score_comparison.py`, `score_comparison_from_export_summary.py`, and `score_utils.py` currently live in `judge/` alongside what moved to `score/`, and none appear in AD-4's target `score/` package tree. Whether each folds into `score/` during the Phase-2 extraction, gets deleted outright, or stays put is undecided beyond the already-deferred fate of `scripts/pool_vera_scores.py` above. Can wait because Phase-2 execution will need to decide file-by-file once the `score/` extraction is actually underway — this is not a decision this spine can make in the abstract. - **Possible future `report/` package.** `scripts/summarize_results.py`'s improvement report is deterministic aggregation over `judge/` output, so it stays under `judge/` for now. If reporting later evolves to call an LLM for creative improvement suggestions rather than just stats, that's a different concern from `judge/`'s pure-core-plus-handler charter and would warrant its own `report/` package. Can wait because no LLM-based reporting exists yet — revisit only once that feature is actually proposed. - **OpenSpec adoption (the OpenSpec phase in `docs/roadmap.md`, independent of the others).** `openspec/` exists today only as empty scaffolding; whether the team actually adopts the OpenSpec workflow for large multi-file features (the existing ESCALATE trigger: new judge dimensions, pipeline CLI changes) is not yet decided. Can wait because no other phase depends on it, and it can be pulled forward independently the moment a qualifying feature would benefit from it — done only once a real change has gone through `openspec/changes/`, not once the config exists. diff --git a/docs/cli.md b/docs/cli.md index ec9bed7a..ca476ef9 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -173,6 +173,27 @@ uv run python vera.py score \ The risk-level breakdown currently works only with SI personas. What each output file contains is described in [scoring.md](scoring.md). +## `vera pool` + +Merges two or more judge evaluations into one `results.csv` and scores it, typically the two user-model evaluations of the recommended profile. Each evaluation is a `j_*` folder or the `results.csv` inside it. + +Most users won't run this directly: it's for combining evaluations you judged separately, such as the two user-model runs of the recommended profile, into the headline score. + +```bash +uv run python vera.py pool --personas data/SI/personas.tsv \ + --evaluations output//evaluations/ \ + output//evaluations/ +``` + +| Flag | Description | Default | +|------|-------------|---------| +| `--evaluations` | Two or more evaluations to merge | required | +| `-o`, `--output` | Folder the pooled `j_*` folder is created in | `output/` | +| `--personas` | Personas file for the risk-level breakdown | none (breakdown skipped) | +| `--skip-risk-analysis` | Skip the risk-level breakdown even when `--personas` is given | off | + +The config section is `pooling` with the same four fields (`evaluations`, `output`, `personas`, `skip_risk_analysis`). The output files are described in [scoring.md](scoring.md#pooling-several-evaluations). + ## `vera pipeline` Runs generate, judge, and score in sequence. Its flag form accepts only the three model roles and the target: @@ -273,5 +294,4 @@ Folder names still use the legacy `p_` (user) and `a_` (chatbot) prefixes. A lay These are specified in [architecture.md](architecture.md) but not implemented: -- **`vera pool`**—combine several evaluations into one score. Until then, use [`scripts/pool_vera_scores.py`](scoring.md#pooling-several-evaluations). - **`vera resume`**—continue a run from its own `config.json` and `state.json`, across stages and without being given a path. Until then, use [`--into`](#continuing-an-interrupted-run---into). diff --git a/docs/legacy-scripts.md b/docs/legacy-scripts.md index 74f6c668..1d3ad02e 100644 --- a/docs/legacy-scripts.md +++ b/docs/legacy-scripts.md @@ -73,6 +73,14 @@ uv run python -m legacy.score -r output//evaluations/ +``` + +Was `scripts/pool_vera_scores.py`. Same behavior as `vera pool`, with legacy flags: positional evaluation paths instead of `--evaluations`, `-o`/`--output-dir`, and `--personas-tsv`, which defaults to `data/SI/personas.tsv` (`vera pool --personas` has no default and skips the risk breakdown when omitted). `--extract-from-log` prints the last evaluation folder found in a `legacy.run_pipeline` log; only `legacy/run_recommended_vera_pipeline.sh` uses it. + ## `legacy/run_pipeline.py` Runs the generation, judging, and scoring scripts in sequence: @@ -114,5 +122,5 @@ Its defaults, and the `VERA_*` environment variables that override them, are doc | `VERA_MAX_PERSONAS` | `--sample N` | | `VERA_USER_A`, `VERA_USER_B` | entries in `generation.user` | | `VERA_JUDGE`, `VERA_JUDGE_EXTRA_PARAMS` | `judging.models` | -| `VERA_POOL_OUTPUT` | `-o` on `scripts/pool_vera_scores.py` | -| `VERA_POOL_SKIP_RISK` | `--skip-risk-analysis` on `scripts/pool_vera_scores.py` | +| `VERA_POOL_OUTPUT` | `-o` on `vera pool` | +| `VERA_POOL_SKIP_RISK` | `--skip-risk-analysis` on `vera pool` | diff --git a/docs/roadmap.md b/docs/roadmap.md index 3b1a3112..8605e3f6 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -17,7 +17,7 @@ Smaller items that belong to no phase are in the [Backlog](#backlog). | Phase | Goal | Done when | |-------|------|-----------| -| [Scoring split](#scoring-split) | `score/` owns pooling and stops importing `judge/` | `vera pool` matches `scripts/pool_vera_scores.py` on the same inputs; `vera pipeline` ends with the pooled score; import-linter `judge/` ⊥ `score/` passes in CI | +| [Scoring split](#scoring-split) | `score/` owns pooling and stops importing `judge/` | `vera pipeline` ends with the pooled score; import-linter `judge/` ⊥ `score/` passes in CI | | [Rubric-agnostic scoring](#rubric-agnostic-scoring) | Scoring works for any rubric, with unambiguous stop tokens | A non-SI target scores and groups by risk with no SI assumptions; every terminating rubric row uses an explicit stop token | | [Traceability](#traceability) | Every run records what produced it, under the target-rooted layout | New runs write `config.json`, `state.json`, and checksums under `output//`; `vera judge --target all` works; `utils/` leaf contract passes | | [Model spec boundary](#model-spec-boundary) | Domains take `ModelSpec` directly | `run_for_user_models` and `_legacy_model_config` are gone; per-model judge parameters work | @@ -28,10 +28,9 @@ Smaller items that belong to no phase are in the [Backlog](#backlog). ### Scoring split -- **`vera pool`**, implemented in `score/pool.py`. `scripts/pool_vera_scores.py` becomes a thin legacy wrapper over it (moved to `legacy/`), so `legacy/run_recommended_vera_pipeline.sh` keeps working. Two user-visible strings already promise this command: `vera judge --conversations` help (`vera_cli/judge.py`) and a `JudgingConfig` error (`utils/config_schema.py`). -- **`vera pipeline` ends with the final score.** Today a multi-user-model pipeline leaves N per-user-model scores and no headline number, and the caller finishes the job by hand with the pool script. The pooled score is the answer the pipeline was run to get, so the pipeline should produce it. An earlier attempt was reverted for three reasons, each now addressed: - 1. The pooled artifact had nowhere correct to live. #215 documents its home at `/c_/pooled/`; until [Traceability](#traceability) builds that layout, pooled output stays where the pool script writes it today. - 2. It inverted the layering (`vera_cli/` imported from `scripts/`). Fixed by `score/pool.py` above. +- **`vera pipeline` ends with the final score.** Today a multi-user-model pipeline leaves N per-user-model scores and no headline number, and the caller finishes the job by hand with `vera pool`. The pooled score is the answer the pipeline was run to get, so the pipeline should produce it. An earlier attempt was reverted for three reasons, each now addressed: + 1. The pooled artifact had nowhere correct to live. #215 documents its home at `/c_/pooled/`; until [Traceability](#traceability) builds that layout, pooled output stays where `vera pool` writes it today. + 2. It inverted the layering (`vera_cli/` imported from `scripts/`). Fixed: pooling now lives in `score/pool.py`. 3. It seemed to contradict "judge never auto-scores". That rule is about `vera judge` staying non-scoring, which still holds; `pipeline` exists to run every stage. A standalone `vera pool` stays useful for pooling evaluations judged separately, across targets, or re-pooled later. @@ -151,3 +150,4 @@ Smaller items that belong to no phase. Each can ship as its own PR whenever it's - **`vera` CLI and config.** `vera.py` (`generate`, `judge`, `score`, `pipeline`) with `--config`, `--target`, `--personas`, `--rubric`, `-u`, `-j`, `--sample`, and `--into` resume. The old root scripts moved to `legacy/` (#243) instead of being deleted; see [Deferred](#deferred). - **Multi-rubric evaluation, through targets.** This is the intended end state, not a stopgap. Evaluating the same conversations with several evaluators (rubrics) means one target per rubric: each target bundles its rubric with its personas and prompts, and each is judged separately with `vera judge --target `, writing its own results. Keeping rubrics separate is deliberate, because scores from different rubrics aren't comparable and must never be merged. The earlier plan for several rubrics inside one run (`judging.rubrics` with length > 1, per-rubric judge-model overrides) is dropped. `judging.rubrics` keeps its list shape with a length-1 check, so revisiting it later wouldn't break the schema. What remains is convenience, not capability: `vera judge --target all` comes with [Traceability](#traceability), once the output path attributes each target. - **Package moves.** `generate_conversations/` → `generate/` (#245); scoring out of `judge/` into `score/` with `run.py`, `aggregate.py`, `viz.py` (#246). +- **`vera pool`.** Pooling moved from `scripts/pool_vera_scores.py` into `score/pool.py`, behind a new `vera pool` command; the script is now the legacy wrapper `legacy/pool_vera_scores.py`. diff --git a/docs/scoring.md b/docs/scoring.md index d2490473..84ee6426 100644 --- a/docs/scoring.md +++ b/docs/scoring.md @@ -39,17 +39,15 @@ The two risk files are written only when a personas file is given (`--personas`, ## Pooling several evaluations -The published VERA-MH score pools two evaluations of the same chatbot, one per user model. Merge them with: +The published VERA-MH score pools two evaluations of the same chatbot, one per user model. Merge them with [`vera pool`](cli.md#vera-pool): ```bash -uv run python scripts/pool_vera_scores.py -o \ - output//evaluations/ \ - output//evaluations/ +uv run python vera.py pool -o --personas data/SI/personas.tsv \ + --evaluations output//evaluations/ \ + output//evaluations/ ``` -This creates one pooled folder (named like `j_gpt-5.4x1__p_gpt_5_2+claude_opus_4_5__a_.../`) with a merged `results.csv`, `pool_metadata.json` recording the sources, and the usual `scores/`. Pass `--skip-risk-analysis` to skip the risk breakdown. Use that pooled folder for headline numbers. See `--help` for other options. - -A `vera pool` command will replace this script. +This creates one pooled folder (named like `j_gpt-5.4x1__p_gpt_5_2+claude_opus_4_5__a_.../`) with a merged `results.csv`, `pool_metadata.json` recording the sources, and the usual `scores/`. As with `vera score`, the risk files are written only when a personas file is given. Use that pooled folder for headline numbers. ## Comparing chatbots diff --git a/legacy/pool_vera_scores.py b/legacy/pool_vera_scores.py new file mode 100755 index 00000000..37c1169f --- /dev/null +++ b/legacy/pool_vera_scores.py @@ -0,0 +1,166 @@ +"""Legacy CLI for pooling: ``uv run python -m legacy.pool_vera_scores ``. + +Deprecated; use ``vera pool --evaluations ``. Both call +`score.run_pooling`. This wrapper keeps the legacy flags (positional paths, +``--output-dir``, ``--personas-tsv``) and the legacy ``data/SI/personas.tsv`` +default, plus ``--extract-from-log``, which only +``legacy/run_recommended_vera_pipeline.sh`` uses to find each suite's +evaluation folder in ``legacy.run_pipeline`` output. +""" + +import argparse +import re +import sys +from pathlib import Path + +from score import run_pooling + +REPO_ROOT = Path(__file__).resolve().parent.parent + + +def extract_last_evaluation_dir_from_pipeline_log(text: str) -> str: + """ + Parse captured ``run_pipeline.py`` log text and return the last evaluation folder. + + The pipeline prints a line containing ``Evaluations saved to:`` followed by the + ``j_*`` evaluation directory (with or without a trailing slash). When the log + contains multiple runs, the **last** match is returned so callers can chain + subprocess output from successive pipeline invocations. + + Args: + text: Full stdout/stderr text (e.g. contents of a temp file used with ``tee``). + + Returns: + Absolute path to the evaluation directory as a string. + + Raises: + ValueError: If no matching line exists in *text*. + """ + matches = re.findall(r"Evaluations saved to:\s*(.+)$", text, flags=re.MULTILINE) + if not matches: + raise ValueError( + "Could not find any 'Evaluations saved to:' line in pipeline log output." + ) + path = matches[-1].strip().rstrip("/") + return str(Path(path).resolve()) + + +def _cli_pool(args: argparse.Namespace) -> int: + """ + Handle the ``pool_vera_scores`` CLI in merge mode (one or more evaluation paths). + + Resolves ``--output-dir`` (defaulting to the repo ``output/`` directory), then + delegates to `score.run_pooling`. + + Args: + args: Parsed namespace with ``eval_paths``, ``output_dir``, ``personas_tsv``, + and ``skip_risk_analysis``. + + Returns: + Process exit code: ``0`` on success, ``2`` if no evaluation paths were given. + """ + if len(args.eval_paths) < 1: + print( + "error: pass at least one evaluation directory or results.csv", + file=sys.stderr, + ) + return 2 + + out_parent = Path(args.output_dir).resolve() if args.output_dir else None + if out_parent is None: + out_parent = (REPO_ROOT / "output").resolve() + out_parent.mkdir(parents=True, exist_ok=True) + + personas = Path(args.personas_tsv).resolve() if args.personas_tsv else None + run_pooling( + evaluations=args.eval_paths, + output_parent=out_parent, + personas_tsv=personas, + skip_risk_analysis=args.skip_risk_analysis, + ) + return 0 + + +def _cli_extract(args: argparse.Namespace) -> int: + """ + Handle ``--extract-from-log``: print the last evaluation directory path. + + Reads the log file as UTF-8 (replacing undecodable bytes), parses it with + :func:`extract_last_evaluation_dir_from_pipeline_log`, and prints the resolved + path to stdout for use in shell command substitution. + + Args: + args: Parsed namespace with ``extract_from_log`` set to the log file path. + + Returns: + ``0`` on success, ``1`` if no evaluation line was found in the log. + """ + log_path = Path(args.extract_from_log) + text = log_path.read_text(encoding="utf-8", errors="replace") + try: + path = extract_last_evaluation_dir_from_pipeline_log(text) + except ValueError as e: + print(f"error: {e}", file=sys.stderr) + return 1 + print(path) + return 0 + + +def main() -> int: + """ + CLI entry point: parse arguments and run pool or extract mode. + + In extract mode (``--extract-from-log``), runs :func:`_cli_extract` and exits. + Otherwise requires at least one evaluation path and runs :func:`_cli_pool`. + + Returns: + Process exit code from the selected subcommand. + """ + parser = argparse.ArgumentParser( + description=( + "Merge multiple j_* evaluation runs (any mix of user agents and/or " + "judge models) into one scored folder, or extract an evaluation path " + "from run_pipeline log output." + ) + ) + parser.add_argument( + "eval_paths", + nargs="*", + help="Evaluation directories (j_*) or results.csv paths to merge", + ) + parser.add_argument( + "-o", + "--output-dir", + help=( + "Parent directory for pooled output (a synthetic j___p_* folder is " + "created inside, with pool_metadata.json next to results.csv). " + "Default: output/ under the repo." + ), + ) + parser.add_argument( + "--personas-tsv", + default=str(REPO_ROOT / "data" / "SI" / "personas.tsv"), + help="Personas file for risk-level scoring (default: data/SI/personas.tsv)", + ) + parser.add_argument( + "--skip-risk-analysis", + action="store_true", + help="Skip risk-level scores and visualization", + ) + parser.add_argument( + "--extract-from-log", + metavar="FILE", + help="Print the last evaluation directory path found in a pipeline log file", + ) + args = parser.parse_args() + + if args.extract_from_log: + return _cli_extract(args) + + if not args.eval_paths: + parser.error("pass at least one eval path, or use --extract-from-log") + return _cli_pool(args) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/legacy/run_recommended_vera_pipeline.sh b/legacy/run_recommended_vera_pipeline.sh index f10cd193..ce564ba6 100755 --- a/legacy/run_recommended_vera_pipeline.sh +++ b/legacy/run_recommended_vera_pipeline.sh @@ -8,11 +8,11 @@ # the provider agent you pass on the command line. # 2) Same pipeline with user model B (default: Claude Opus 4.5). # 3) Merge the two evaluation folders into one pooled bundle (results.csv -# and scores/) using scripts/pool_vera_scores.py. +# and scores/) using legacy/pool_vera_scores.py. # # Implementation detail: each phase runs run_pipeline.py; the evaluation # directory path printed by the pipeline is captured from the log via -# scripts/pool_vera_scores.py --extract-from-log so the final pooling step knows which +# legacy/pool_vera_scores.py --extract-from-log so the final pooling step knows which # folders to merge. # # Second source of truth, on purpose and temporarily: the same published @@ -89,7 +89,7 @@ fi # Throttling and persona cap (defaults here; override with VERA_* env vars). COMMON_ARGS+=(--max-concurrent "${VERA_MAX_CONCURRENT:-10}" --max-personas "${VERA_MAX_PERSONAS:-100}") -# Arguments for the final scripts/pool_vera_scores.py invocation only. +# Arguments for the final legacy/pool_vera_scores.py invocation only. POOL_ARGS=(-o "$POOL_PARENT") if [[ -n "${VERA_POOL_SKIP_RISK:-}" ]]; then POOL_ARGS+=(--skip-risk-analysis) @@ -110,7 +110,7 @@ run_pipeline_capture_eval() { return "$st" fi local ev - ev="$(uv run python scripts/pool_vera_scores.py --extract-from-log "$log")" || { + ev="$(uv run python -m legacy.pool_vera_scores --extract-from-log "$log")" || { rm -f "$log" return 1 } @@ -128,4 +128,4 @@ EVAL_B="$(run_pipeline_capture_eval --user-agent "$USER_B" "${COMMON_ARGS[@]}" " echo "" echo "== Pooling evaluation scores into $POOL_PARENT ==" # Merge the two evaluation roots into one j___p_* folder under POOL_PARENT. -uv run python scripts/pool_vera_scores.py "${POOL_ARGS[@]}" "$EVAL_A" "$EVAL_B" +uv run python -m legacy.pool_vera_scores "${POOL_ARGS[@]}" "$EVAL_A" "$EVAL_B" diff --git a/score/__init__.py b/score/__init__.py index fa91247c..c997e6a2 100644 --- a/score/__init__.py +++ b/score/__init__.py @@ -1,5 +1,6 @@ -"""Scoring package - aggregation and visualization of judge results""" +"""Scoring package - aggregation, pooling, and visualization of judge results""" +from .pool import run_pooling from .run import run_scoring -__all__ = ["run_scoring"] +__all__ = ["run_pooling", "run_scoring"] diff --git a/scripts/pool_vera_scores.py b/score/pool.py old mode 100755 new mode 100644 similarity index 58% rename from scripts/pool_vera_scores.py rename to score/pool.py index 4f1722c7..56bbeaa2 --- a/scripts/pool_vera_scores.py +++ b/score/pool.py @@ -1,39 +1,20 @@ -#!/usr/bin/env uv run python -""" -Pool judge evaluation results from multiple ``j_*`` evaluation directories into -one ``results.csv``, then recompute VERA-MH scores and visualizations. - -Inputs may differ by user agent, judge model, or both—the merged dataframe is a -straight concatenation of every source's rows. Common uses include combining the -two user-agent suites of the recommended profile (each judged with ``gpt-5.4`` -by default), or merging separate judge runs (e.g. GPT-4o and Sonnet) over the -same conversations. ``legacy/run_recommended_vera_pipeline.sh`` calls this -itself; ``vera pipeline`` does not — it prints the evaluation folders to pass -here, because pooling is a separate command by design (``docs/pipeline.md``). - -Typical layout for each input path: - output/p___a___t30__r1__/evaluations/j_<...>/results.csv - -You may pass either the evaluation directory (``j_*`` folder) or ``results.csv`` -inside it. For example:: - - uv run python scripts/pool_vera_scores.py \\ - output/p_claude__a_gpt_4o__t6__r1_20260422_100000/evaluations/j_gpt_4o__... \\ - output/p_gpt_4o__a_gpt_4o__t6__r1_20260422_110000/evaluations/j_gpt_4o__.../results.csv +"""Pool several judge evaluations into one scored result. -**Legacy layout** (e.g. top-level ``evaluations/j_*`` next to a flat ``conversations/`` -run): merging and scoring still work; only the auto-generated merged ``j_*`` folder -name may fall back to ``unknown`` placeholders because the script cannot infer the -``p_*`` generation basename from the path alone. +Merges the ``results.csv`` of several ``j_*`` evaluation folders (any mix of user +models and judge models), writes the merged CSV plus ``pool_metadata.json`` into +a new ``j_*``-style folder, and scores it like `score.run.run_scoring`. Typical +use is combining the two user-model suites of the recommended profile. -Merged folder names follow ``judge.py`` conventions: ``j_x`` for a -single judge, or ``j_x2+x1__p_...`` when multiple judges are present. +`run_pooling` is the scoring domain's application function for ``vera pool``; +``legacy/pool_vera_scores.py`` wraps the same function for the legacy CLI. -Also supports extracting the last evaluation directory from a run_pipeline log: - uv run python scripts/pool_vera_scores.py --extract-from-log /path/to/log.txt +Until the Traceability phase (docs/roadmap.md) builds the target-rooted layout, +the pooled folder is named in the legacy ``j___p_+__a___...`` +style and written under the caller's output folder. """ -import argparse +from __future__ import annotations + import json import re import sys @@ -41,12 +22,17 @@ from pathlib import Path from typing import Any -# in case running from the scripts directory -REPO_ROOT = Path(__file__).resolve().parent.parent -if str(REPO_ROOT) not in sys.path: - sys.path.insert(0, str(REPO_ROOT)) +import pandas as pd -from utils.naming import parse_generation_run_folder_name # noqa: E402 +from judge.score_utils import ensure_results_csv +from score.aggregate import ( + print_scores, + save_results_json, + score_results, + score_results_by_risk, +) +from score.viz import create_risk_level_visualizations, create_visualizations +from utils.naming import parse_generation_run_folder_name # ``j_{judge_info}_{timestamp}__p_...`` or ``...__conversations`` (legacy flat layout) _JUDGE_EVAL_DIR_HEAD = re.compile( @@ -56,33 +42,6 @@ _JUDGE_SPEC_TOKEN = re.compile(r"([^_]+x\d+)") -def extract_last_evaluation_dir_from_pipeline_log(text: str) -> str: - """ - Parse captured ``run_pipeline.py`` log text and return the last evaluation folder. - - The pipeline prints a line containing ``Evaluations saved to:`` followed by the - ``j_*`` evaluation directory (with or without a trailing slash). When the log - contains multiple runs, the **last** match is returned so callers can chain - subprocess output from successive pipeline invocations. - - Args: - text: Full stdout/stderr text (e.g. contents of a temp file used with ``tee``). - - Returns: - Absolute path to the evaluation directory as a string. - - Raises: - ValueError: If no matching line exists in *text*. - """ - matches = re.findall(r"Evaluations saved to:\s*(.+)$", text, flags=re.MULTILINE) - if not matches: - raise ValueError( - "Could not find any 'Evaluations saved to:' line in pipeline log output." - ) - path = matches[-1].strip().rstrip("/") - return str(Path(path).resolve()) - - def _resolve_eval_input(path: Path) -> Path: """ Normalize a CLI path to the judge evaluation directory (``j_*`` folder). @@ -314,12 +273,12 @@ def _annotate_pooled_results( } -def pool_evaluation_directories( - source_paths: list[str | Path], - output_parent: Path, +def run_pooling( *, - personas_tsv: Path | None = None, - skip_risk_analysis: bool = False, + evaluations: list[str | Path], + output_parent: str | Path, + personas_tsv: str | Path | None, + skip_risk_analysis: bool, judge_slug: str | None = None, ) -> Path: """ @@ -330,11 +289,11 @@ def pool_evaluation_directories( scoring artifacts under a new synthetic ``j___p_...`` directory. Args: - source_paths: Paths to ``j_*`` folders or to ``results.csv`` inside them. + evaluations: Paths to ``j_*`` folders or to ``results.csv`` inside them. output_parent: Directory under which the new merged ``j_*`` folder is - created (e.g. repo ``output/``). - personas_tsv: Personas file for risk-level analysis; defaults to - ``data/SI/personas.tsv`` under the repo when None. + created; created if missing. + personas_tsv: Personas file for risk-level analysis, or None to skip that + step, as in `score.run.run_scoring`. skip_risk_analysis: When True, skip ``score_results_by_risk`` and risk charts. judge_slug: Optional override for the judge name (e.g. ``gpt-4ox1+sonnet45x1``). @@ -346,21 +305,7 @@ def pool_evaluation_directories( resolving ``results.csv`` to its parent. ValueError: If the merged dataframe has no rows. """ - import pandas as pd - - from judge.score_utils import ensure_results_csv - from score.aggregate import ( - print_scores, - save_results_json, - score_results, - score_results_by_risk, - ) - from score.viz import ( - create_risk_level_visualizations, - create_visualizations, - ) - - eval_dirs = [_resolve_eval_input(Path(p)) for p in source_paths] + eval_dirs = [_resolve_eval_input(Path(p)) for p in evaluations] for d in eval_dirs: if not d.is_dir(): raise FileNotFoundError(d) @@ -386,7 +331,7 @@ def pool_evaluation_directories( f"({preview}). " "Merged results and scores are unchanged; the new merged j_* folder name " "may use unknown placeholders for persona/agent/turns/runs. " - "Use nested paths from generate.py / run_pipeline.py for descriptive " + "Use nested paths from vera generate / vera judge for descriptive " "names.", file=sys.stderr, ) @@ -394,7 +339,7 @@ def pool_evaluation_directories( synth_name = _synthetic_pooled_folder_basename( eval_dirs, combined, judge_slug=judge_slug ) - out_eval = (output_parent / synth_name).resolve() + out_eval = (Path(output_parent) / synth_name).resolve() out_eval.mkdir(parents=True, exist_ok=True) results_csv = out_eval / "results.csv" combined.to_csv(results_csv, index=False) @@ -422,25 +367,30 @@ def pool_evaluation_directories( except Exception as e: print(f"Warning: could not create standard visualizations: {e}") - personas_tsv = personas_tsv or (REPO_ROOT / "data" / "SI" / "personas.tsv") - if not skip_risk_analysis and personas_tsv.is_file(): - try: - risk_results = score_results_by_risk( - str(results_csv), str(personas_tsv), write_json=False + if not skip_risk_analysis: + if personas_tsv is None: + print("No personas file given; skipping risk-level analysis.") + elif not Path(personas_tsv).is_file(): + print( + f"Warning: personas TSV not found ({personas_tsv}), " + "skipping risk analysis." ) - risk_results["judge_model"] = "pooled" - risk_results["persona_model"] = "pooled" - risk_json = out_eval / "scores" / "scores_by_risk.json" - risk_json.parent.mkdir(parents=True, exist_ok=True) - risk_json.write_text(json.dumps(risk_results, indent=2), encoding="utf-8") - risk_viz = out_eval / "scores" / "scores_by_risk_visualization.png" - create_risk_level_visualizations(risk_results, risk_viz) - except Exception as e: - print(f"Warning: could not create risk-level analysis: {e}") - elif not skip_risk_analysis: - print( - f"Warning: personas TSV not found ({personas_tsv}), skipping risk analysis." - ) + else: + try: + risk_results = score_results_by_risk( + str(results_csv), str(personas_tsv), write_json=False + ) + risk_results["judge_model"] = "pooled" + risk_results["persona_model"] = "pooled" + risk_json = out_eval / "scores" / "scores_by_risk.json" + risk_json.parent.mkdir(parents=True, exist_ok=True) + risk_json.write_text( + json.dumps(risk_results, indent=2), encoding="utf-8" + ) + risk_viz = out_eval / "scores" / "scores_by_risk_visualization.png" + create_risk_level_visualizations(risk_results, risk_viz) + except Exception as e: + print(f"Warning: could not create risk-level analysis: {e}") print("") print("Pooled outputs:") @@ -453,124 +403,3 @@ def pool_evaluation_directories( print(f" {out_eval / 'scores' / 'scores_by_risk_visualization.png'}") print(f" {out_eval / 'pool_metadata.json'}") return out_eval - - -def _cli_pool(args: argparse.Namespace) -> int: - """ - Handle the ``pool_vera_scores`` CLI in merge mode (one or more evaluation paths). - - Resolves ``--output-dir`` (defaulting to the repo ``output/`` directory), then - delegates to :func:`pool_evaluation_directories`. - - Args: - args: Parsed namespace with ``eval_paths``, ``output_dir``, ``personas_tsv``, - and ``skip_risk_analysis``. - - Returns: - Process exit code: ``0`` on success, ``2`` if no evaluation paths were given. - """ - if len(args.eval_paths) < 1: - print( - "error: pass at least one evaluation directory or results.csv", - file=sys.stderr, - ) - return 2 - - out_parent = Path(args.output_dir).resolve() if args.output_dir else None - if out_parent is None: - out_parent = (REPO_ROOT / "output").resolve() - out_parent.mkdir(parents=True, exist_ok=True) - - personas = Path(args.personas_tsv).resolve() if args.personas_tsv else None - pool_evaluation_directories( - args.eval_paths, - out_parent, - personas_tsv=personas, - skip_risk_analysis=args.skip_risk_analysis, - ) - return 0 - - -def _cli_extract(args: argparse.Namespace) -> int: - """ - Handle ``--extract-from-log``: print the last evaluation directory path. - - Reads the log file as UTF-8 (replacing undecodable bytes), parses it with - :func:`extract_last_evaluation_dir_from_pipeline_log`, and prints the resolved - path to stdout for use in shell command substitution. - - Args: - args: Parsed namespace with ``extract_from_log`` set to the log file path. - - Returns: - ``0`` on success, ``1`` if no evaluation line was found in the log. - """ - log_path = Path(args.extract_from_log) - text = log_path.read_text(encoding="utf-8", errors="replace") - try: - path = extract_last_evaluation_dir_from_pipeline_log(text) - except ValueError as e: - print(f"error: {e}", file=sys.stderr) - return 1 - print(path) - return 0 - - -def main() -> int: - """ - CLI entry point: parse arguments and run pool or extract mode. - - In extract mode (``--extract-from-log``), runs :func:`_cli_extract` and exits. - Otherwise requires at least one evaluation path and runs :func:`_cli_pool`. - - Returns: - Process exit code from the selected subcommand. - """ - parser = argparse.ArgumentParser( - description=( - "Merge multiple j_* evaluation runs (any mix of user agents and/or " - "judge models) into one scored folder, or extract an evaluation path " - "from run_pipeline log output." - ) - ) - parser.add_argument( - "eval_paths", - nargs="*", - help="Evaluation directories (j_*) or results.csv paths to merge", - ) - parser.add_argument( - "-o", - "--output-dir", - help=( - "Parent directory for pooled output (a synthetic j___p_* folder is " - "created inside, with pool_metadata.json next to results.csv). " - "Default: output/ under the repo." - ), - ) - parser.add_argument( - "--personas-tsv", - default=str(REPO_ROOT / "data" / "SI" / "personas.tsv"), - help="Personas file for risk-level scoring (default: data/SI/personas.tsv)", - ) - parser.add_argument( - "--skip-risk-analysis", - action="store_true", - help="Skip risk-level scores and visualization", - ) - parser.add_argument( - "--extract-from-log", - metavar="FILE", - help="Print the last evaluation directory path found in a pipeline log file", - ) - args = parser.parse_args() - - if args.extract_from_log: - return _cli_extract(args) - - if not args.eval_paths: - parser.error("pass at least one eval path, or use --extract-from-log") - return _cli_pool(args) - - -if __name__ == "__main__": - raise SystemExit(main()) diff --git a/tests/integration/test_pool_parity.py b/tests/integration/test_pool_parity.py new file mode 100644 index 00000000..03a9f762 --- /dev/null +++ b/tests/integration/test_pool_parity.py @@ -0,0 +1,125 @@ +"""Parity between the legacy pool script and ``vera pool``. + +TEMPORARY — DELETE WITH `legacy/pool_vera_scores.py`. + +Both entry points call `score.run_pooling`; the claim worth proving while the +legacy script still has users is that, given the same inputs and the same +personas file, they write the same pooled results and scores. They diverge only +when no personas file is named (the legacy script defaults to the SI file, +`vera pool` skips the risk breakdown), which tests/unit/test_vera_pool.py covers. +Its end is the deferred removal of `legacy/` (`docs/roadmap.md`). +""" + +from __future__ import annotations + +import json +import shutil +import sys +from pathlib import Path +from unittest.mock import patch + +import pandas as pd +import pytest + +import vera +from legacy import pool_vera_scores as legacy_pool + +REPO_ROOT = Path(__file__).resolve().parents[2] +EVAL_FIXTURE = REPO_ROOT / "tests/fixtures/eval_tsv_rebuild__20260415_140037" +PERSONAS_FIXTURE = REPO_ROOT / "tests/fixtures/personas_with_risk.tsv" + + +def _sources(root: Path) -> list[Path]: + """Two independent copies of the fixture, as the two evaluations to pool.""" + runs = [] + for name in ("j_run_a", "j_run_b"): + run = root / name + shutil.copytree(EVAL_FIXTURE, run) + runs.append(run) + return runs + + +def _pooled_folder(output: Path) -> Path: + (folder,) = [path for path in output.iterdir() if path.is_dir()] + return folder + + +def _without_paths(scores: dict) -> dict: + """Drop the provenance block, which names each side's own source folders.""" + return {key: value for key, value in scores.items() if key != "pooled"} + + +@pytest.mark.parametrize( + "extra_legacy,extra_unified", + [ + (["--skip-risk-analysis"], ["--skip-risk-analysis"]), + ( + ["--personas-tsv", str(PERSONAS_FIXTURE)], + ["--personas", str(PERSONAS_FIXTURE)], + ), + ], + ids=["skip-risk", "with-personas"], +) +def test_both_entry_points_produce_the_same_pool( + tmp_path: Path, extra_legacy: list[str], extra_unified: list[str] +) -> None: + legacy_out = tmp_path / "legacy_out" + unified_out = tmp_path / "unified_out" + legacy_sources = _sources(tmp_path / "legacy_src") + unified_sources = _sources(tmp_path / "unified_src") + + legacy_argv = [ + "pool_vera_scores.py", + *map(str, legacy_sources), + "-o", + str(legacy_out), + *extra_legacy, + ] + with patch.object(sys, "argv", legacy_argv): + assert legacy_pool.main() == 0 + + parser = vera.build_parser() + args = parser.parse_args( + [ + "pool", + "--evaluations", + *map(str, unified_sources), + "-o", + str(unified_out), + *extra_unified, + ] + ) + assert args.handler(args) == 0 + + legacy_folder = _pooled_folder(legacy_out) + unified_folder = _pooled_folder(unified_out) + + pd.testing.assert_frame_equal( + pd.read_csv(unified_folder / "results.csv"), + pd.read_csv(legacy_folder / "results.csv"), + ) + for name in ("scores.json", "scores_by_risk.json"): + legacy_file = legacy_folder / "scores" / name + unified_file = unified_folder / "scores" / name + assert legacy_file.is_file() == unified_file.is_file() + if legacy_file.is_file(): + assert _without_paths(json.loads(unified_file.read_text())) == ( + _without_paths(json.loads(legacy_file.read_text())) + ) + legacy_meta = json.loads((legacy_folder / "pool_metadata.json").read_text()) + unified_meta = json.loads((unified_folder / "pool_metadata.json").read_text()) + assert unified_meta["rows_per_source"] == legacy_meta["rows_per_source"] + + +def test_legacy_extract_from_log_still_finds_the_evaluation_folder( + tmp_path: Path, capsys: pytest.CaptureFixture +) -> None: + log = tmp_path / "pipeline.log" + log.write_text( + "noise\nEvaluations saved to: /a/first\nmore\nEvaluations saved to: /b/last/\n" + ) + with patch.object( + sys, "argv", ["pool_vera_scores.py", "--extract-from-log", str(log)] + ): + assert legacy_pool.main() == 0 + assert capsys.readouterr().out.strip() == str(Path("/b/last").resolve()) diff --git a/tests/unit/score/test_pool.py b/tests/unit/score/test_pool.py new file mode 100644 index 00000000..b9867485 --- /dev/null +++ b/tests/unit/score/test_pool.py @@ -0,0 +1,106 @@ +"""Unit tests for the naming and input helpers in score/pool.py.""" + +from __future__ import annotations + +from pathlib import Path + +import pandas as pd +import pytest + +from score import pool +from utils.naming import build_generation_run_folder_name + +# Generation-run folder names, built with the real naming helper so the tests +# follow its format. The folders themselves are created under `tmp_path` by +# `_nested_eval`; nothing has to exist beforehand. +GEN_RUN = build_generation_run_folder_name( + "claude-opus-4-5", "gpt-5.4-nano", 30, 1, "20260422_150000" +) +GEN_RUN_B = build_generation_run_folder_name( + "gpt-5.2", "gpt-5.4-nano", 30, 1, "20260422_160000" +) + + +def _nested_eval(tmp_path: Path, gen_run: str, eval_name: str) -> Path: + """An evaluation folder in the standard ``p_*/evaluations/j_*`` layout.""" + eval_dir = tmp_path / gen_run / "evaluations" / eval_name + eval_dir.mkdir(parents=True) + return eval_dir + + +def test_resolve_eval_input_accepts_a_folder_or_its_results_csv( + tmp_path: Path, +) -> None: + (tmp_path / "results.csv").write_text("x\n") + assert pool._resolve_eval_input(tmp_path) == tmp_path.resolve() + assert pool._resolve_eval_input(tmp_path / "results.csv") == tmp_path.resolve() + + +def test_resolve_eval_input_rejects_other_files_and_missing_paths( + tmp_path: Path, +) -> None: + (tmp_path / "other.csv").write_text("x\n") + with pytest.raises(ValueError, match="results.csv"): + pool._resolve_eval_input(tmp_path / "other.csv") + with pytest.raises(FileNotFoundError): + pool._resolve_eval_input(tmp_path / "absent") + + +def test_generation_folder_found_only_in_the_nested_layout(tmp_path: Path) -> None: + nested = _nested_eval(tmp_path, GEN_RUN, "j_x") + flat = tmp_path / "evaluations" / "j_y" + flat.mkdir(parents=True) + + assert pool._generation_folder_for_eval(nested) == (tmp_path / GEN_RUN).resolve() + assert pool._generation_folder_for_eval(flat) is None + + +def test_judge_slug_from_dataframe_counts_instances_per_model() -> None: + df = pd.DataFrame( + { + "judge_model": ["gpt-4o", "gpt-4o", "claude-sonnet-4-5"], + "judge_instance": [1, 2, 1], + } + ) + assert pool._judge_slug_from_dataframe(df) == "claude-sonnet-4-5x1+gpt-4ox2" + + +def test_judge_slug_from_dataframe_without_judge_column() -> None: + assert pool._judge_slug_from_dataframe(pd.DataFrame({"a": [1]})) == "unknown" + + +def test_merge_judge_slug_tokens_keeps_the_highest_count_per_model() -> None: + assert ( + pool._merge_judge_slug_tokens("gpt-4ox1", "gpt-4ox3+sonnetx1") + == "gpt-4ox3+sonnetx1" + ) + assert pool._merge_judge_slug_tokens("not-a-token") == "unknown" + + +def test_judge_slug_from_eval_dir_name() -> None: + name = "j_gpt-5.4x1_20260422_150000_123__p_claude" + assert pool._judge_slug_from_eval_dir_name(name) == "gpt-5.4x1" + assert pool._judge_slug_from_eval_dir_name("j_pooled__p_x") is None + assert pool._judge_slug_from_eval_dir_name("something_else") is None + + +def test_pooled_folder_name_uses_the_first_generation_run(tmp_path: Path) -> None: + eval_a = _nested_eval(tmp_path, GEN_RUN, "j_a") + eval_b = _nested_eval(tmp_path, GEN_RUN_B, "j_b") + combined = pd.DataFrame({"judge_model": ["gpt-5.4"], "judge_instance": [1]}) + + name = pool._synthetic_pooled_folder_basename([eval_a, eval_b], combined) + + assert name.startswith( + "j_gpt-5.4x1__p_claude_opus_4_5+gpt_5_2__a_gpt_5_4_nano__t30__r1__" + ) + + +def test_pooled_folder_name_falls_back_without_a_generation_run( + tmp_path: Path, +) -> None: + combined = pd.DataFrame({"judge_model": ["gpt-5.4"], "judge_instance": [1]}) + + name = pool._synthetic_pooled_folder_basename([tmp_path], combined) + + assert name.startswith("j_gpt-5.4x1__p_unknown__a_unknown__t30__r1__") diff --git a/tests/unit/test_vera_pool.py b/tests/unit/test_vera_pool.py new file mode 100644 index 00000000..e4de4399 --- /dev/null +++ b/tests/unit/test_vera_pool.py @@ -0,0 +1,275 @@ +"""Tests for the ``vera pool`` command.""" + +from __future__ import annotations + +import json +import shutil +from pathlib import Path + +import pandas as pd +import pytest + +import vera +from vera_cli import config as cli_config +from vera_cli import pool + +REPO_ROOT = Path(__file__).resolve().parents[2] +EVAL_FIXTURE = REPO_ROOT / "tests/fixtures/eval_tsv_rebuild__20260415_140037" +EVAL_FIXTURE_REL = "tests/fixtures/eval_tsv_rebuild__20260415_140037" +PERSONAS_FIXTURE = REPO_ROOT / "tests/fixtures/personas_with_risk.tsv" + + +@pytest.fixture(autouse=True) +def clear_env_config(monkeypatch: pytest.MonkeyPatch) -> None: + monkeypatch.delenv(cli_config.VERA_RUN_CONFIG_ENV, raising=False) + + +def _evaluations(tmp_path: Path) -> list[Path]: + """Two copies of the checked-in evaluation fixture, as two runs to pool.""" + runs = [] + for name in ("j_run_a", "j_run_b"): + run = tmp_path / name + shutil.copytree(EVAL_FIXTURE, run) + runs.append(run) + return runs + + +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 _pooling_config(evaluations: list[str], **overrides: object) -> dict: + pooling: dict[str, object] = { + "evaluations": evaluations, + "output": "output", + "personas": None, + "skip_risk_analysis": False, + } + pooling.update(overrides) + return {"pooling": pooling} + + +def _pooled_folder(output: Path) -> Path: + (folder,) = [path for path in output.iterdir() if path.is_dir()] + return folder + + +def test_pool_is_registered_and_has_help() -> None: + parser = vera.build_parser() + with pytest.raises(SystemExit) as exit_info: + parser.parse_args(["pool", "--help"]) + assert exit_info.value.code == 0 + + +def test_pool_requires_evaluations() -> None: + parser = vera.build_parser() + with pytest.raises(cli_config.ConfigError, match="--evaluations"): + pool.resolve_configs(parser.parse_args(["pool"])) + + +def test_pool_needs_at_least_two_evaluations(tmp_path: Path) -> None: + run, _ = _evaluations(tmp_path) + parser = vera.build_parser() + with pytest.raises(cli_config.ConfigError, match="at least two"): + pool.resolve_configs(parser.parse_args(["pool", "--evaluations", str(run)])) + + +def test_missing_evaluation_fails_before_any_work(tmp_path: Path) -> None: + run, _ = _evaluations(tmp_path) + parser = vera.build_parser() + args = parser.parse_args( + ["pool", "--evaluations", str(run), str(tmp_path / "absent")] + ) + with pytest.raises(cli_config.ConfigError, match="evaluation folder"): + pool.resolve_configs(args) + + +def test_evaluation_may_be_a_folder_or_its_results_csv(tmp_path: Path) -> None: + run_a, run_b = _evaluations(tmp_path) + parser = vera.build_parser() + + (resolved,) = pool.resolve_configs( + parser.parse_args( + ["pool", "--evaluations", str(run_a), str(run_b / "results.csv")] + ) + ) + + assert resolved.pooling is not None + assert resolved.pooling.evaluations == [ + str(run_a.resolve()), + str((run_b / "results.csv").resolve()), + ] + + +def test_cli_defaults(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None: + """No personas (unlike the legacy script) and output/ in the working directory.""" + run_a, run_b = _evaluations(tmp_path) + monkeypatch.chdir(tmp_path) + parser = vera.build_parser() + + (resolved,) = pool.resolve_configs( + parser.parse_args(["pool", "--evaluations", "j_run_a", "j_run_b"]) + ) + + assert resolved.pooling is not None + assert resolved.pooling.evaluations == [str(run_a.resolve()), str(run_b.resolve())] + assert resolved.pooling.output == str((tmp_path / "output").resolve()) + assert resolved.pooling.personas is None + assert resolved.pooling.skip_risk_analysis is False + + +def test_config_paths_resolve_from_repository_root(tmp_path: Path) -> None: + config = _write_config( + tmp_path, + _pooling_config( + [EVAL_FIXTURE_REL, f"{EVAL_FIXTURE_REL}/results.csv"], + personas="tests/fixtures/personas_with_risk.tsv", + ), + ) + parser = vera.build_parser() + + (resolved,) = pool.resolve_configs( + parser.parse_args(["pool", "--config", str(config)]) + ) + + assert resolved.pooling is not None + assert resolved.pooling.evaluations[0] == str(EVAL_FIXTURE.resolve()) + assert resolved.pooling.output == str((REPO_ROOT / "output").resolve()) + assert resolved.pooling.personas == str(PERSONAS_FIXTURE.resolve()) + + +@pytest.mark.parametrize( + "field", ["evaluations", "output", "personas", "skip_risk_analysis"] +) +def test_config_requires_every_pooling_field(tmp_path: Path, field: str) -> None: + data = _pooling_config([EVAL_FIXTURE_REL, EVAL_FIXTURE_REL]) + del data["pooling"][field] # type: ignore[union-attr] + config = _write_config(tmp_path, data) + parser = vera.build_parser() + + with pytest.raises( + cli_config.ConfigError, match=f"missing required field: {field}" + ): + pool.resolve_configs(parser.parse_args(["pool", "--config", str(config)])) + + +def test_config_rejects_one_evaluation(tmp_path: Path) -> None: + config = _write_config(tmp_path, _pooling_config([EVAL_FIXTURE_REL])) + parser = vera.build_parser() + + with pytest.raises(cli_config.ConfigError, match="at least two"): + pool.resolve_configs(parser.parse_args(["pool", "--config", str(config)])) + + +def test_config_rejects_any_run_defining_cli_flag(tmp_path: Path) -> None: + config = _write_config( + tmp_path, _pooling_config([EVAL_FIXTURE_REL, EVAL_FIXTURE_REL]) + ) + parser = vera.build_parser() + + with pytest.raises(cli_config.ConfigError, match="cannot be combined"): + pool.resolve_configs( + parser.parse_args(["pool", "--config", str(config), "-o", "elsewhere"]) + ) + + +def test_config_rejects_sections_pool_does_not_own(tmp_path: Path) -> None: + config = _write_config(tmp_path, {"scoring": {}}) + parser = vera.build_parser() + + with pytest.raises(cli_config.ConfigError, match="unknown top-level config field"): + pool.resolve_configs(parser.parse_args(["pool", "--config", str(config)])) + + +def test_print_round_trips_through_the_environment( + tmp_path: Path, monkeypatch: pytest.MonkeyPatch, capsys: pytest.CaptureFixture +) -> None: + run_a, run_b = _evaluations(tmp_path) + parser = vera.build_parser() + args = parser.parse_args( + ["pool", "--evaluations", str(run_a), str(run_b), "--print"] + ) + (original,) = pool.resolve_configs(args) + + assert pool.run(args) == 0 + printed = capsys.readouterr().out.strip() + assert printed.endswith("uv run python vera.py pool") + + payload = printed.split("=", 1)[1].rsplit(" uv run", 1)[0].strip("'") + monkeypatch.setenv(cli_config.VERA_RUN_CONFIG_ENV, payload) + (replayed,) = pool.resolve_configs(parser.parse_args(["pool"])) + + assert replayed == original + + +def test_pool_writes_merged_results_and_scores(tmp_path: Path) -> None: + run_a, run_b = _evaluations(tmp_path) + output = tmp_path / "pooled" + parser = vera.build_parser() + + exit_code = pool.run( + parser.parse_args( + ["pool", "--evaluations", str(run_a), str(run_b), "-o", str(output)] + ) + ) + + assert exit_code == 0 + folder = _pooled_folder(output) + rows = len(pd.read_csv(EVAL_FIXTURE / "results.csv")) + assert len(pd.read_csv(folder / "results.csv")) == 2 * rows + metadata = json.loads((folder / "pool_metadata.json").read_text()) + assert metadata["rows_per_source"] == [rows, rows] + scores = json.loads((folder / "scores/scores.json").read_text()) + assert scores["judge_model"] == "pooled" + assert not (folder / "scores/scores_by_risk.json").exists() + + +def test_personas_adds_the_risk_breakdown(tmp_path: Path) -> None: + run_a, run_b = _evaluations(tmp_path) + output = tmp_path / "pooled" + parser = vera.build_parser() + + exit_code = pool.run( + parser.parse_args( + [ + "pool", + "--evaluations", + str(run_a), + str(run_b), + "-o", + str(output), + "--personas", + str(PERSONAS_FIXTURE), + ] + ) + ) + + assert exit_code == 0 + assert (_pooled_folder(output) / "scores/scores_by_risk.json").is_file() + + +def test_skip_risk_analysis_wins_over_a_given_personas_file(tmp_path: Path) -> None: + run_a, run_b = _evaluations(tmp_path) + output = tmp_path / "pooled" + parser = vera.build_parser() + + exit_code = pool.run( + parser.parse_args( + [ + "pool", + "--evaluations", + str(run_a), + str(run_b), + "-o", + str(output), + "--personas", + str(PERSONAS_FIXTURE), + "--skip-risk-analysis", + ] + ) + ) + + assert exit_code == 0 + assert not (_pooled_folder(output) / "scores/scores_by_risk.json").exists() diff --git a/utils/config_schema.py b/utils/config_schema.py index f8927253..0d363382 100644 --- a/utils/config_schema.py +++ b/utils/config_schema.py @@ -533,12 +533,57 @@ def to_dict(self) -> dict[str, Any]: } +@dataclasses.dataclass(frozen=True) +class PoolingConfig: + """What pooling run to perform: the run-defining section for `vera pool`. + + `evaluations` names two or more judge evaluations (``j_*`` folders or the + ``results.csv`` inside them); pooling merges their rows and scores the + result, so one input would just be `vera score`. `output` is the folder the + pooled ``j_*`` folder is created in. `personas` and `skip_risk_analysis` + mean exactly what they mean in `ScoringSpec`: null personas skips the + risk-level breakdown rather than defaulting to the SI personas file. + """ + + evaluations: list[str] + output: str + personas: str | None + skip_risk_analysis: bool + + def __post_init__(self) -> None: + if ( + not isinstance(self.evaluations, list) + or len(self.evaluations) < 2 + or not all(isinstance(path, str) and path for path in self.evaluations) + ): + raise ValueError( + "pooling.evaluations must be a list of at least two path strings" + ) + if not isinstance(self.output, str) or not self.output: + raise ValueError("pooling.output must be a non-empty path string") + if self.personas is not None and ( + not isinstance(self.personas, str) or not self.personas + ): + raise ValueError("pooling.personas must be null or a non-empty path string") + if not isinstance(self.skip_risk_analysis, bool): + raise ValueError("pooling.skip_risk_analysis must be a boolean") + + def to_dict(self) -> dict[str, Any]: + return { + "evaluations": list(self.evaluations), + "output": self.output, + "personas": self.personas, + "skip_risk_analysis": self.skip_risk_analysis, + } + + @dataclasses.dataclass(frozen=True) class RunConfig: """One fully resolved `vera` run, ready to execute. Holds one section per command taking part in the run. `generate` populates - `generation`, `judge` populates `judging`, `score` populates `scoring`, and + `generation`, `judge` populates `judging`, `score` populates `scoring`, + `pool` populates `pooling`, and a later `pipeline` populates all three; at least one is required. `invocation` is a different kind of thing from those two, and the split is @@ -575,11 +620,18 @@ class RunConfig: generation: GenerationConfig | None = None judging: JudgingConfig | None = None scoring: ScoringConfig | None = None + pooling: PoolingConfig | None = None def __post_init__(self) -> None: - if self.generation is None and self.judging is None and self.scoring is None: + if ( + self.generation is None + and self.judging is None + and self.scoring is None + and self.pooling is None + ): raise ValueError( - "a run must define generation, judging, scoring, or a combination" + "a run must define generation, judging, scoring, pooling, " + "or a combination" ) def to_dict(self) -> dict[str, Any]: @@ -597,4 +649,6 @@ def to_dict(self) -> dict[str, Any]: config["judging"] = self.judging.to_dict() if self.scoring is not None: config["scoring"] = self.scoring.to_dict() + if self.pooling is not None: + config["pooling"] = self.pooling.to_dict() return config diff --git a/vera.py b/vera.py index 21de55de..2fc948b0 100644 --- a/vera.py +++ b/vera.py @@ -10,6 +10,7 @@ from vera_cli.generate import register as register_generate from vera_cli.judge import register as register_judge from vera_cli.pipeline import register as register_pipeline +from vera_cli.pool import register as register_pool from vera_cli.score import register as register_score @@ -21,6 +22,7 @@ def build_parser() -> argparse.ArgumentParser: register_generate(subparsers) register_judge(subparsers) register_score(subparsers) + register_pool(subparsers) register_pipeline(subparsers) return parser diff --git a/vera_cli/README.md b/vera_cli/README.md index 4b95f084..33119b2a 100644 --- a/vera_cli/README.md +++ b/vera_cli/README.md @@ -4,8 +4,8 @@ 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`, and `pipeline`. `pool` and -`resume` are specified in [../docs/architecture.md](../docs/architecture.md) but +Currently implemented: `generate`, `judge`, `score`, `pool`, and `pipeline`. +`resume` is specified in [../docs/architecture.md](../docs/architecture.md) but not built yet. ## Layout @@ -16,6 +16,7 @@ not built yet. | `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 two ways, both recorded in [../docs/vera-cli-use-cases.md](../docs/vera-cli-use-cases.md): no single-conversation mode, and no implicit working-directory output. | | `score.py` | The `score` command, same five steps. Two structural differences, both following from scoring calling no model: no `--target` (it reads a `results.csv` judging already wrote), and a synchronous `_execute`. Differs from legacy [`legacy/score.py`](../legacy/score.py) in one way, recorded in [../CHANGELOG.md](../CHANGELOG.md): omitting `--personas` skips the risk-level breakdown instead of defaulting to an SI personas file. | +| `pool.py` | The `pool` command, structured like `score`: no `--target`, a synchronous `_execute`, and no personas default. Calls `score.run_pooling`. | | `pipeline.py` | The `pipeline` command: generate, then judge, then score, feeding each stage's output to the next. A shim — it reuses the three single-stage resolvers and domain functions and owns no logic or defaults of its own. Its CLI shorthand is only `-c`/`-u`/`-j`/`--target`, because `--output` and `--max-concurrent` mean different things per stage; everything stage-specific comes from `--config`. Its input resolves in two halves because the chained paths cannot be known up front — see [../docs/pipeline.md](../docs/pipeline.md). | | `config.py` | Shared input handling: loading config JSON, the config-or-flags rule, turning config-stated strings into verified absolute paths, resolved-run rendering. | | `targets.py` | Target discovery and manifest validation — turning a target *name* into concrete, verified file paths. Builds on `config.py`'s path helpers rather than owning them. | diff --git a/vera_cli/pool.py b/vera_cli/pool.py new file mode 100644 index 00000000..612275de --- /dev/null +++ b/vera_cli/pool.py @@ -0,0 +1,289 @@ +"""The ``vera pool`` command: flags, resolution, and the workflow call. + +Structured like `vera_cli/score.py`, which follows the command contract in +`vera_cli/README.md`: + +1. `run` is the entry point `vera.py` dispatches to. +2. `resolve_configs` picks one input form and produces canonical `RunConfig`s. +3. `_execute` hands the resolved run to the scoring domain's `run_pooling`. +4. `register` declares the flags and attaches `run` as the subparser's handler. + +Pooling merges two or more judge evaluations into one ``results.csv`` and +scores it. It is a separate command from `score` (and `judge` never pools), +per `docs/architecture.md`. Like `score`, it calls no model, so `_execute` is +synchronous, and a missing `--personas` skips the risk-level breakdown rather +than defaulting to `data/SI/personas.tsv` as the legacy script does. +""" + +from __future__ import annotations + +import argparse +from pathlib import Path +from typing import Any + +from score import run_pooling +from utils.config_schema import InvocationConfig, PoolingConfig, RunConfig +from utils.debug import set_debug + +from .config import ( + ConfigError, + config_path, + existing_file, + flag_value, + path_from_root, + print_resolved_config, + render_invocation, + required, + resolve_input, +) + +# CLI behavior defaults, applied during resolution by `flag_value` rather than +# by the parser; see `vera_cli/generate.py` for why the parser cannot hold them. +DEFAULTS: dict[str, Any] = { + "output": "output", + "personas": None, + "skip_risk_analysis": False, +} + +# Top-level config keys `pool` accepts. +ALLOWED_CONFIG_FIELDS = {"pooling"} + + +def run(args: argparse.Namespace) -> int: + """Resolve the requested run and execute it. + + Resolution happens up front and completely, so a missing evaluation or + personas file fails before anything is written. + """ + run_configs = resolve_configs(args) + if args.print_only: + for run_config in run_configs: + print(render_invocation(run_config, command="pool")) + 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) + return _execute(run_configs) + + +def resolve_configs(args: argparse.Namespace) -> list[RunConfig]: + """Resolve either config JSON or CLI flags into exactly one canonical run.""" + try: + config, invocation = resolve_input( + args, + 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 pooling config: {error}") from error + + +def _existing_evaluation(path: str, *, field: str) -> str: + """Return `path` absolute, failing unless it is a folder or a results.csv. + + An evaluation is passed either as its ``j_*`` folder or as the + ``results.csv`` inside it, the two forms `run_pooling` accepts. + """ + resolved = Path(path).resolve() + if resolved.is_dir(): + return str(resolved) + if resolved.is_file() and resolved.name == "results.csv": + return str(resolved) + raise ConfigError( + f"{field} must be an evaluation folder or its results.csv: {resolved}" + ) + + +def _from_cli( + args: argparse.Namespace, invocation: InvocationConfig +) -> list[RunConfig]: + """Resolve CLI flags into one canonical run, applying CLI defaults. + + Paths resolve against the working directory, like any command-line tool. + """ + evaluations: list[str] | None = getattr(args, "evaluations", None) + # Not `required=True` on the parser: a config may supply the value instead. + if not evaluations: + raise ConfigError("pool requires --evaluations") + if len(evaluations) < 2: + raise ConfigError("pool needs at least two --evaluations to merge") + + personas = flag_value(args, "personas", defaults=DEFAULTS) + return [ + _run_config( + invocation=invocation, + evaluations=[ + _existing_evaluation(path, field="--evaluations") + for path in evaluations + ], + output=str(Path(flag_value(args, "output", defaults=DEFAULTS)).resolve()), + personas=( + existing_file(str(Path(personas).resolve()), field="--personas") + if personas is not None + else None + ), + skip_risk_analysis=flag_value( + args, "skip_risk_analysis", defaults=DEFAULTS + ), + ) + ] + + +def _from_config( + config: dict[str, Any], invocation: InvocationConfig +) -> list[RunConfig]: + """Resolve a config object into one canonical run. + + Every field is required, as in the other commands: a stored config fully + describes a run. Paths resolve against the repository root, and explicit + `null` is how a config says "no personas file". + """ + value = config.get("pooling") + if not isinstance(value, dict): + raise ConfigError("pool requires a pooling config object") + pooling = dict(value) + + evaluations = required(pooling, "evaluations", section="pooling config") + if not isinstance(evaluations, list) or not all( + isinstance(path, str) and path for path in evaluations + ): + raise ConfigError("pooling.evaluations must be a list of paths") + output = required(pooling, "output", section="pooling config") + if not isinstance(output, str) or not output: + raise ConfigError("pooling.output must be a path string") + personas = required(pooling, "personas", section="pooling config") + + return [ + _run_config( + invocation=invocation, + evaluations=[ + _existing_evaluation(path_from_root(path), field="pooling.evaluations") + for path in evaluations + ], + output=path_from_root(output), + personas=( + config_path(personas, field="pooling.personas") + if personas is not None + else None + ), + skip_risk_analysis=required( + pooling, "skip_risk_analysis", section="pooling config" + ), + ) + ] + + +def _run_config( + *, + invocation: InvocationConfig, + evaluations: list[str], + output: str, + personas: str | None, + skip_risk_analysis: bool, +) -> RunConfig: + """Assemble one canonical `RunConfig` holding a pooling section. + + Type enforcement happens at runtime in `PoolingConfig.__post_init__`. + """ + return RunConfig( + invocation=invocation, + pooling=PoolingConfig( + evaluations=evaluations, + output=output, + personas=personas, + skip_risk_analysis=skip_risk_analysis, + ), + ) + + +def _execute(run_configs: list[RunConfig]) -> int: + """Hand each resolved run to the scoring domain. + + `run_pooling` raises on unusable input (no rows, a missing folder); that is + translated to the `ConfigError` every other bad input produces. + """ + for run_config in run_configs: + pooling = run_config.pooling + if pooling is None: # pragma: no cover - resolve_configs always sets it + raise ConfigError("pool produced a run with no pooling section") + try: + run_pooling( + evaluations=list(pooling.evaluations), + output_parent=pooling.output, + personas_tsv=pooling.personas, + skip_risk_analysis=pooling.skip_risk_analysis, + ) + except (FileNotFoundError, ValueError) as error: + raise ConfigError(str(error)) from error + return 0 + + +def register(subparsers: argparse._SubParsersAction) -> None: + """Register ``pool`` with the root parser. + + Uses the same `argparse.SUPPRESS` convention as the other commands, and + likewise omits `--sample` and `--into`: pooling reads finished results in + one pass, so there is no work to cap or continue. + """ + parser = subparsers.add_parser( + "pool", + help="Merge two or more judge evaluations into one scored result", + ) + parser.add_argument( + "--evaluations", + nargs="+", + metavar="", + default=argparse.SUPPRESS, + help=( + "Two or more judge evaluations to merge, each an evaluation folder " + "(j_*) or the results.csv inside it" + ), + ) + parser.add_argument( + "-o", + "--output", + metavar="", + default=argparse.SUPPRESS, + help=( + "Folder the pooled j_* folder is created in (default: output/ in " + "the working directory)" + ), + ) + parser.add_argument( + "--personas", + metavar="", + default=argparse.SUPPRESS, + help=( + "Personas file backing the risk-level breakdown " + "(default: none, which skips that breakdown)" + ), + ) + parser.add_argument( + "--skip-risk-analysis", + action="store_true", + default=argparse.SUPPRESS, + help="Skip the risk-level breakdown even when --personas is given", + ) + parser.add_argument("--config", help="JSON path or '-' for stdin") + 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)