From f9bc7fc546c82ddec1234ecd8680cb7da966dbdf Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:30:13 +0000 Subject: [PATCH 1/3] Make golden-diff eval runs reproducible across providers Every evals/run_evals.py invocation now writes a provider-neutral, schema-versioned run record (run-record.json, sorted keys, LF) to its work dir, described by the committed evals/run-record.schema.json. It captures the skilldeck version, checkout commit and dirty state, the runner's own digest, each fixture's content digest, each skill's version, canonical digest and installed-file digest, the harness name, exact command template, version probe and model, and one entry per planned run: status (passed, failed, agent_failed, timed_out, error, not_run), timestamps, duration, exit code, finding count, scorer reasons, and work-dir-relative paths to the raw report and stderr. Raw output is only embedded with --include-reports; usage and cost are null until a harness reports them. - --harness claude|codex|custom presets pair each agent CLI's non-interactive command (claude -p, codex exec) with its adapter; --agent-cmd still overrides, and --model is passed through {model}. - --dry-run validates fixtures (expected.yaml, layout, installability) and prints the plan without running any subprocess; exits 2 on problems. - --max-runs (default 50) refuses an oversized plan before anything runs; execution stays sequential. - --replay RECORD re-runs a record's configuration and refuses if any fixture, skill, or installed skill file digest changed. - A missing agent command, an unbuildable repo, or an interrupt is recorded instead of aborting without a record; the remaining runs are not_run. Stand-in-agent tests cover the record fields and ordering (validated against the schema with a minimal in-test validator), failures, timeouts, missing commands, dry runs making no subprocess calls, budgets, harness presets, and replay digest mismatches. Part of #74. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- CHANGELOG.md | 18 + evals/README.md | 120 ++++- evals/run-record.schema.json | 259 ++++++++++ evals/run_evals.py | 902 +++++++++++++++++++++++++++++++++-- tests/test_eval_runs.py | 776 ++++++++++++++++++++++++++++++ tests/test_eval_scoring.py | 30 +- 6 files changed, 2045 insertions(+), 60 deletions(-) create mode 100644 evals/run-record.schema.json create mode 100644 tests/test_eval_runs.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 7df8fde..f3c5e57 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -618,6 +618,24 @@ All notable changes to this project are documented here. The format is based on positives. A structural test rejects plant keywords that appear verbatim in the planted file, and per-fixture sample reports check that a correct report passes and a finding about a neighbouring defect satisfies no plant. +- Reproducible eval runs (#74): every `evals/run_evals.py` invocation writes + a provider-neutral, schema-versioned `run-record.json` + (`evals/run-record.schema.json`, sorted keys) to its work dir. It holds the + skilldeck version and git commit, each fixture's content digest, each + skill's version, canonical digest and installed-file digest, the harness, + exact command template, version and model, and one entry per planned run + (passed, failed, agent failure, timeout, error, or not run) with timing, + exit code, finding count and the scorer's reasons. Raw reports and stderr + stay in separate files it points to (`--include-reports` embeds them). + `--harness claude|codex|custom` presets pair each agent CLI's + non-interactive command with its adapter, and `--model` passes a model + through `{model}`. `--dry-run` validates the fixtures and prints the plan + without running anything; `--max-runs N` (default 50) refuses an oversized + plan before it starts; runs stay sequential. `--replay RECORD` re-runs a + record's configuration and refuses if any fixture or skill changed since. + A missing agent command is now recorded, and stops the remaining runs, + instead of aborting without a record; a passing run keeps its record and + raw output (only the review repos are deleted). - `skilldeck provenance --verify` re-hashes each installed skill's `meta.yaml` and `skill.md` and exits 1, naming the skill, when one no longer matches its recorded canonical digest, is missing, or has unexpected files beside it. diff --git a/evals/README.md b/evals/README.md index 367b069..8089a46 100644 --- a/evals/README.md +++ b/evals/README.md @@ -8,28 +8,39 @@ when changing a skill's wording. ## Running -Requires the [Claude Code CLI](https://claude.com/claude-code) (or another -agent CLI, see `--adapter`) and an API key; **it calls a real agent and costs -real money**, which is why it is manual and not part of CI. +Requires an agent CLI — [Claude Code](https://claude.com/claude-code) by +default, or the OpenAI Codex CLI, or any other through `--agent-cmd` — and its +credentials; **it calls a real agent and costs real money**, which is why it is +manual and not part of CI. Check the plan first with `--dry-run`, which invokes +no agent. ```bash -python evals/run_evals.py # all fixtures +python evals/run_evals.py --dry-run # validate fixtures, print the plan +python evals/run_evals.py # all fixtures, Claude Code python evals/run_evals.py --skill logging # one skill's fixtures python evals/run_evals.py --skill authentication-review-saml # one fixture -python evals/run_evals.py --repeat 5 # run each fixture 5 times -python evals/run_evals.py --agent-cmd 'claude -p {prompt}' # default -python evals/run_evals.py --adapter codex --agent-cmd 'codex exec {prompt}' -python evals/run_evals.py --keep # keep temp repos + reports +python evals/run_evals.py --repeat 5 --skill logging # pass rate per fixture +python evals/run_evals.py --harness codex # Codex CLI + the codex adapter +python evals/run_evals.py --model sonnet # request a model from the harness +python evals/run_evals.py --agent-cmd 'my-agent --print {prompt}' # any CLI +python evals/run_evals.py --replay path/to/run-record.json # same config again +python evals/run_evals.py --keep # keep the review repos ``` | Option | Meaning | | --- | --- | | `--skill NAME` | Run only the fixtures that exercise skill `NAME`, or the one fixture whose directory is `NAME`. | -| `--agent-cmd CMD` | Agent command line; `{prompt}` is replaced by the review prompt. Default `claude -p {prompt}`. | -| `--adapter NAME` | Which skilldeck adapter installs the skill into the temp repo (`claude`, `codex`, `copilot`, `cursor`, `kiro`; default `claude`). The prompt names the installed file's path, so pair it with that agent's `--agent-cmd`. | +| `--harness NAME` | Agent CLI preset: `claude` (default), `codex`, or `custom` (see [Harnesses](#harnesses)). A preset sets the command and the adapter that matches it. | +| `--agent-cmd CMD` | Agent command line, overriding the preset's; `{prompt}` is replaced by the review prompt and `{model}` by `--model`. Without `--harness`, it makes a `custom` harness. | +| `--adapter NAME` | Which skilldeck adapter installs the skill into the temp repo (`claude`, `codex`, `copilot`, `cursor`, `kiro`). Defaults to the harness's (`claude` for `custom`). The prompt names the installed file's path, so it must be the adapter the agent reads. | +| `--model NAME` | Model to request, substituted for `{model}` (the presets pass it as `--model NAME`) and recorded. Without it the harness's own default is used, and not recorded. | | `--repeat N` | Run each fixture `N` times (fresh repo each time) and print its pass rate — agents are nondeterministic, so one run says little about a borderline fixture. | | `--timeout S` | Per-run agent timeout in seconds (default 600). | -| `--keep` | Keep the temp repos even when every run passes. | +| `--max-runs N` | Refuse to start if more than `N` runs (fixtures × repeats) are planned (default 50). | +| `--dry-run` | Validate the fixtures and print the planned runs; no agent (or anything else) is run. Exits 2 on an invalid fixture or a plan over `--max-runs`. | +| `--replay RECORD` | Re-run a [run record](#run-records)'s exact configuration; see [Replaying](#replaying). | +| `--include-reports` | Also copy each run's raw stdout and stderr into the run record. | +| `--keep` | Keep the review repos even when every run passes. | For each fixture (and each repeat) the runner: @@ -42,9 +53,86 @@ For each fixture (and each repeat) the runner: 4. scores the agent's **stdout** (see [Scoring](#scoring)). A run fails outright — without scoring — if the agent exits non-zero or times -out. Failing runs print the agent's stderr and keep their temp directory; each -repo contains the raw `report.txt` (stdout) and `stderr.txt`. The process exits -non-zero if any run failed. +out; failing runs print the agent's stderr. Runs are **sequential** (concurrency +1): one agent at a time, so `--max-runs` bounds the spend and the wall-clock +time together. + +Everything lands in a temp work dir, printed at the start: + +``` +skilldeck-evals-XXXX/ +├── run-record.json # the run record (below) +├── artifacts/run-//report.txt # raw stdout, the scored report +├── artifacts/run-//stderr.txt +└── repos/run-// # the review repos +``` + +The review repos are deleted when every run passes (unless `--keep`); the +record and the raw output are always kept. The process exits 0 when every run +passed, 1 when any failed, 2 when the evals could not run (invalid fixture, +budget, changed digests on replay, agent command not found) and 130 when +interrupted — the record is written in every case that started running. + +## Harnesses + +| Harness | Command | Adapter | Version probe | +| --- | --- | --- | --- | +| `claude` | `claude -p {prompt}`; with a model `claude --model {model} -p {prompt}` | `claude` | `claude --version` | +| `codex` | `codex exec {prompt}`; with a model `codex exec --model {model} {prompt}` | `codex` | `codex --version` | +| `custom` | `--agent-cmd`, verbatim | `--adapter` (default `claude`) | none | + +Both presets run the agent's documented non-interactive mode with no other +flags: Claude Code's print mode, and `codex exec`, which prints the final +message on stdout (progress goes to stderr, which is not scored) and runs in a +read-only sandbox by default. The Codex preset is best-effort — it has not yet +been exercised in a recorded run. `--agent-cmd` overrides a preset's command +but keeps its name, adapter and version probe (the command's own executable +with `--version`); add flags there, such as an approval or sandbox mode. A +harness that reports token usage or cost would fill the record's `usage` and +`cost_usd`; no preset parses them yet, so both are `null`. + +## Run records + +Every run writes `run-record.json`, a provider-neutral, schema-versioned record +([`run-record.schema.json`](run-record.schema.json), JSON Schema 2020-12) with +sorted keys and LF newlines, so equal records are equal bytes on every +platform: + +- **what ran**: the skilldeck version, the checkout's git commit and whether + it had uncommitted changes (`null` outside a checkout), and the digest of + `run_evals.py` itself (the scorer and the prompt); +- **against what**: per fixture, a digest of every file in its directory + (newlines normalised, so a Windows checkout agrees), the skill's name, + version, canonical digest (the one in `src/skilldeck/_content_manifest.json`) + and the digest of the file the adapter installed, plus the exact prompt; +- **how**: the harness name, the exact command template, the model (if + requested), the harness version (the probe's first line, `null` if it + failed), the adapter, and the repeat, timeout and budget; +- **every planned run**: its status — `passed`, `failed` (scored and + missed), `agent_failed` (non-zero exit), `timed_out`, `error` (the repo + could not be built, or the agent could not start) or `not_run` (the + invocation stopped early) — with start and end timestamps, duration, exit + code, parsed finding count, the scorer's reasons, and the paths of its raw + output, relative to the record; +- a **summary**: planned, attempted, passed, failed and not-run counts. + +The record never contains the raw reports or stderr unless you pass +`--include-reports`, so it can be shared as-is; the raw files stay in the work +dir beside it. + +## Replaying + +`--replay RECORD` re-runs the recorded fixtures with the recorded harness, +command, model, adapter, repeat count and timeout (so it takes no other +configuration options; `--max-runs`, `--dry-run`, `--keep` and +`--include-reports` still apply). Before anything runs it recomputes every +fixture digest and skill digest and **refuses** (exit 2) if any fixture, skill +or installed skill file changed since the record — a changed eval is a +different experiment. A different harness version or a changed runner is +printed as a note, not refused. The new record's `config.replay_of` holds the +digest of the record it replayed; comparing the two records' runs is the +variance check. `--replay RECORD --dry-run` verifies the digests without +running anything. ## Scoring @@ -122,7 +210,9 @@ calls. Each planted fixture also has sample reports there (`SAMPLE_REPORTS`): a correct report must pass, and a finding about a different real defect in the same file must satisfy no plant, which catches keywords that are too narrow to match or generic enough to match the wrong -finding. The scorer itself is unit-tested in `tests/test_eval_scoring.py`. +finding. The scorer itself is unit-tested in `tests/test_eval_scoring.py`, and +the run records, harness presets, budgets, dry runs and replay, with stand-in +agents, in `tests/test_eval_runs.py`. A skill may have more than one fixture: name the directory for the skill, or add a `-` suffix (e.g. `ci-workflow-review-gitlab`) and set the diff --git a/evals/run-record.schema.json b/evals/run-record.schema.json new file mode 100644 index 0000000..fb263fe --- /dev/null +++ b/evals/run-record.schema.json @@ -0,0 +1,259 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/IcebergAI/skilldeck/blob/main/evals/run-record.schema.json", + "title": "skilldeck eval run record", + "description": "One invocation of evals/run_evals.py: what was run, against which skill and fixture content, by which harness, and every planned run's outcome. Raw agent output is referenced by path (artifacts) and only embedded with --include-reports (raw).", + "type": "object", + "additionalProperties": false, + "required": [ + "schema_version", + "record_type", + "status", + "started_at", + "finished_at", + "skilldeck", + "environment", + "harness", + "adapter", + "config", + "fixtures", + "summary" + ], + "properties": { + "schema_version": {"const": 1}, + "record_type": {"const": "skilldeck-eval-run"}, + "status": { + "description": "incomplete: the invocation stopped early (agent command missing, interrupted); the remaining runs are recorded as not_run.", + "enum": ["complete", "incomplete"] + }, + "started_at": {"$ref": "#/$defs/timestamp"}, + "finished_at": {"$ref": "#/$defs/timestamp"}, + "skilldeck": { + "type": "object", + "additionalProperties": false, + "required": ["version", "git_commit", "git_dirty", "runner_sha256"], + "properties": { + "version": {"type": "string"}, + "git_commit": { + "description": "HEAD of the skilldeck checkout, or null outside one.", + "type": ["string", "null"], + "pattern": "^[0-9a-f]{40}([0-9a-f]{24})?$" + }, + "git_dirty": { + "description": "Whether the checkout had uncommitted changes; null if unknown.", + "type": ["boolean", "null"] + }, + "runner_sha256": { + "description": "Digest of evals/run_evals.py (scorer and prompt).", + "$ref": "#/$defs/digest" + } + } + }, + "environment": { + "type": "object", + "additionalProperties": false, + "required": ["python", "platform"], + "properties": { + "python": {"type": "string"}, + "platform": {"type": "string"} + } + }, + "harness": { + "type": "object", + "additionalProperties": false, + "required": ["name", "command", "model", "version", "version_command"], + "properties": { + "name": {"enum": ["claude", "codex", "custom"]}, + "command": { + "description": "The exact agent command template: {prompt} is the review prompt, {model} the model.", + "type": "string", + "pattern": "\\{prompt\\}" + }, + "model": { + "description": "The model requested through {model}; null leaves the harness default.", + "type": ["string", "null"] + }, + "version": { + "description": "First line of the version probe's output; null if it failed or the harness is custom.", + "type": ["string", "null"] + }, + "version_command": { + "type": ["array", "null"], + "items": {"type": "string"} + } + } + }, + "adapter": {"type": "string"}, + "config": { + "type": "object", + "additionalProperties": false, + "required": [ + "repeat", + "timeout_s", + "max_runs", + "jobs", + "include_reports", + "replay_of" + ], + "properties": { + "repeat": {"type": "integer", "minimum": 1}, + "timeout_s": {"type": "integer", "minimum": 1}, + "max_runs": {"type": "integer", "minimum": 1}, + "jobs": { + "description": "Concurrency; runs are sequential.", + "const": 1 + }, + "include_reports": {"type": "boolean"}, + "replay_of": { + "description": "Digest of the run record this invocation replayed, if any.", + "anyOf": [{"$ref": "#/$defs/digest"}, {"type": "null"}] + } + } + }, + "fixtures": { + "type": "array", + "items": {"$ref": "#/$defs/fixture"} + }, + "summary": { + "type": "object", + "additionalProperties": false, + "required": ["planned", "attempted", "passed", "failed", "not_run"], + "properties": { + "planned": {"type": "integer", "minimum": 0}, + "attempted": {"type": "integer", "minimum": 0}, + "passed": {"type": "integer", "minimum": 0}, + "failed": {"type": "integer", "minimum": 0}, + "not_run": {"type": "integer", "minimum": 0} + } + } + }, + "$defs": { + "digest": {"type": "string", "pattern": "^sha256:[0-9a-f]{64}$"}, + "timestamp": { + "description": "UTC, ISO 8601.", + "type": "string", + "pattern": "^[0-9]{4}-[0-9]{2}-[0-9]{2}T[0-9]{2}:[0-9]{2}:[0-9]{2}(\\.[0-9]+)?Z$" + }, + "fixture": { + "type": "object", + "additionalProperties": false, + "required": [ + "name", + "digest", + "skill", + "prompt", + "plant_count", + "max_findings", + "runs" + ], + "properties": { + "name": {"type": "string"}, + "digest": { + "description": "Digest of every file in the fixture directory (newlines normalised).", + "$ref": "#/$defs/digest" + }, + "skill": { + "type": "object", + "additionalProperties": false, + "required": ["name", "version", "canonical_sha256", "rendered_sha256"], + "properties": { + "name": {"type": "string"}, + "version": {"type": "string"}, + "canonical_sha256": { + "description": "skilldeck.provenance.canonical_skill_digest of meta.yaml and skill.md.", + "$ref": "#/$defs/digest" + }, + "rendered_sha256": { + "description": "Digest of the file the adapter installs: what the agent reads.", + "$ref": "#/$defs/digest" + } + } + }, + "prompt": {"type": "string"}, + "plant_count": {"type": "integer", "minimum": 0}, + "max_findings": {"type": "integer", "minimum": 0}, + "runs": { + "type": "array", + "items": {"$ref": "#/$defs/run"} + } + } + }, + "run": { + "type": "object", + "additionalProperties": false, + "required": [ + "attempt", + "status", + "passed", + "problems", + "started_at", + "finished_at", + "duration_s", + "exit_code", + "timed_out", + "finding_count", + "artifacts", + "raw", + "usage", + "cost_usd" + ], + "properties": { + "attempt": {"type": "integer", "minimum": 1}, + "status": { + "enum": [ + "passed", + "failed", + "agent_failed", + "timed_out", + "error", + "not_run" + ] + }, + "passed": {"type": "boolean"}, + "problems": { + "description": "Why the run failed, was not scored, or was not run.", + "type": "array", + "items": {"type": "string"} + }, + "started_at": {"anyOf": [{"$ref": "#/$defs/timestamp"}, {"type": "null"}]}, + "finished_at": {"anyOf": [{"$ref": "#/$defs/timestamp"}, {"type": "null"}]}, + "duration_s": {"type": ["number", "null"], "minimum": 0}, + "exit_code": { + "description": "null if the agent never exited on its own (timeout) or never ran.", + "type": ["integer", "null"] + }, + "timed_out": {"type": "boolean"}, + "finding_count": {"type": ["integer", "null"], "minimum": 0}, + "artifacts": { + "description": "Raw output files, as POSIX paths relative to the record's directory.", + "type": ["object", "null"], + "additionalProperties": false, + "required": ["report", "stderr"], + "properties": { + "report": {"type": "string"}, + "stderr": {"type": "string"} + } + }, + "raw": { + "description": "The raw report and stderr, only with --include-reports.", + "type": ["object", "null"], + "additionalProperties": false, + "required": ["report", "stderr"], + "properties": { + "report": {"type": "string"}, + "stderr": {"type": "string"} + } + }, + "usage": { + "description": "Token usage, when the harness reports it; null otherwise.", + "type": ["object", "null"] + }, + "cost_usd": { + "description": "Cost, when the harness reports it; null otherwise.", + "type": ["number", "null"], + "minimum": 0 + } + } + } + } +} diff --git a/evals/run_evals.py b/evals/run_evals.py index 4ed85aa..2458f06 100644 --- a/evals/run_evals.py +++ b/evals/run_evals.py @@ -18,24 +18,43 @@ words, case-insensitive) at or above its optional ``min-severity``, and the finding count must not exceed ``max-findings``. +Every invocation writes a provider-neutral run record (``run-record.json``, +see ``evals/run-record.schema.json``) to its work dir: the skilldeck version +and commit, each skill's and fixture's digest, the harness, its command +template, version and model, and one entry per planned run -- passed, failed, +timed out, errored or not run. Raw reports and stderr stay in the work dir as +separate files the record points to. ``--replay`` re-runs a record's exact +configuration after checking that no skill or fixture changed since. + This calls a real agent and costs real money -- it is run manually (e.g. before a release), not in CI. CI only validates fixture structure and the scorer, via ``tests/test_eval_fixtures.py`` and ``tests/test_eval_scoring.py``. +Runs are sequential (concurrency 1), and more than ``--max-runs`` planned runs +(default 50) are refused before anything starts. Usage: python evals/run_evals.py # all fixtures, claude CLI python evals/run_evals.py --skill logging # one skill's fixtures - python evals/run_evals.py --repeat 5 # pass rate per fixture + python evals/run_evals.py --repeat 5 --skill logging # pass rate + python evals/run_evals.py --harness codex # codex CLI + codex adapter + python evals/run_evals.py --model sonnet # pass a model to the harness python evals/run_evals.py --agent-cmd 'claude -p {prompt}' python evals/run_evals.py --adapter codex --agent-cmd 'codex exec {prompt}' + python evals/run_evals.py --dry-run # validate + plan, no agent + python evals/run_evals.py --replay run-record.json # same config again python evals/run_evals.py --keep # keep temp repos to inspect """ from __future__ import annotations import argparse +import contextlib +import dataclasses import functools +import hashlib +import json import os +import platform import re import shlex import shutil @@ -44,16 +63,26 @@ import sys import tempfile import textwrap +import time from collections.abc import Sequence from dataclasses import dataclass +from datetime import datetime, timezone from pathlib import Path +from typing import TypedDict import yaml ROOT = Path(__file__).resolve().parent.parent sys.path.insert(0, str(ROOT / "src")) +from skilldeck import __version__ # noqa: E402 from skilldeck.adapters import ADAPTERS # noqa: E402 +from skilldeck.provenance import ( # noqa: E402 + canonical_json, + canonical_skill_digest, + normalise_text, + sha256_text, +) from skilldeck.registry import Skill, discover_skills # noqa: E402 from skilldeck.targets import Scope # noqa: E402 @@ -65,6 +94,17 @@ "using the {skill} skill installed at {skill_path}. Output the findings " "report exactly as the skill specifies." ) +#: the default cap on planned runs (fixtures x repeats) per invocation +DEFAULT_MAX_RUNS = 50 +DEFAULT_TIMEOUT = 600 +#: seconds allowed for a harness's ``--version`` probe +VERSION_PROBE_TIMEOUT = 30 +RECORD_NAME = "run-record.json" +RECORD_SCHEMA_VERSION = 1 +RECORD_TYPE = "skilldeck-eval-run" +_FIXTURE_DOMAIN = b"skilldeck-eval-fixture-v1\0" +_COMMIT_RE = re.compile(r"^[0-9a-f]{40}$") +_DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$") #: the finding severity scale from docs/finding-output.md, lowest first SEVERITIES = ("low", "medium", "high", "critical") @@ -105,6 +145,65 @@ class FixtureError(ValueError): """An ``expected.yaml`` that doesn't match the fixture schema.""" +class ConfigError(ValueError): + """Options that can't make a valid run (harness, budget, replay record).""" + + +class AgentStartError(RuntimeError): + """The agent command could not be started at all.""" + + +@dataclass(frozen=True) +class HarnessPreset: + """A known agent CLI: its non-interactive command and matching adapter.""" + + adapter: str + #: the command template; ``{prompt}`` is replaced by the review prompt + command: str + #: the same command with the harness's model option (``{model}``) + model_command: str + + +#: built-in harnesses; ``--harness custom`` takes its command from --agent-cmd +HARNESSES = { + # Claude Code's print mode: one non-interactive turn, the reply on stdout + "claude": HarnessPreset( + adapter="claude", + command=DEFAULT_AGENT_CMD, + model_command="claude --model {model} -p {prompt}", + ), + # Codex CLI's non-interactive mode: progress on stderr, the final message + # on stdout; its default sandbox is read-only, which a review needs no + # more than + "codex": HarnessPreset( + adapter="codex", + command="codex exec {prompt}", + model_command="codex exec --model {model} {prompt}", + ), +} +DEFAULT_HARNESS = "claude" +CUSTOM_HARNESS = "custom" + + +@dataclass(frozen=True) +class Harness: + """The resolved agent to run: which CLI, how, and with which adapter.""" + + name: str + #: the exact command template: ``{prompt}``, and ``{model}`` with a model + command: str + adapter: str + #: the model passed through ``{model}``; None leaves the harness default + model: str | None = None + + @property + def version_command(self) -> list[str] | None: + """How to ask a preset harness its version (None for a custom one).""" + if self.name not in HARNESSES: + return None + return [shlex.split(self.command)[0], "--version"] + + @dataclass(frozen=True) class Plant: file: str @@ -595,8 +694,28 @@ def _text(output: str | bytes | None) -> str: return output or "" -def run_agent(agent_cmd: str, prompt: str, repo: Path, timeout: int) -> AgentRun: - cmd = [part.replace("{prompt}", prompt) for part in shlex.split(agent_cmd)] +def agent_argv(agent_cmd: str, prompt: str, model: str | None = None) -> list[str]: + """The agent's command line with ``{prompt}`` and ``{model}`` substituted. + + One pass, so a prompt that happens to contain ``{model}`` (or a model + name containing ``{prompt}``) is passed through as-is. + """ + values = {"prompt": prompt} if model is None else {"prompt": prompt, "model": model} + pattern = re.compile(r"\{(" + "|".join(values) + r")\}") + return [ + pattern.sub(lambda match: values[match[1]], part) + for part in shlex.split(agent_cmd) + ] + + +def run_agent( + agent_cmd: str, + prompt: str, + repo: Path, + timeout: int, + model: str | None = None, +) -> AgentRun: + cmd = agent_argv(agent_cmd, prompt, model) try: result = subprocess.run( cmd, @@ -608,12 +727,15 @@ def run_agent(agent_cmd: str, prompt: str, repo: Path, timeout: int) -> AgentRun timeout=timeout, ) except FileNotFoundError: - raise SystemExit( - f"error: agent command not found: {cmd[0]!r} — install it or pass " - "--agent-cmd" + raise AgentStartError( + f"agent command not found: {cmd[0]!r} — install it or pass --agent-cmd" ) from None except subprocess.TimeoutExpired as exc: return AgentRun(_text(exc.stdout), _text(exc.stderr), None, timeout) + except OSError as exc: # found but not runnable, e.g. not executable + raise AgentStartError( + f"agent command {cmd[0]!r} could not start: {exc}" + ) from None return AgentRun(result.stdout, result.stderr, result.returncode, timeout) @@ -625,6 +747,562 @@ def select_fixtures(skill: str | None = None) -> list[Fixture]: return [f for f in fixtures if skill is None or skill in (f.name, f.skill)] +# -- harnesses --------------------------------------------------------------- + + +def resolve_harness( + name: str | None = None, + agent_cmd: str | None = None, + adapter: str | None = None, + model: str | None = None, +) -> Harness: + """Combine --harness, --agent-cmd, --adapter and --model into a Harness. + + Without ``name``, an ``agent_cmd`` makes a custom harness and no command + the default (claude). A preset supplies its command -- the one that passes + ``{model}`` when a model is given -- and its matching adapter; + ``agent_cmd`` and ``adapter`` override them. + """ + if name is None: + name = CUSTOM_HARNESS if agent_cmd is not None else DEFAULT_HARNESS + if name == CUSTOM_HARNESS: + if agent_cmd is None: + raise ConfigError("--harness custom needs --agent-cmd") + command, default_adapter = agent_cmd, DEFAULT_ADAPTER + elif name in HARNESSES: + preset = HARNESSES[name] + if agent_cmd is not None: + command = agent_cmd + else: + command = preset.command if model is None else preset.model_command + default_adapter = preset.adapter + else: + choices = [*sorted(HARNESSES), CUSTOM_HARNESS] + raise ConfigError(f"unknown harness {name!r}; choose from {choices}") + adapter = adapter or default_adapter + if adapter not in ADAPTERS: + raise ConfigError( + f"unknown adapter {adapter!r}; choose from {sorted(ADAPTERS)}" + ) + if model is not None and not model.strip(): + raise ConfigError("--model must not be empty") + try: + parts = shlex.split(command) + except ValueError as exc: + raise ConfigError(f"agent command {command!r}: {exc}") from None + if not parts: + raise ConfigError("the agent command is empty") + if not any("{prompt}" in part for part in parts): + raise ConfigError(f"agent command {command!r} has no {{prompt}} placeholder") + has_model = any("{model}" in part for part in parts) + if model is not None and not has_model: + raise ConfigError( + f"--model needs a {{model}} placeholder in the agent command {command!r}" + ) + if model is None and has_model: + raise ConfigError( + f"agent command {command!r} has a {{model}} placeholder; pass --model" + ) + return Harness(name=name, command=command, adapter=adapter, model=model) + + +def harness_version(command: Sequence[str] | None) -> str | None: + """The first line a harness's ``--version`` prints; None if that fails.""" + if not command: + return None + try: + result = subprocess.run( + list(command), + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=VERSION_PROBE_TIMEOUT, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + lines = [line.strip() for line in result.stdout.splitlines() if line.strip()] + return lines[0] if result.returncode == 0 and lines else None + + +# -- identities and digests -------------------------------------------------- + + +class SkillIdentity(TypedDict): + name: str + version: str + #: skilldeck.provenance.canonical_skill_digest of meta.yaml + skill.md + canonical_sha256: str + #: the file the adapter installs, i.e. exactly what the agent reads + rendered_sha256: str + + +class FixtureIdentity(TypedDict): + name: str + digest: str + skill: SkillIdentity + + +def fixture_digest(path: Path) -> str: + """Hash every file under a fixture directory, by POSIX path and content. + + Domain-separated and length-framed like the canonical skill digest. UTF-8 + text is hashed with normalised newlines, so a CRLF checkout on Windows + and an LF one agree; any other file is hashed as raw bytes. + """ + digest = hashlib.sha256(_FIXTURE_DOMAIN) + files = sorted( + (file.relative_to(path).as_posix(), file) + for file in path.rglob("*") + if file.is_file() + ) + for relative, file in files: + data = file.read_bytes() + with contextlib.suppress(UnicodeDecodeError): + data = normalise_text(data.decode("utf-8")).encode("utf-8") + name = relative.encode("utf-8") + digest.update(len(name).to_bytes(4, "big")) + digest.update(name) + digest.update(len(data).to_bytes(8, "big")) + digest.update(data) + return f"sha256:{digest.hexdigest()}" + + +def runner_digest() -> str: + """The digest of this runner (the scorer and the prompt live here).""" + return sha256_text(Path(__file__).read_text(encoding="utf-8")) + + +def _git_output(*args: str) -> str | None: + try: + result = subprocess.run( + ["git", *args], + cwd=ROOT, + capture_output=True, + text=True, + encoding="utf-8", + errors="replace", + timeout=VERSION_PROBE_TIMEOUT, + check=False, + ) + except (OSError, subprocess.SubprocessError): + return None + return result.stdout if result.returncode == 0 else None + + +def _same_path(a: Path, b: Path) -> bool: + try: + return a.resolve() == b.resolve() + except OSError: + return False + + +def source_identity() -> dict[str, object]: + """The skilldeck version and, in a git checkout, its commit and state.""" + commit: str | None = None + dirty: bool | None = None + toplevel = _git_output("rev-parse", "--show-toplevel") + # only this checkout's own commit: a source tree unpacked inside some + # other repository must not borrow that repository's HEAD + if toplevel is not None and _same_path(Path(toplevel.strip()), ROOT): + head = (_git_output("rev-parse", "HEAD") or "").strip() + if _COMMIT_RE.fullmatch(head): + commit = head + status = _git_output("status", "--porcelain") + dirty = None if status is None else bool(status.strip()) + return { + "version": __version__, + "git_commit": commit, + "git_dirty": dirty, + "runner_sha256": runner_digest(), + } + + +# -- planning ---------------------------------------------------------------- + + +@dataclass(frozen=True) +class PlannedFixture: + """A validated fixture, with the identity its runs are recorded under.""" + + fixture: Fixture + prompt: str + identity: FixtureIdentity + + +def fixture_layout_problems(fixture: Fixture) -> list[str]: + """What stops ``fixture`` from building a review repo with its plants.""" + problems = [ + f"{fixture.name}: missing {part}/ directory" + for part in ("base", "change") + if not (fixture.path / part).is_dir() + ] + problems.extend( + f"{fixture.name}: plant file {plant.file} is not in change/" + for plant in fixture.plants + if not (fixture.path / "change" / plant.file).is_file() + ) + return problems + + +def plan_fixture(fixture: Fixture, adapter: str) -> PlannedFixture: + """Validate ``fixture`` for ``adapter`` and pin down its identity.""" + problems = fixture_layout_problems(fixture) + if problems: + raise FixtureError("; ".join(problems)) + skill = installable_skill(fixture, adapter) + meta_text = (skill.path / "meta.yaml").read_text(encoding="utf-8") + return PlannedFixture( + fixture=fixture, + prompt=build_prompt(fixture, adapter), + identity={ + "name": fixture.name, + "digest": fixture_digest(fixture.path), + "skill": { + "name": skill.name, + "version": skill.version, + "canonical_sha256": canonical_skill_digest(meta_text, skill.body), + "rendered_sha256": sha256_text(ADAPTERS[adapter].render(skill)), + }, + }, + ) + + +def plan_fixtures( + fixtures: Sequence[Fixture], adapter: str +) -> tuple[list[PlannedFixture], list[str]]: + """Plan every fixture; also return every problem found, not just the first.""" + planned: list[PlannedFixture] = [] + problems: list[str] = [] + for fixture in fixtures: + try: + planned.append(plan_fixture(fixture, adapter)) + except FixtureError as exc: + problems.append(str(exc)) + return planned, problems + + +def print_plan( + planned: Sequence[PlannedFixture], harness: Harness, repeat: int, max_runs: int +) -> None: + runs = len(planned) * repeat + print( + f"plan: {len(planned)} fixture(s) x {repeat} repeat(s) = {runs} run(s), " + f"sequential (concurrency 1), max {max_runs}" + ) + model = harness.model or "harness default" + print(f"harness: {harness.name} (adapter {harness.adapter}, model {model})") + print(f"command: {harness.command}") + width = max((len(p.fixture.name) for p in planned), default=0) + for p in planned: + skill = p.identity["skill"] + print( + f" {p.fixture.name:<{width}} {skill['name']} {skill['version']} " + f"fixture {p.identity['digest'][:19]} x{repeat}" + ) + + +# -- replay ------------------------------------------------------------------ + + +@dataclass(frozen=True) +class ReplaySpec: + """The configuration and identities a run record pins down.""" + + harness: Harness + harness_version: str | None + repeat: int + timeout: int + fixtures: tuple[FixtureIdentity, ...] + runner_sha256: str | None + #: sha256 of the record itself, stored in the new record's replay_of + record_sha256: str + + +def _get(where: str, mapping: object, key: str) -> object: + if not isinstance(mapping, dict) or key not in mapping: + raise ConfigError(f"{where}: missing {key!r}") + return mapping[key] + + +def _get_str(where: str, mapping: object, key: str) -> str: + value = _get(where, mapping, key) + if not isinstance(value, str) or not value: + raise ConfigError(f"{where}: {key!r} must be a non-empty string") + return value + + +def _get_optional_str(where: str, mapping: object, key: str) -> str | None: + value = _get(where, mapping, key) + if value is not None and not isinstance(value, str): + raise ConfigError(f"{where}: {key!r} must be a string or null") + return value + + +def _get_count(where: str, mapping: object, key: str) -> int: + value = _get(where, mapping, key) + if not isinstance(value, int) or isinstance(value, bool) or value < 1: + raise ConfigError(f"{where}: {key!r} must be a positive integer") + return value + + +def _get_digest(where: str, mapping: object, key: str) -> str: + value = _get_str(where, mapping, key) + if not _DIGEST_RE.fullmatch(value): + raise ConfigError(f"{where}: {key!r} is not a sha256 digest") + return value + + +def load_replay(path: Path) -> ReplaySpec: + """Read the configuration and identities to replay from a run record.""" + try: + text = path.read_text(encoding="utf-8") + data = json.loads(text) + except (OSError, UnicodeDecodeError, json.JSONDecodeError) as exc: + raise ConfigError(f"{path}: cannot read the run record: {exc}") from None + where = str(path) + if _get(where, data, "record_type") != RECORD_TYPE: + raise ConfigError(f"{where}: not a skilldeck eval run record") + version = _get(where, data, "schema_version") + if version != RECORD_SCHEMA_VERSION: + raise ConfigError( + f"{where}: unsupported schema_version {version!r} " + f"(this runner reads {RECORD_SCHEMA_VERSION})" + ) + harness_data = _get(where, data, "harness") + config = _get(where, data, "config") + raw_fixtures = _get(where, data, "fixtures") + if not isinstance(raw_fixtures, list) or not raw_fixtures: + raise ConfigError(f"{where}: 'fixtures' must be a non-empty list") + identities: list[FixtureIdentity] = [] + for i, entry in enumerate(raw_fixtures): + at = f"{where}: fixtures[{i}]" + skill = _get(at, entry, "skill") + identities.append( + { + "name": _get_str(at, entry, "name"), + "digest": _get_digest(at, entry, "digest"), + "skill": { + "name": _get_str(f"{at}.skill", skill, "name"), + "version": _get_str(f"{at}.skill", skill, "version"), + "canonical_sha256": _get_digest( + f"{at}.skill", skill, "canonical_sha256" + ), + "rendered_sha256": _get_digest( + f"{at}.skill", skill, "rendered_sha256" + ), + }, + } + ) + names = [identity["name"] for identity in identities] + if len(names) != len(set(names)): + raise ConfigError(f"{where}: a fixture is listed twice") + source = _get(where, data, "skilldeck") + return ReplaySpec( + harness=resolve_harness( + _get_str(f"{where}: harness", harness_data, "name"), + _get_str(f"{where}: harness", harness_data, "command"), + _get_str(where, data, "adapter"), + _get_optional_str(f"{where}: harness", harness_data, "model"), + ), + harness_version=_get_optional_str(f"{where}: harness", harness_data, "version"), + repeat=_get_count(f"{where}: config", config, "repeat"), + timeout=_get_count(f"{where}: config", config, "timeout_s"), + fixtures=tuple(identities), + runner_sha256=_get_optional_str(f"{where}: skilldeck", source, "runner_sha256"), + record_sha256=sha256_text(text), + ) + + +def replay_fixtures(spec: ReplaySpec) -> list[Fixture]: + """Load the fixtures a record names, by directory name, in record order.""" + available = {path.name: path for path in FIXTURES.iterdir() if path.is_dir()} + fixtures = [] + for identity in spec.fixtures: + path = available.get(identity["name"]) + if path is None: + raise ConfigError( + f"fixture {identity['name']!r} from the record no longer exists" + ) + fixtures.append(load_fixture(path)) + return fixtures + + +def replay_problems(recorded: FixtureIdentity, planned: PlannedFixture) -> list[str]: + """How ``planned`` differs from the identity a run record pinned.""" + current = planned.identity + name = recorded["name"] + + def changed(what: str, before: str, after: str) -> str: + return f"{name}: {what} changed since the record ({before} -> {after})" + + before, after = recorded["skill"], current["skill"] + pairs = ( + ("fixture content", recorded["digest"], current["digest"]), + ("skill", before["name"], after["name"]), + ("skill version", before["version"], after["version"]), + ("skill content", before["canonical_sha256"], after["canonical_sha256"]), + ("installed skill file", before["rendered_sha256"], after["rendered_sha256"]), + ) + return [changed(what, old, new) for what, old, new in pairs if old != new] + + +# -- running and recording --------------------------------------------------- + + +def _now() -> str: + stamp = datetime.now(timezone.utc).isoformat(timespec="milliseconds") + return stamp.replace("+00:00", "Z") + + +@dataclass +class RunEntry: + """One planned run in the record; what the run never reached stays null.""" + + attempt: int + #: passed | failed | agent_failed | timed_out | error | not_run + status: str + problems: list[str] + started_at: str | None = None + finished_at: str | None = None + duration_s: float | None = None + exit_code: int | None = None + finding_count: int | None = None + #: work-dir-relative POSIX paths of the raw report and stderr + artifacts: dict[str, str] | None = None + #: the raw report and stderr themselves, only with --include-reports + raw: dict[str, str] | None = None + + @property + def passed(self) -> bool: + return self.status == "passed" + + def to_record(self) -> dict[str, object]: + return { + **dataclasses.asdict(self), + "passed": self.passed, + "timed_out": self.status == "timed_out", + # no harness preset reports these yet; null means "not reported" + "usage": None, + "cost_usd": None, + } + + +def run_status(run: AgentRun, passed: bool) -> str: + if run.returncode is None: + return "timed_out" + if run.returncode != 0: + return "agent_failed" + return "passed" if passed else "failed" + + +def attempt_run( + planned: PlannedFixture, + harness: Harness, + attempt: int, + workdir: Path, + timeout: int, + include_reports: bool = False, +) -> tuple[RunEntry, AgentRun | None]: + """Build a fresh repo, run the agent in it, score it, and store its output. + + Raises AgentStartError if the agent command can't be started at all. + """ + fixture = planned.fixture + try: + repo = prepare_repo( + fixture, workdir / "repos" / f"run-{attempt}", harness.adapter + ) + except (OSError, subprocess.CalledProcessError) as exc: + problem = f"could not build the review repo: {exc}" + return RunEntry(attempt, "error", [problem]), None + started_at, clock = _now(), time.monotonic() + run = run_agent(harness.command, planned.prompt, repo, timeout, harness.model) + duration = round(time.monotonic() - clock, 3) + finished_at = _now() + + # raw output lives beside the record, never in it (unless asked for) + out = workdir / "artifacts" / f"run-{attempt}" / fixture.name + out.mkdir(parents=True, exist_ok=True) + (out / "report.txt").write_text(run.stdout, encoding="utf-8") + (out / "stderr.txt").write_text(run.stderr, encoding="utf-8") + problems, passed = evaluate(fixture, run) + entry = RunEntry( + attempt=attempt, + status=run_status(run, passed), + problems=problems, + started_at=started_at, + finished_at=finished_at, + duration_s=duration, + exit_code=run.returncode, + finding_count=len(parse_findings(run.stdout)), + artifacts={ + name: (out / f"{name}.txt").relative_to(workdir).as_posix() + for name in ("report", "stderr") + }, + raw={"report": run.stdout, "stderr": run.stderr} if include_reports else None, + ) + return entry, run + + +def build_record( + *, + source: dict[str, object], + harness: Harness, + version: str | None, + planned: Sequence[PlannedFixture], + entries: dict[str, list[RunEntry]], + config: dict[str, object], + started_at: str, + stopped: bool, +) -> dict[str, object]: + """The run record: provider-neutral, and free of raw agent output.""" + runs = [entry for p in planned for entry in entries[p.fixture.name]] + not_run = sum(entry.status == "not_run" for entry in runs) + passed = sum(entry.passed for entry in runs) + return { + "schema_version": RECORD_SCHEMA_VERSION, + "record_type": RECORD_TYPE, + "status": "incomplete" if stopped else "complete", + "started_at": started_at, + "finished_at": _now(), + "skilldeck": source, + "environment": {"python": platform.python_version(), "platform": sys.platform}, + "harness": { + "name": harness.name, + "command": harness.command, + "model": harness.model, + "version": version, + "version_command": harness.version_command, + }, + "adapter": harness.adapter, + "config": config, + "fixtures": [ + { + **p.identity, + "prompt": p.prompt, + "plant_count": len(p.fixture.plants), + "max_findings": p.fixture.max_findings, + "runs": [entry.to_record() for entry in entries[p.fixture.name]], + } + for p in planned + ], + "summary": { + "planned": len(runs), + "attempted": len(runs) - not_run, + "passed": passed, + "failed": len(runs) - not_run - passed, + "not_run": not_run, + }, + } + + +def write_record(path: Path, record: dict[str, object]) -> None: + # sorted keys, LF newlines: the same record is the same bytes everywhere + path.write_text(canonical_json(record), encoding="utf-8", newline="\n") + + def _positive_int(value: str) -> int: number = int(value) if number < 1: @@ -632,7 +1310,7 @@ def _positive_int(value: str) -> int: return number -def main(argv: Sequence[str] | None = None) -> int: +def _parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter ) @@ -640,80 +1318,222 @@ def main(argv: Sequence[str] | None = None) -> int: "--skill", help="run only this skill's fixtures, or one fixture by directory name", ) + parser.add_argument( + "--harness", + choices=[*sorted(HARNESSES), CUSTOM_HARNESS], + help=f"agent CLI preset (default: {DEFAULT_HARNESS}, or {CUSTOM_HARNESS} " + "with --agent-cmd); sets the command and the matching adapter", + ) parser.add_argument( "--agent-cmd", - default=DEFAULT_AGENT_CMD, - help="agent command; {prompt} is substituted (default: %(default)r)", + help="agent command; {prompt} (and {model}) are substituted " + f"(default: the harness's, {DEFAULT_AGENT_CMD!r} for claude)", ) parser.add_argument( "--adapter", - default=DEFAULT_ADAPTER, choices=sorted(ADAPTERS), - help="skilldeck adapter that installs the skill (default: %(default)s)", + help="skilldeck adapter that installs the skill (default: the " + f"harness's; {DEFAULT_ADAPTER} for a custom harness)", + ) + parser.add_argument( + "--model", + help="model to request, passed through the command's {model}; recorded " + "in the run record (default: the harness's own default, unrecorded)", ) parser.add_argument( "--repeat", type=_positive_int, - default=1, metavar="N", - help="run each fixture N times and report its pass rate", + help="run each fixture N times and report its pass rate (default: 1)", ) parser.add_argument( - "--timeout", type=int, default=600, help="per-run agent timeout (s)" + "--timeout", + type=_positive_int, + metavar="S", + help=f"per-run agent timeout in seconds (default: {DEFAULT_TIMEOUT})", + ) + parser.add_argument( + "--max-runs", + type=_positive_int, + default=DEFAULT_MAX_RUNS, + metavar="N", + help="refuse to start if more runs than this are planned " + "(default: %(default)s)", + ) + parser.add_argument( + "--dry-run", + action="store_true", + help="validate the fixtures and print the planned runs; invoke no agent", + ) + parser.add_argument( + "--replay", + type=Path, + metavar="RECORD", + help="re-run a run record's configuration, refusing if a skill or " + "fixture changed since", + ) + parser.add_argument( + "--include-reports", + action="store_true", + help="also copy each run's raw stdout and stderr into the run record", ) parser.add_argument( "--keep", action="store_true", help="keep the temp repos for inspection" ) - args = parser.parse_args(argv) + return parser + +def main(argv: Sequence[str] | None = None) -> int: + args = _parser().parse_args(argv) + + replay: ReplaySpec | None = None try: - fixtures = select_fixtures(args.skill) - # fail fast, before any paid run, on a skill the adapter can't install - prompts = {f.name: build_prompt(f, args.adapter) for f in fixtures} - except FixtureError as exc: + if args.replay is not None: + fixed = { + "--skill": args.skill, + "--harness": args.harness, + "--agent-cmd": args.agent_cmd, + "--adapter": args.adapter, + "--model": args.model, + "--repeat": args.repeat, + "--timeout": args.timeout, + } + clashes = [flag for flag, value in fixed.items() if value is not None] + if clashes: + raise ConfigError( + "--replay takes its configuration from the record; drop " + + ", ".join(clashes) + ) + replay = load_replay(args.replay) + harness, repeat, timeout = replay.harness, replay.repeat, replay.timeout + fixtures = replay_fixtures(replay) + else: + harness = resolve_harness( + args.harness, args.agent_cmd, args.adapter, args.model + ) + repeat = args.repeat or 1 + timeout = args.timeout or DEFAULT_TIMEOUT + fixtures = select_fixtures(args.skill) + if not fixtures: + raise ConfigError(f"no fixture for {args.skill!r}") + except (ConfigError, FixtureError) as exc: print(f"error: {exc}", file=sys.stderr) return 2 - if not fixtures: - print(f"error: no fixture for {args.skill!r}", file=sys.stderr) + + # fail fast, before any paid run, on a broken fixture or a skill the + # adapter can't install -- or, replaying, on anything that changed + planned, problems = plan_fixtures(fixtures, harness.adapter) + if replay is not None and not problems: + for recorded, current in zip(replay.fixtures, planned, strict=True): + problems.extend(replay_problems(recorded, current)) + if problems: + for problem in problems: + print(f"error: {problem}", file=sys.stderr) + return 2 + + print_plan(planned, harness, repeat, args.max_runs) + total = len(planned) * repeat + if total > args.max_runs: + print( + f"error: {total} planned runs exceed --max-runs {args.max_runs}; " + "narrow --skill, lower --repeat, or raise --max-runs", + file=sys.stderr, + ) return 2 + if args.dry_run: + print("\ndry run: fixtures are valid; no agent was invoked") + return 0 + + source = source_identity() + version = harness_version(harness.version_command) + if replay is not None: + if version != replay.harness_version: + print( + f"note: harness version {version!r} differs from the record's " + f"{replay.harness_version!r}" + ) + if source["runner_sha256"] != replay.runner_sha256: + print("note: the eval runner (scorer or prompt) changed since the record") workdir = Path(tempfile.mkdtemp(prefix="skilldeck-evals-")) print(f"work dir: {workdir}\n") + started_at = _now() + entries: dict[str, list[RunEntry]] = {} pass_counts: dict[str, int] = {} - for fixture in fixtures: + stop: str | None = None # why the remaining runs were not attempted + status = 0 + for p in planned: + fixture = p.fixture + entries[fixture.name] = [] passes = 0 - for attempt in range(1, args.repeat + 1): - run_dir = workdir if args.repeat == 1 else workdir / f"run-{attempt}" - repo = prepare_repo(fixture, run_dir, args.adapter) - run = run_agent(args.agent_cmd, prompts[fixture.name], repo, args.timeout) - (repo / "report.txt").write_text(run.stdout, encoding="utf-8") - (repo / "stderr.txt").write_text(run.stderr, encoding="utf-8") - problems, passed = evaluate(fixture, run) - label = fixture.name if args.repeat == 1 else f"{fixture.name} #{attempt}" - findings = len(parse_findings(run.stdout)) - print(f"{'PASS' if passed else 'FAIL'} {label} ({findings} findings)") - for problem in problems: + for attempt in range(1, repeat + 1): + if stop is not None: + entry = RunEntry(attempt, "not_run", [f"not run: {stop}"]) + entries[fixture.name].append(entry) + continue + label = fixture.name if repeat == 1 else f"{fixture.name} #{attempt}" + run: AgentRun | None = None + try: + entry, run = attempt_run( + p, harness, attempt, workdir, timeout, args.include_reports + ) + except AgentStartError as exc: + stop, status = str(exc), 2 + entry = RunEntry(attempt, "error", [stop]) + except KeyboardInterrupt: + stop, status = "interrupted", 130 + entry = RunEntry(attempt, "error", [stop]) + entries[fixture.name].append(entry) + count = entry.finding_count + findings = "" if count is None else f" ({count} findings)" + print(f"{'PASS' if entry.passed else 'FAIL'} {label}{findings}") + for problem in entry.problems: print(f" {problem}") - if not passed and run.stderr.strip(): + if not entry.passed and run is not None and run.stderr.strip(): print(" agent stderr:") print(textwrap.indent(run.stderr.rstrip(), " " * 8)) - passes += passed + passes += entry.passed pass_counts[fixture.name] = passes - runs = len(fixtures) * args.repeat + config: dict[str, object] = { + "repeat": repeat, + "timeout_s": timeout, + "max_runs": args.max_runs, + "jobs": 1, + "include_reports": args.include_reports, + "replay_of": None if replay is None else replay.record_sha256, + } + record = build_record( + source=source, + harness=harness, + version=version, + planned=planned, + entries=entries, + config=config, + started_at=started_at, + stopped=stop is not None, + ) + record_path = workdir / RECORD_NAME + write_record(record_path, record) + + runs = len(planned) * repeat failed = runs - sum(pass_counts.values()) - if args.repeat > 1: + if repeat > 1: print("\npass rate per fixture:") for name, passes in pass_counts.items(): - print(f" {passes}/{args.repeat} ({passes / args.repeat:4.0%}) {name}") + print(f" {passes}/{repeat} ({passes / repeat:4.0%}) {name}") print(f"\n{runs - failed}/{runs} runs passed") else: print(f"\n{runs - failed}/{runs} fixtures passed") + if stop is not None: + print(f"stopped early: {stop}", file=sys.stderr) + print(f"run record: {record_path}") + print(f"reports and stderr: {workdir / 'artifacts'}") if args.keep or failed: - print(f"reports kept in {workdir}") + print(f"review repos kept in {workdir / 'repos'}") else: - remove_tree(workdir) - return 1 if failed else 0 + remove_tree(workdir / "repos") + return status or (1 if failed else 0) if __name__ == "__main__": diff --git a/tests/test_eval_runs.py b/tests/test_eval_runs.py new file mode 100644 index 0000000..8bad748 --- /dev/null +++ b/tests/test_eval_runs.py @@ -0,0 +1,776 @@ +"""Tests for eval run records, harness presets, budgets, dry runs and replay. + +Everything here uses stand-in agents (a Python script in the harness's place) +-- no agent CLI, credentials or paid API calls. +""" + +import itertools +import json +import re +import shlex +import shutil +import subprocess +import sys +import textwrap +from pathlib import Path + +import pytest +import run_evals # loaded from evals/run_evals.py by conftest.py + +from skilldeck import __version__ +from skilldeck.provenance import canonical_json, sha256_text + +ROOT = Path(__file__).resolve().parent.parent +SCHEMA = json.loads( + (ROOT / "evals" / "run-record.schema.json").read_text(encoding="utf-8") +) +CONTENT_MANIFEST = json.loads( + (ROOT / "src" / "skilldeck" / "_content_manifest.json").read_text(encoding="utf-8") +) + +# a report that passes the logging fixture +PASSING_AGENT = """\ + import sys + print("Reviewed main..HEAD (1 file): 1 finding, high.") + print("- **[high] Log injection** — `auth/session.py:15`") + print(" **Issue:** a CR/LF in the username forges log lines.") + print(" **Fix:** escape control characters.") +""" + + +# -- a minimal JSON Schema validator ------------------------------------------- +# +# Just the draft 2020-12 keywords evals/run-record.schema.json uses, so the +# schema is checked without a runtime or dev dependency. An unsupported +# keyword fails loudly rather than being silently ignored. + +_KEYWORDS = frozenset( + { + "$schema", + "$id", + "$defs", + "$ref", + "title", + "description", + "type", + "const", + "enum", + "anyOf", + "properties", + "required", + "additionalProperties", + "items", + "pattern", + "minimum", + } +) + + +def _is_type(value, kind): + number = isinstance(value, (int, float)) and not isinstance(value, bool) + return { + "object": isinstance(value, dict), + "array": isinstance(value, list), + "string": isinstance(value, str), + "boolean": isinstance(value, bool), + "null": value is None, + "integer": number and isinstance(value, int), + "number": number, + }[kind] + + +def _same(a, b): + # JSON equality: 1 is not true + return type(a) is type(b) and a == b + + +def schema_errors(value, schema=SCHEMA, where="$"): + unknown = set(schema) - _KEYWORDS + assert not unknown, f"validator does not support {sorted(unknown)}" + errors = [] + if "$ref" in schema: + prefix = "#/$defs/" + assert schema["$ref"].startswith(prefix) + target = SCHEMA["$defs"][schema["$ref"].removeprefix(prefix)] + errors += schema_errors(value, target, where) + if "type" in schema: + kinds = schema["type"] if isinstance(schema["type"], list) else [schema["type"]] + if not any(_is_type(value, kind) for kind in kinds): + return [*errors, f"{where}: {value!r} is not of type {kinds}"] + if "const" in schema and not _same(value, schema["const"]): + errors.append(f"{where}: {value!r} is not {schema['const']!r}") + if "enum" in schema and not any(_same(value, v) for v in schema["enum"]): + errors.append(f"{where}: {value!r} is not one of {schema['enum']}") + if "anyOf" in schema and all( + schema_errors(value, branch, where) for branch in schema["anyOf"] + ): + errors.append(f"{where}: {value!r} matches no anyOf branch") + if isinstance(value, dict): + properties = schema.get("properties", {}) + errors += [ + f"{where}: missing {key!r}" + for key in schema.get("required", ()) + if key not in value + ] + for key, item in value.items(): + if key in properties: + errors += schema_errors(item, properties[key], f"{where}.{key}") + elif schema.get("additionalProperties", True) is False: + errors.append(f"{where}: unexpected {key!r}") + if isinstance(value, list) and "items" in schema: + for i, item in enumerate(value): + errors += schema_errors(item, schema["items"], f"{where}[{i}]") + if ( + isinstance(value, str) + and "pattern" in schema + and not re.search(schema["pattern"], value) + ): + errors.append(f"{where}: {value!r} does not match {schema['pattern']!r}") + if _is_type(value, "number") and "minimum" in schema and value < schema["minimum"]: + errors.append(f"{where}: {value!r} is below {schema['minimum']}") + return errors + + +# -- helpers ------------------------------------------------------------------- + + +def _stand_in_agent(tmp_path, body, name="agent.py"): + script = tmp_path / name + script.write_text(textwrap.dedent(body), encoding="utf-8") + return f"{shlex.quote(sys.executable)} {shlex.quote(str(script))} {{prompt}}" + + +@pytest.fixture +def run_main(monkeypatch, tmp_path): + """Return ``run(*argv) -> (status, workdir)``, a fresh work dir per call.""" + counter = itertools.count(1) + + def mkdtemp(**_): + workdir = tmp_path / f"work-{next(counter)}" + workdir.mkdir() + return str(workdir) + + monkeypatch.setattr(run_evals.tempfile, "mkdtemp", mkdtemp) + + def run(*argv): + before = set(tmp_path.glob("work-*")) + status = run_evals.main(list(argv)) + (workdir,) = set(tmp_path.glob("work-*")) - before or {None} + return status, workdir + + return run + + +def _record(workdir): + text = (workdir / run_evals.RECORD_NAME).read_text(encoding="utf-8") + return text, json.loads(text) + + +@pytest.fixture +def no_subprocess(monkeypatch): + """Fail the test if anything -- an agent, git, a version probe -- runs.""" + + def forbidden(cmd, **_): + raise AssertionError(f"unexpected subprocess: {cmd}") + + monkeypatch.setattr(run_evals.subprocess, "run", forbidden) + monkeypatch.setattr( + run_evals.tempfile, + "mkdtemp", + lambda **_: pytest.fail("a dry run must not create a work dir"), + ) + + +# -- the run record ------------------------------------------------------------ + + +def test_record_captures_every_field_in_a_deterministic_order(run_main, tmp_path): + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + status, workdir = run_main( + "--skill", "logging", "--agent-cmd", agent, "--repeat", "2" + ) + assert status == 0 + text, record = _record(workdir) + assert schema_errors(record) == [] + # sorted keys, two-space indent, one final newline -- byte-stable + assert text == canonical_json(record) + assert (workdir / run_evals.RECORD_NAME).read_bytes().count(b"\r") == 0 + + assert record["schema_version"] == 1 + assert record["status"] == "complete" + assert record["skilldeck"]["version"] == __version__ + assert record["skilldeck"]["runner_sha256"] == run_evals.runner_digest() + assert record["harness"] == { + "name": "custom", + "command": agent, + "model": None, + "version": None, + "version_command": None, + } + assert record["adapter"] == "claude" + assert record["config"] == { + "repeat": 2, + "timeout_s": 600, + "max_runs": 50, + "jobs": 1, + "include_reports": False, + "replay_of": None, + } + + (fixture,) = record["fixtures"] + (manifest,) = [s for s in CONTENT_MANIFEST["skills"] if s["name"] == "logging"] + assert fixture["name"] == "logging" + assert fixture["digest"] == run_evals.fixture_digest(run_evals.FIXTURES / "logging") + assert fixture["skill"] == { + "name": "logging", + "version": manifest["version"], + "canonical_sha256": manifest["canonical_sha256"], + "rendered_sha256": manifest["claude_rendered_sha256"], + } + assert ".claude/skills/logging/SKILL.md" in fixture["prompt"] + assert [run["attempt"] for run in fixture["runs"]] == [1, 2] + for run in fixture["runs"]: + assert run["status"] == "passed" and run["passed"] is True + assert run["problems"] == [] + assert run["exit_code"] == 0 and run["timed_out"] is False + assert run["finding_count"] == 1 + assert run["duration_s"] >= 0 + assert run["usage"] is None and run["cost_usd"] is None + assert run["raw"] is None + # raw output stays beside the record, referenced by a relative path + report = workdir / run["artifacts"]["report"] + assert "Log injection" in report.read_text(encoding="utf-8") + assert (workdir / run["artifacts"]["stderr"]).is_file() + assert "\\" not in run["artifacts"]["report"] + assert "Log injection" not in text + assert record["summary"] == { + "planned": 2, + "attempted": 2, + "passed": 2, + "failed": 0, + "not_run": 0, + } + # every run passed: the repos go, the record and raw output stay + assert not (workdir / "repos").exists() + + +def test_include_reports_embeds_raw_output(run_main, tmp_path): + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + status, workdir = run_main( + "--skill", "logging", "--agent-cmd", agent, "--include-reports" + ) + assert status == 0 + _, record = _record(workdir) + assert schema_errors(record) == [] + assert record["config"]["include_reports"] is True + (run,) = record["fixtures"][0]["runs"] + assert "Log injection" in run["raw"]["report"] + assert run["raw"]["stderr"] == "" + + +def test_a_failing_agent_is_recorded_with_its_exit_code(run_main, tmp_path, capsys): + agent = _stand_in_agent( + tmp_path, + """\ + import sys + print("upstream overloaded", file=sys.stderr) + sys.exit(3) + """, + ) + status, workdir = run_main("--skill", "logging", "--agent-cmd", agent) + assert status == 1 + assert "upstream overloaded" in capsys.readouterr().out + text, record = _record(workdir) + assert schema_errors(record) == [] + (run,) = record["fixtures"][0]["runs"] + assert run["status"] == "agent_failed" and run["passed"] is False + assert run["exit_code"] == 3 + assert run["problems"] == ["agent exited with status 3"] + assert "upstream overloaded" not in text # stderr stays in its artifact + stderr = workdir / run["artifacts"]["stderr"] + assert "upstream overloaded" in stderr.read_text(encoding="utf-8") + assert (workdir / "repos").is_dir() # kept for inspection on failure + assert record["summary"]["failed"] == 1 + + +def test_a_timeout_is_recorded(run_main, monkeypatch, tmp_path): + def timed_out(agent_cmd, prompt, repo, timeout, model=None): + return run_evals.AgentRun("- **[high] partial", "", None, timeout) + + monkeypatch.setattr(run_evals, "run_agent", timed_out) + status, workdir = run_main( + "--skill", "logging", "--agent-cmd", "agent {prompt}", "--timeout", "7" + ) + assert status == 1 + _, record = _record(workdir) + assert schema_errors(record) == [] + (run,) = record["fixtures"][0]["runs"] + assert run["status"] == "timed_out" and run["timed_out"] is True + assert run["exit_code"] is None + assert run["problems"] == ["agent timed out after 7s"] + assert record["config"]["timeout_s"] == 7 + + +def test_a_missing_agent_command_is_recorded_and_stops_the_rest(run_main, capsys): + status, workdir = run_main( + "--skill", + "authentication-review", + "--agent-cmd", + "no-such-agent-skilldeck-test {prompt}", + "--repeat", + "2", + ) + assert status == 2 + assert "stopped early: agent command not found" in capsys.readouterr().err + _, record = _record(workdir) + assert schema_errors(record) == [] + assert record["status"] == "incomplete" + runs = [run for f in record["fixtures"] for run in f["runs"]] + assert [run["status"] for run in runs] == ["error", *["not_run"] * 3] + assert runs[0]["problems"][0].startswith("agent command not found") + assert all(run["problems"][0].startswith("not run: ") for run in runs[1:]) + assert record["summary"] == { + "planned": 4, + "attempted": 1, + "passed": 0, + "failed": 1, + "not_run": 3, + } + + +def test_an_interrupt_still_writes_the_record(run_main, monkeypatch): + def interrupted(*_args, **_kwargs): + raise KeyboardInterrupt + + monkeypatch.setattr(run_evals, "attempt_run", interrupted) + status, workdir = run_main("--skill", "logging", "--agent-cmd", "a {prompt}") + assert status == 130 + _, record = _record(workdir) + assert record["status"] == "incomplete" + assert record["fixtures"][0]["runs"][0]["problems"] == ["interrupted"] + + +def test_a_repo_that_cannot_be_built_is_recorded(run_main, monkeypatch): + def broken(*_args): + raise subprocess.CalledProcessError(128, ["git", "init"]) + + monkeypatch.setattr(run_evals, "prepare_repo", broken) + status, workdir = run_main("--skill", "logging", "--agent-cmd", "a {prompt}") + assert status == 1 + _, record = _record(workdir) + (run,) = record["fixtures"][0]["runs"] + assert run["status"] == "error" + assert run["problems"][0].startswith("could not build the review repo") + + +def test_the_schema_rejects_a_malformed_record(run_main, tmp_path): + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + _, workdir = run_main("--skill", "logging", "--agent-cmd", agent) + _, record = _record(workdir) + run = record["fixtures"][0]["runs"][0] + run["report_text"] = "raw" + del run["exit_code"] + run["status"] = "maybe" + record["config"]["jobs"] = True + assert sorted(schema_errors(record)) == sorted( + [ + "$.config.jobs: True is not 1", + "$.fixtures[0].runs[0]: missing 'exit_code'", + "$.fixtures[0].runs[0]: unexpected 'report_text'", + "$.fixtures[0].runs[0].status: 'maybe' is not one of " + "['passed', 'failed', 'agent_failed', 'timed_out', 'error', 'not_run']", + ] + ) + + +def _git(*args): + result = subprocess.run( + ["git", *args], + cwd=ROOT, + capture_output=True, + text=True, + encoding="utf-8", + check=False, + ) + return result.stdout.strip() if result.returncode == 0 else None + + +def test_source_identity_names_this_checkout(): + source = run_evals.source_identity() + assert source["version"] == __version__ + toplevel = _git("rev-parse", "--show-toplevel") + if toplevel is None or Path(toplevel).resolve() != ROOT.resolve(): + # not a checkout of its own (an unpacked sdist, say) + assert source["git_commit"] is None + else: + assert source["git_commit"] == _git("rev-parse", "HEAD") + assert isinstance(source["git_dirty"], bool) + + +# -- fixture digests ----------------------------------------------------------- + + +def _copy_fixture(tmp_path, name="logging"): + fixtures = tmp_path / "fixtures" + shutil.copytree(run_evals.FIXTURES / name, fixtures / name) + return fixtures + + +def test_fixture_digest_covers_content_and_paths_but_not_line_endings(tmp_path): + path = _copy_fixture(tmp_path) / "logging" + original = run_evals.fixture_digest(path) + assert original == run_evals.fixture_digest(run_evals.FIXTURES / "logging") + + session = path / "change" / "auth" / "session.py" + text = session.read_text(encoding="utf-8") + session.write_bytes(text.replace("\n", "\r\n").encode("utf-8")) + assert run_evals.fixture_digest(path) == original # a Windows checkout + + session.write_text(text + "# changed\n", encoding="utf-8") + assert run_evals.fixture_digest(path) != original + session.write_text(text, encoding="utf-8") + session.rename(session.with_name("renamed.py")) + assert run_evals.fixture_digest(path) != original + + +# -- harness presets ----------------------------------------------------------- + + +def test_the_default_harness_is_claude(): + harness = run_evals.resolve_harness() + assert harness == run_evals.Harness("claude", "claude -p {prompt}", "claude") + assert harness.version_command == ["claude", "--version"] + assert run_evals.agent_argv(harness.command, "P") == ["claude", "-p", "P"] + + +@pytest.mark.parametrize( + ("name", "model", "adapter", "argv"), + [ + ("claude", None, "claude", ["claude", "-p", "P"]), + ("claude", "opus", "claude", ["claude", "--model", "opus", "-p", "P"]), + ("codex", None, "codex", ["codex", "exec", "P"]), + ("codex", "gpt-5", "codex", ["codex", "exec", "--model", "gpt-5", "P"]), + ], +) +def test_harness_presets_pair_a_command_with_their_adapter(name, model, adapter, argv): + harness = run_evals.resolve_harness(name, model=model) + assert harness.name == name + assert harness.adapter == adapter + assert harness.model == model + assert run_evals.agent_argv(harness.command, "P", harness.model) == argv + assert harness.version_command == [argv[0], "--version"] + + +def test_agent_cmd_and_adapter_override_a_preset(): + custom = run_evals.resolve_harness(agent_cmd="my-agent {prompt}") + assert (custom.name, custom.adapter) == ("custom", "claude") + assert custom.version_command is None + + codex = run_evals.resolve_harness("codex", "codex exec --json {prompt}") + assert (codex.name, codex.adapter) == ("codex", "codex") + assert codex.command == "codex exec --json {prompt}" + + kiro = run_evals.resolve_harness("custom", "kiro-cli {prompt}", "kiro") + assert kiro.adapter == "kiro" + + +@pytest.mark.parametrize( + ("kwargs", "message"), + [ + ({"name": "custom"}, "--harness custom needs --agent-cmd"), + ({"agent_cmd": "agent"}, "has no {prompt} placeholder"), + ({"agent_cmd": "agent {prompt}", "model": "m"}, "needs a {model} placeholder"), + ({"agent_cmd": "agent -m {model} {prompt}"}, "pass --model"), + ({"agent_cmd": "agent '{prompt}"}, "No closing quotation"), + ({"name": "claude", "model": " "}, "--model must not be empty"), + ({"name": "gemini"}, "unknown harness 'gemini'"), + ], +) +def test_invalid_harness_options_are_config_errors(kwargs, message): + with pytest.raises(run_evals.ConfigError, match=re.escape(message)): + run_evals.resolve_harness(**kwargs) + + +def test_harness_version_is_best_effort(): + version = run_evals.harness_version([sys.executable, "--version"]) + assert version is not None and version.startswith("Python ") + assert run_evals.harness_version(["no-such-agent-skilldeck-test", "-V"]) is None + failing = [sys.executable, "-c", "import sys; print('x'); sys.exit(1)"] + assert run_evals.harness_version(failing) is None + assert run_evals.harness_version(None) is None + + +def test_a_preset_harness_records_its_version_and_model(run_main, tmp_path, capsys): + # --harness codex with a stand-in in its place: the codex adapter installs + # the skill, the model reaches the command, and the version probe runs + # the command's own executable + agent = _stand_in_agent( + tmp_path, + """\ + import sys + model, prompt = sys.argv[1:] + assert model == "tiny-model", model + assert ".agents/skills/logging/SKILL.md" in prompt + print("Reviewed main..HEAD: no findings.") + """, + ) + agent = agent.replace("{prompt}", "{model} {prompt}") + status, workdir = run_main( + "--skill", + "logging", + "--harness", + "codex", + "--agent-cmd", + agent, + "--model", + "tiny-model", + ) + out = capsys.readouterr().out + assert status == 1, out # the empty review misses the plant + assert "harness: codex (adapter codex, model tiny-model)" in out + _, record = _record(workdir) + assert schema_errors(record) == [] + assert record["adapter"] == "codex" + assert record["harness"]["name"] == "codex" + assert record["harness"]["model"] == "tiny-model" + assert record["harness"]["version_command"] == [sys.executable, "--version"] + assert record["harness"]["version"].startswith("Python ") + (run,) = record["fixtures"][0]["runs"] + assert run["status"] == "failed" + assert ".agents/skills/logging/SKILL.md" in record["fixtures"][0]["prompt"] + + +# -- dry runs and budgets ------------------------------------------------------ + + +def test_dry_run_validates_and_plans_without_running_anything( + no_subprocess, tmp_path, capsys +): + marker = tmp_path / "agent-ran" + agent = _stand_in_agent(tmp_path, f"open({str(marker)!r}, 'w').close()\n") + status = run_evals.main(["--agent-cmd", agent, "--dry-run", "--repeat", "2"]) + out = capsys.readouterr().out + assert status == 0, out + fixtures = [p for p in run_evals.FIXTURES.iterdir() if p.is_dir()] + runs = 2 * len(fixtures) + assert ( + f"plan: {len(fixtures)} fixture(s) x 2 repeat(s) = {runs} run(s), " + "sequential (concurrency 1), max 50" + ) in out + for path in fixtures: + assert f" {path.name} " in out + assert "dry run: fixtures are valid; no agent was invoked" in out + assert not marker.exists() + + +def _broken_fixtures(tmp_path): + fixtures = _copy_fixture(tmp_path) + shutil.rmtree(fixtures / "logging" / "change") + return fixtures + + +def test_dry_run_fails_on_an_invalid_fixture( + no_subprocess, monkeypatch, tmp_path, capsys +): + monkeypatch.setattr(run_evals, "FIXTURES", _broken_fixtures(tmp_path)) + assert run_evals.main(["--dry-run"]) == 2 + err = capsys.readouterr().err + assert "error: logging: missing change/ directory" in err + assert "plant file auth/session.py is not in change/" in err + + bad = tmp_path / "fixtures" / "logging" / "expected.yaml" + bad.write_text("skill: logging\nplants: []\n", encoding="utf-8") + assert run_evals.main(["--dry-run"]) == 2 + assert "missing required key(s) ['max-findings']" in capsys.readouterr().err + + +def test_dry_run_fails_on_a_skill_the_adapter_cannot_install( + no_subprocess, monkeypatch, tmp_path, capsys +): + fixtures = _copy_fixture(tmp_path) + (fixtures / "logging" / "expected.yaml").write_text( + "skill: no-such-skill\nplants: []\nmax-findings: 0\n", encoding="utf-8" + ) + monkeypatch.setattr(run_evals, "FIXTURES", fixtures) + assert run_evals.main(["--dry-run"]) == 2 + assert "no bundled skill named 'no-such-skill'" in capsys.readouterr().err + + +def test_max_runs_refuses_before_anything_runs(no_subprocess, capsys): + argv = ["--skill", "authentication-review", "--repeat", "3", "--max-runs", "5"] + assert run_evals.main(argv) == 2 + captured = capsys.readouterr() + assert "6 planned runs exceed --max-runs 5" in captured.err + assert "= 6 run(s)" in captured.out + # a dry run reports the same refusal + assert run_evals.main([*argv, "--dry-run"]) == 2 + assert "exceed --max-runs 5" in capsys.readouterr().err + + +def test_the_default_budget_caps_a_full_repeated_run(no_subprocess, capsys): + fixtures = [p for p in run_evals.FIXTURES.iterdir() if p.is_dir()] + repeat = run_evals.DEFAULT_MAX_RUNS // len(fixtures) + 1 + assert run_evals.main(["--repeat", str(repeat)]) == 2 + assert "exceed --max-runs 50" in capsys.readouterr().err + + +def test_max_runs_allows_a_plan_within_budget(run_main, tmp_path): + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + status, workdir = run_main( + "--skill", + "logging", + "--agent-cmd", + agent, + "--repeat", + "2", + "--max-runs", + "2", + ) + assert status == 0 + _, record = _record(workdir) + assert record["config"]["max_runs"] == 2 + + +# -- replay -------------------------------------------------------------------- + + +def _first_record(run_main, tmp_path, agent_body=PASSING_AGENT, *extra): + agent = _stand_in_agent(tmp_path, agent_body) + status, workdir = run_main("--skill", "logging", "--agent-cmd", agent, *extra) + assert status in (0, 1) + return workdir / run_evals.RECORD_NAME + + +def test_replay_reruns_the_recorded_configuration(run_main, tmp_path): + path = _first_record(run_main, tmp_path, PASSING_AGENT, "--repeat", "2") + original = json.loads(path.read_text(encoding="utf-8")) + status, workdir = run_main("--replay", str(path)) + assert status == 0 + _, replayed = _record(workdir) + assert schema_errors(replayed) == [] + assert replayed["config"]["replay_of"] == sha256_text( + path.read_text(encoding="utf-8") + ) + for key in ("harness", "adapter"): + assert replayed[key] == original[key] + assert replayed["config"]["repeat"] == 2 + identity = ("name", "digest", "skill", "prompt") + assert [{k: f[k] for k in identity} for f in replayed["fixtures"]] == [ + {k: f[k] for k in identity} for f in original["fixtures"] + ] + assert replayed["summary"]["passed"] == 2 + + +def test_replay_refuses_a_changed_fixture(run_main, monkeypatch, tmp_path, capsys): + fixtures = _copy_fixture(tmp_path) + monkeypatch.setattr(run_evals, "FIXTURES", fixtures) + path = _first_record(run_main, tmp_path) + session = fixtures / "logging" / "change" / "auth" / "session.py" + session.write_text( + session.read_text(encoding="utf-8") + "# edited\n", encoding="utf-8" + ) + marker = tmp_path / "agent-ran" + record = json.loads(path.read_text(encoding="utf-8")) + record["harness"]["command"] = _stand_in_agent( + tmp_path, f"open({str(marker)!r}, 'w').close()\n", "marker.py" + ) + path.write_text(json.dumps(record), encoding="utf-8") + + status, workdir = run_main("--replay", str(path)) + assert status == 2 + assert workdir is None # refused before a work dir, let alone an agent + assert not marker.exists() + assert "logging: fixture content changed since the record" in ( + capsys.readouterr().err + ) + + +def _minimal_record(tmp_path, **skill_changes): + """A hand-written record of the logging fixture's current identity.""" + fixture = run_evals.load_fixture(run_evals.FIXTURES / "logging") + identity = run_evals.plan_fixture(fixture, "claude").identity + identity["skill"].update(skill_changes) + record = { + "schema_version": 1, + "record_type": "skilldeck-eval-run", + "skilldeck": {"runner_sha256": None}, + "harness": { + "name": "claude", + "command": "claude -p {prompt}", + "model": None, + "version": None, + }, + "adapter": "claude", + "config": {"repeat": 1, "timeout_s": 60}, + "fixtures": [identity], + } + path = tmp_path / "record.json" + path.write_text(json.dumps(record), encoding="utf-8") + return path + + +@pytest.mark.parametrize( + ("changes", "message"), + [ + ( + {"canonical_sha256": "sha256:" + "0" * 64}, + "logging: skill content changed since the record", + ), + ( + {"rendered_sha256": "sha256:" + "0" * 64}, + "logging: installed skill file changed since the record", + ), + ({"version": "9.9.9"}, "logging: skill version changed since the record"), + ], +) +def test_replay_refuses_a_changed_skill( + no_subprocess, tmp_path, capsys, changes, message +): + path = _minimal_record(tmp_path, **changes) + assert run_evals.main(["--replay", str(path)]) == 2 + assert message in capsys.readouterr().err + + +def test_replay_dry_run_verifies_digests_and_plans(no_subprocess, tmp_path, capsys): + path = _minimal_record(tmp_path) + assert run_evals.main(["--replay", str(path), "--dry-run"]) == 0 + out = capsys.readouterr().out + assert "plan: 1 fixture(s) x 1 repeat(s) = 1 run(s)" in out + assert "harness: claude (adapter claude, model harness default)" in out + + +def test_replay_takes_its_configuration_only_from_the_record(tmp_path, capsys): + path = tmp_path / "record.json" + path.write_text("{}", encoding="utf-8") + assert run_evals.main(["--replay", str(path), "--repeat", "3", "--skill", "x"]) == 2 + assert "drop --skill, --repeat" in capsys.readouterr().err + + +@pytest.mark.parametrize( + ("text", "message"), + [ + ("not json", "cannot read the run record"), + ("{}", "missing 'record_type'"), + ('{"record_type": "other"}', "not a skilldeck eval run record"), + ( + '{"record_type": "skilldeck-eval-run", "schema_version": 99}', + "unsupported schema_version 99", + ), + ], +) +def test_replay_rejects_an_unreadable_record(tmp_path, capsys, text, message): + path = tmp_path / "record.json" + path.write_text(text, encoding="utf-8") + assert run_evals.main(["--replay", str(path)]) == 2 + assert message in capsys.readouterr().err + + +def test_replay_refuses_a_fixture_that_no_longer_exists(run_main, tmp_path, capsys): + path = _first_record(run_main, tmp_path) + record = json.loads(path.read_text(encoding="utf-8")) + record["fixtures"][0]["name"] = "../logging" + path.write_text(json.dumps(record), encoding="utf-8") + assert run_evals.main(["--replay", str(path)]) == 2 + assert "fixture '../logging' from the record no longer exists" in ( + capsys.readouterr().err + ) diff --git a/tests/test_eval_scoring.py b/tests/test_eval_scoring.py index 70f3766..5bdbc14 100644 --- a/tests/test_eval_scoring.py +++ b/tests/test_eval_scoring.py @@ -591,10 +591,26 @@ def test_stderr_is_not_scored(): def test_missing_agent_command_is_a_clean_error(monkeypatch, tmp_path): _fake_subprocess_run(monkeypatch, raises=FileNotFoundError()) - with pytest.raises(SystemExit, match="agent command not found"): + with pytest.raises(run_evals.AgentStartError, match="agent command not found"): run_evals.run_agent("no-such-agent {prompt}", "p", tmp_path, 5) +def test_an_agent_that_cannot_start_is_a_clean_error(monkeypatch, tmp_path): + _fake_subprocess_run(monkeypatch, raises=PermissionError("not executable")) + with pytest.raises(run_evals.AgentStartError, match="could not start"): + run_evals.run_agent("./agent {prompt}", "p", tmp_path, 5) + + +def test_run_agent_substitutes_the_model_and_prompt_in_one_pass(monkeypatch, tmp_path): + calls = _fake_subprocess_run( + monkeypatch, subprocess.CompletedProcess([], 0, stdout="", stderr="") + ) + run_evals.run_agent("agent -m {model} {prompt}", "{model}?", tmp_path, 5, "m1") + assert calls[0][0] == ["agent", "-m", "m1", "{model}?"] + run_evals.run_agent("agent -m {model} {prompt}", "p", tmp_path, 5, "{prompt}") + assert calls[1][0] == ["agent", "-m", "{prompt}", "p"] + + # -- adapters, prompts, and fixture selection ---------------------------------- @@ -661,7 +677,11 @@ def test_main_repeats_each_fixture_and_reports_the_pass_rate( assert status == 0, out assert "PASS logging #1" in out and "PASS logging #2" in out assert "2/2 (100%) logging" in out - assert not workdir.exists() # cleaned up when every run passes + # the review repos are cleaned up when every run passes; the run record + # and the raw reports stay + assert not (workdir / "repos").exists() + assert (workdir / run_evals.RECORD_NAME).is_file() + assert (workdir / "artifacts" / "run-2" / "logging" / "report.txt").is_file() def test_main_reports_a_failing_agent_with_its_stderr(monkeypatch, tmp_path, capsys): @@ -682,9 +702,11 @@ def test_main_reports_a_failing_agent_with_its_stderr(monkeypatch, tmp_path, cap assert "FAIL logging" in out assert "agent exited with status 3" in out assert "upstream overloaded" in out - report = workdir / "logging" / "report.txt" + artifacts = workdir / "artifacts" / "run-1" / "logging" + report = artifacts / "report.txt" assert report.read_text(encoding="utf-8") == "partial output\n" - assert (workdir / "logging" / "stderr.txt").is_file() + assert (artifacts / "stderr.txt").is_file() + assert (workdir / "repos" / "run-1" / "logging" / ".git").is_dir() def test_main_rejects_an_unknown_fixture(capsys): From cb96bfdd75c0043006ad68e9f9f2b8b951c530ca Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:46:22 +0000 Subject: [PATCH 2/3] Harden eval runs: stdin, lost records, replay trust, digests Address review findings on the reproducible-eval-runs change (#74): - Agents, version probes and git run with stdin=DEVNULL, so `codex exec` neither blocks on nor ingests the runner's stdin. - A run whose repo can't be built (e.g. a SkillError from the adapter) or whose report can't be stored or scored is recorded as `error` and the rest go on; the run loop writes the record in a finally block, so an interrupt (exit 130) or a runner bug keeps the runs already paid for. - --replay only runs a built-in preset's own command under that preset's name; a custom or altered command needs --trust-record-command. Model names that look like options are rejected. - rendered_sha256 is the install stamp's hash (rendered content, stamp excluded); docs and schema say so, and a test checks it against the installed file. - The version probe runs only the preset's own executable, so wrappers like `env ... claude` or `npx @openai/codex` record version null. - Replay refuses a changed review prompt, and notes a changed runner in --dry-run too. - Fixture digests cover git-tracked plus untracked-not-ignored files (all files outside git), skip __pycache__/*.pyc/.DS_Store, and include the exec bit (from the index when tracked); review repos are built from exactly those files. - schema_version rejects `true`, commit ids may be SHA-256, error problems carry work-dir-relative paths, and the docstring example uses --harness codex. - An autouse test guard fails any test that would run a bare `claude` or `codex` from PATH. Part of #74. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- CHANGELOG.md | 20 +- evals/README.md | 58 +++-- evals/run-record.schema.json | 6 +- evals/run_evals.py | 468 +++++++++++++++++++++++++---------- tests/conftest.py | 20 ++ tests/test_eval_runs.py | 406 ++++++++++++++++++++++++++++-- 6 files changed, 797 insertions(+), 181 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0efcdee..f14fb14 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -629,9 +629,10 @@ All notable changes to this project are documented here. The format is based on - Reproducible eval runs (#74): every `evals/run_evals.py` invocation writes a provider-neutral, schema-versioned `run-record.json` (`evals/run-record.schema.json`, sorted keys) to its work dir. It holds the - skilldeck version and git commit, each fixture's content digest, each - skill's version, canonical digest and installed-file digest, the harness, - exact command template, version and model, and one entry per planned run + skilldeck version and git commit, each fixture's content digest (files, + exec bits and content), each skill's version, canonical digest and rendered + digest (the install stamp's hash), the harness, exact command template, + version and model, and one entry per planned run (passed, failed, agent failure, timeout, error, or not run) with timing, exit code, finding count and the scorer's reasons. Raw reports and stderr stay in separate files it points to (`--include-reports` embeds them). @@ -640,10 +641,15 @@ All notable changes to this project are documented here. The format is based on through `{model}`. `--dry-run` validates the fixtures and prints the plan without running anything; `--max-runs N` (default 50) refuses an oversized plan before it starts; runs stay sequential. `--replay RECORD` re-runs a - record's configuration and refuses if any fixture or skill changed since. - A missing agent command is now recorded, and stops the remaining runs, - instead of aborting without a record; a passing run keeps its record and - raw output (only the review repos are deleted). + record's configuration and refuses if any fixture, skill or prompt changed + since, or if its command is not a built-in preset's (unless + `--trust-record-command`). A missing agent command, a repo the adapter + can't install into, a scoring error, an interrupt or a runner bug is now + recorded instead of losing the record; a passing run keeps its record and + raw output (only the review repos are deleted). Agents now run with stdin + closed (`/dev/null`), so `codex exec` no longer waits on or ingests the + runner's stdin, and a review repo is built from exactly the fixture files + its digest covers (no `__pycache__` or `.DS_Store`). - `skilldeck provenance --verify` re-hashes each installed skill's `meta.yaml` and `skill.md` and exits 1, naming the skill, when one no longer matches its recorded canonical digest, is missing, or has unexpected files beside it. diff --git a/evals/README.md b/evals/README.md index 8089a46..b4e8631 100644 --- a/evals/README.md +++ b/evals/README.md @@ -37,8 +37,9 @@ python evals/run_evals.py --keep # keep the review repos | `--repeat N` | Run each fixture `N` times (fresh repo each time) and print its pass rate — agents are nondeterministic, so one run says little about a borderline fixture. | | `--timeout S` | Per-run agent timeout in seconds (default 600). | | `--max-runs N` | Refuse to start if more than `N` runs (fixtures × repeats) are planned (default 50). | -| `--dry-run` | Validate the fixtures and print the planned runs; no agent (or anything else) is run. Exits 2 on an invalid fixture or a plan over `--max-runs`. | +| `--dry-run` | Validate the fixtures and print the planned runs; no agent or version probe is run (only read-only `git ls-files`, to list fixture files). Exits 2 on an invalid fixture or a plan over `--max-runs`. | | `--replay RECORD` | Re-run a [run record](#run-records)'s exact configuration; see [Replaying](#replaying). | +| `--trust-record-command` | With `--replay`: run the record's command even though it is not a built-in preset's command (see [Replaying](#replaying)). | | `--include-reports` | Also copy each run's raw stdout and stderr into the run record. | | `--keep` | Keep the review repos even when every run passes. | @@ -68,10 +69,15 @@ skilldeck-evals-XXXX/ ``` The review repos are deleted when every run passes (unless `--keep`); the -record and the raw output are always kept. The process exits 0 when every run -passed, 1 when any failed, 2 when the evals could not run (invalid fixture, -budget, changed digests on replay, agent command not found) and 130 when -interrupted — the record is written in every case that started running. +record and the raw output are always kept. The agent gets no stdin (it reads +`/dev/null`), so a CLI that reads a piped stdin neither blocks nor ingests the +runner's. The process exits 0 when every run passed, 1 when any failed, 2 when +the evals could not run (invalid fixture, budget, changed digests or prompt on +replay, agent command not found) and 130 when interrupted. Once runs start, the +record is always written: a run whose repo can't be built (the adapter refuses +to install, git fails) or whose report can't be stored or scored is recorded as +`error` and the rest go on; an interrupt or a runner bug records the runs so +far, the one in flight as `error` and the rest as `not_run`. ## Harnesses @@ -86,10 +92,14 @@ flags: Claude Code's print mode, and `codex exec`, which prints the final message on stdout (progress goes to stderr, which is not scored) and runs in a read-only sandbox by default. The Codex preset is best-effort — it has not yet been exercised in a recorded run. `--agent-cmd` overrides a preset's command -but keeps its name, adapter and version probe (the command's own executable -with `--version`); add flags there, such as an approval or sandbox mode. A -harness that reports token usage or cost would fill the record's `usage` and -`cost_usd`; no preset parses them yet, so both are `null`. +but keeps its name and adapter; add flags there, such as an approval or sandbox +mode. The version probe runs the command's own executable with `--version`, +and only when that executable is the preset's (`claude` or `codex`, by any +path, with or without `.exe`/`.cmd`): through a wrapper such as +`env FOO=1 claude …` or `npx @openai/codex …` it would report the wrapper's +version, so the record's `version` is `null` instead. A harness that reports +token usage or cost would fill the record's `usage` and `cost_usd`; no preset +parses them yet, so both are `null`. ## Run records @@ -101,10 +111,15 @@ platform: - **what ran**: the skilldeck version, the checkout's git commit and whether it had uncommitted changes (`null` outside a checkout), and the digest of `run_evals.py` itself (the scorer and the prompt); -- **against what**: per fixture, a digest of every file in its directory - (newlines normalised, so a Windows checkout agrees), the skill's name, - version, canonical digest (the one in `src/skilldeck/_content_manifest.json`) - and the digest of the file the adapter installed, plus the exact prompt; +- **against what**: per fixture, a digest of its files — in a git checkout + the files git tracks plus untracked ones it doesn't ignore, elsewhere every + file, never `__pycache__`, `*.pyc` or `.DS_Store` — covering each path, + executable bit (from the git index when tracked) and content (newlines + normalised, so a Windows checkout agrees); the review repo is built from + exactly those files. Then the skill's name, version, canonical digest (the + one in `src/skilldeck/_content_manifest.json`) and `rendered_sha256`: the + rendered skill content the adapter installs, excluding the install stamp + (it equals the stamp's `hash=`); plus the exact prompt; - **how**: the harness name, the exact command template, the model (if requested), the harness version (the probe's first line, `null` if it failed), the adapter, and the repeat, timeout and budget; @@ -126,13 +141,20 @@ dir beside it. command, model, adapter, repeat count and timeout (so it takes no other configuration options; `--max-runs`, `--dry-run`, `--keep` and `--include-reports` still apply). Before anything runs it recomputes every -fixture digest and skill digest and **refuses** (exit 2) if any fixture, skill -or installed skill file changed since the record — a changed eval is a -different experiment. A different harness version or a changed runner is +fixture digest, skill digest and review prompt and **refuses** (exit 2) if any +fixture, skill, rendered skill or prompt changed since the record — the agent +would see a different input, so it is a different experiment. A different +harness version or a changed runner (`run_evals.py`, which holds the scorer) is printed as a note, not refused. The new record's `config.replay_of` holds the digest of the record it replayed; comparing the two records' runs is the -variance check. `--replay RECORD --dry-run` verifies the digests without -running anything. +variance check. `--replay RECORD --dry-run` verifies all of this without +running an agent. + +**A record is data, and replaying it runs its command.** A record can be +edited, or come from someone else, so replay accepts only a built-in preset's +own command (`claude` or `codex`, with or without a model) under that preset's +name. A custom harness's command, or a preset name on any other command, is +refused unless you read the command and pass `--trust-record-command`. ## Scoring diff --git a/evals/run-record.schema.json b/evals/run-record.schema.json index fb263fe..c420699 100644 --- a/evals/run-record.schema.json +++ b/evals/run-record.schema.json @@ -74,7 +74,7 @@ "type": ["string", "null"] }, "version": { - "description": "First line of the version probe's output; null if it failed or the harness is custom.", + "description": "First line of the version probe's output; null if it failed, the harness is custom, or the command's executable is not the preset's.", "type": ["string", "null"] }, "version_command": { @@ -149,7 +149,7 @@ "properties": { "name": {"type": "string"}, "digest": { - "description": "Digest of every file in the fixture directory (newlines normalised).", + "description": "Digest of the fixture's files (git-tracked or untracked-and-not-ignored in a checkout, else all; junk excluded): path, executable bit, content with newlines normalised.", "$ref": "#/$defs/digest" }, "skill": { @@ -164,7 +164,7 @@ "$ref": "#/$defs/digest" }, "rendered_sha256": { - "description": "Digest of the file the adapter installs: what the agent reads.", + "description": "Rendered skill content the adapter installs, excluding the install stamp (equals the stamp's hash=).", "$ref": "#/$defs/digest" } } diff --git a/evals/run_evals.py b/evals/run_evals.py index 2458f06..dfd7660 100644 --- a/evals/run_evals.py +++ b/evals/run_evals.py @@ -24,7 +24,7 @@ template, version and model, and one entry per planned run -- passed, failed, timed out, errored or not run. Raw reports and stderr stay in the work dir as separate files the record points to. ``--replay`` re-runs a record's exact -configuration after checking that no skill or fixture changed since. +configuration after checking that no skill, fixture or prompt changed. This calls a real agent and costs real money -- it is run manually (e.g. before a release), not in CI. CI only validates fixture structure and the @@ -39,7 +39,7 @@ python evals/run_evals.py --harness codex # codex CLI + codex adapter python evals/run_evals.py --model sonnet # pass a model to the harness python evals/run_evals.py --agent-cmd 'claude -p {prompt}' - python evals/run_evals.py --adapter codex --agent-cmd 'codex exec {prompt}' + python evals/run_evals.py --harness codex --agent-cmd 'codex exec {prompt}' python evals/run_evals.py --dry-run # validate + plan, no agent python evals/run_evals.py --replay run-record.json # same config again python evals/run_evals.py --keep # keep temp repos to inspect @@ -67,7 +67,7 @@ from collections.abc import Sequence from dataclasses import dataclass from datetime import datetime, timezone -from pathlib import Path +from pathlib import Path, PureWindowsPath from typing import TypedDict import yaml @@ -103,7 +103,11 @@ RECORD_SCHEMA_VERSION = 1 RECORD_TYPE = "skilldeck-eval-run" _FIXTURE_DOMAIN = b"skilldeck-eval-fixture-v1\0" -_COMMIT_RE = re.compile(r"^[0-9a-f]{40}$") +# a SHA-1 or SHA-256 object name +_COMMIT_RE = re.compile(r"^[0-9a-f]{40}(?:[0-9a-f]{24})?$") +#: files a fixture directory may pick up that are not part of the fixture +_JUNK_NAMES = frozenset({"__pycache__", ".DS_Store"}) +_JUNK_SUFFIXES = (".pyc", ".pyo") _DIGEST_RE = re.compile(r"^sha256:[0-9a-f]{64}$") #: the finding severity scale from docs/finding-output.md, lowest first @@ -163,6 +167,13 @@ class HarnessPreset: #: the same command with the harness's model option (``{model}``) model_command: str + @property + def executable(self) -> str: + return shlex.split(self.command)[0] + + def expected_command(self, model: str | None) -> str: + return self.command if model is None else self.model_command + #: built-in harnesses; ``--harness custom`` takes its command from --agent-cmd HARNESSES = { @@ -198,10 +209,23 @@ class Harness: @property def version_command(self) -> list[str] | None: - """How to ask a preset harness its version (None for a custom one).""" - if self.name not in HARNESSES: + """How to ask the harness its version, or None if that can't be known. + + Only a preset's own executable is probed: a custom harness, or a + preset whose --agent-cmd runs it through a wrapper (``env ... claude``, + ``npx @openai/codex``), would otherwise credit the wrong program. + """ + preset = HARNESSES.get(self.name) + if preset is None: return None - return [shlex.split(self.command)[0], "--version"] + executable = shlex.split(self.command)[0] + # PureWindowsPath splits on both separators; drop a Windows suffix + name = PureWindowsPath(executable).name.lower() + for suffix in (".exe", ".cmd"): + name = name.removesuffix(suffix) + if name != preset.executable: + return None + return [executable, "--version"] @dataclass(frozen=True) @@ -616,6 +640,7 @@ def _git(repo: Path, *args: str) -> None: ["git", "-c", "user.name=evals", "-c", "user.email=evals@localhost", *args], cwd=repo, check=True, + stdin=subprocess.DEVNULL, capture_output=True, ) @@ -645,13 +670,89 @@ def build_prompt(fixture: Fixture, adapter: str = DEFAULT_ADAPTER) -> str: ) +@dataclass(frozen=True) +class FixtureFile: + #: POSIX path relative to the fixture directory + relative: str + path: Path + executable: bool + + +def _is_junk(relative: str) -> bool: + return relative.endswith(_JUNK_SUFFIXES) or any( + part in _JUNK_NAMES for part in relative.split("/") + ) + + +def _executable(path: Path) -> bool: + # Windows has no exec bit; git there doesn't track one either + return os.name != "nt" and bool(path.stat().st_mode & stat.S_IXUSR) + + +def _git_listed_files(root: Path) -> dict[str, bool] | None: + """Tracked and untracked-but-not-ignored files under ``root``, if in git. + + Maps each path (relative to ``root``) to its executable bit -- from the + index for a tracked file, so every platform agrees. None outside a git + work tree, or when git lists nothing there (e.g. an ignored directory). + """ + staged = _git_output(root, "ls-files", "-z", "--stage") + others = _git_output(root, "ls-files", "-z", "--others", "--exclude-standard") + if staged is None or others is None: + return None + files: dict[str, bool] = {} + for entry in staged.split("\0"): + meta, _, relative = entry.partition("\t") + if relative: + files[relative] = meta.split()[0] == "100755" + for relative in others.split("\0"): + if relative: + files[relative] = _executable(root / relative) + return files or None + + +def fixture_files(root: Path) -> list[FixtureFile]: + """The files that make up a fixture, sorted by relative POSIX path. + + In a git work tree: what git tracks plus untracked files it doesn't + ignore. Elsewhere (a copy of a fixture, say): every file. Either way + without stray junk (``__pycache__``, ``*.pyc``, ``.DS_Store``). + """ + listed = _git_listed_files(root) + if listed is None: + listed = { + file.relative_to(root).as_posix(): _executable(file) + for file in root.rglob("*") + if file.is_file() + } + return [ + FixtureFile(relative, root / relative, executable) + for relative, executable in sorted(listed.items()) + if not _is_junk(relative) and (root / relative).is_file() + ] + + +def _overlay(files: Sequence[FixtureFile], part: str, repo: Path) -> None: + """Copy the fixture's ``part/`` files (base or change) into ``repo``.""" + prefix = f"{part}/" + for file in files: + if file.relative.startswith(prefix): + target = repo / file.relative.removeprefix(prefix) + target.parent.mkdir(parents=True, exist_ok=True) + shutil.copy2(file.path, target) + + def prepare_repo( fixture: Fixture, workdir: Path, adapter: str = DEFAULT_ADAPTER ) -> Path: """Materialize the fixture as a git repo with a ``change`` branch.""" skill = installable_skill(fixture, adapter) repo = workdir / fixture.name - shutil.copytree(fixture.path / "base", repo) + # exactly the files fixture_digest covers, so a record's digest names + # what the agent saw + files = fixture_files(fixture.path) + repo.mkdir(parents=True) + _overlay(files, "base", repo) # installed before the base commit, so the skill file is neither part of # the diff under review nor an untracked change the skill's scope step # would pick up @@ -660,7 +761,7 @@ def prepare_repo( _git(repo, "add", "-A") _git(repo, "commit", "-q", "-m", "base") _git(repo, "checkout", "-q", "-b", "change") - shutil.copytree(fixture.path / "change", repo, dirs_exist_ok=True) + _overlay(files, "change", repo) _git(repo, "add", "-A") _git(repo, "commit", "-q", "-m", "change under review") return repo @@ -720,6 +821,9 @@ def run_agent( result = subprocess.run( cmd, cwd=repo, + # an agent CLI that reads a non-TTY stdin (codex exec does) must + # neither block on nor ingest the runner's own stdin + stdin=subprocess.DEVNULL, capture_output=True, text=True, encoding="utf-8", @@ -771,10 +875,7 @@ def resolve_harness( command, default_adapter = agent_cmd, DEFAULT_ADAPTER elif name in HARNESSES: preset = HARNESSES[name] - if agent_cmd is not None: - command = agent_cmd - else: - command = preset.command if model is None else preset.model_command + command = agent_cmd if agent_cmd is not None else preset.expected_command(model) default_adapter = preset.adapter else: choices = [*sorted(HARNESSES), CUSTOM_HARNESS] @@ -786,6 +887,9 @@ def resolve_harness( ) if model is not None and not model.strip(): raise ConfigError("--model must not be empty") + if model is not None and model.startswith("-"): + # it lands in the harness's argv: a "model" like --yolo is a flag + raise ConfigError(f"--model {model!r} looks like an option, not a model") try: parts = shlex.split(command) except ValueError as exc: @@ -813,6 +917,7 @@ def harness_version(command: Sequence[str] | None) -> str | None: try: result = subprocess.run( list(command), + stdin=subprocess.DEVNULL, capture_output=True, text=True, encoding="utf-8", @@ -834,7 +939,9 @@ class SkillIdentity(TypedDict): version: str #: skilldeck.provenance.canonical_skill_digest of meta.yaml + skill.md canonical_sha256: str - #: the file the adapter installs, i.e. exactly what the agent reads + #: the rendered skill content the adapter installs, excluding the install + #: stamp -- the stamp's own ``hash=`` (and, for claude, the content + #: manifest's claude_rendered_sha256) rendered_sha256: str @@ -845,25 +952,21 @@ class FixtureIdentity(TypedDict): def fixture_digest(path: Path) -> str: - """Hash every file under a fixture directory, by POSIX path and content. + """Hash a fixture's files (see fixture_files): path, exec bit, content. Domain-separated and length-framed like the canonical skill digest. UTF-8 text is hashed with normalised newlines, so a CRLF checkout on Windows and an LF one agree; any other file is hashed as raw bytes. """ digest = hashlib.sha256(_FIXTURE_DOMAIN) - files = sorted( - (file.relative_to(path).as_posix(), file) - for file in path.rglob("*") - if file.is_file() - ) - for relative, file in files: - data = file.read_bytes() + for file in fixture_files(path): + data = file.path.read_bytes() with contextlib.suppress(UnicodeDecodeError): data = normalise_text(data.decode("utf-8")).encode("utf-8") - name = relative.encode("utf-8") + name = file.relative.encode("utf-8") digest.update(len(name).to_bytes(4, "big")) digest.update(name) + digest.update(b"\x01" if file.executable else b"\x00") digest.update(len(data).to_bytes(8, "big")) digest.update(data) return f"sha256:{digest.hexdigest()}" @@ -874,11 +977,13 @@ def runner_digest() -> str: return sha256_text(Path(__file__).read_text(encoding="utf-8")) -def _git_output(*args: str) -> str | None: +def _git_output(cwd: Path, *args: str) -> str | None: + """git's stdout in ``cwd``, or None if git is missing or fails.""" try: result = subprocess.run( ["git", *args], - cwd=ROOT, + cwd=cwd, + stdin=subprocess.DEVNULL, capture_output=True, text=True, encoding="utf-8", @@ -902,14 +1007,14 @@ def source_identity() -> dict[str, object]: """The skilldeck version and, in a git checkout, its commit and state.""" commit: str | None = None dirty: bool | None = None - toplevel = _git_output("rev-parse", "--show-toplevel") + toplevel = _git_output(ROOT, "rev-parse", "--show-toplevel") # only this checkout's own commit: a source tree unpacked inside some # other repository must not borrow that repository's HEAD if toplevel is not None and _same_path(Path(toplevel.strip()), ROOT): - head = (_git_output("rev-parse", "HEAD") or "").strip() + head = (_git_output(ROOT, "rev-parse", "HEAD") or "").strip() if _COMMIT_RE.fullmatch(head): commit = head - status = _git_output("status", "--porcelain") + status = _git_output(ROOT, "status", "--porcelain") dirty = None if status is None else bool(status.strip()) return { "version": __version__, @@ -946,6 +1051,18 @@ def fixture_layout_problems(fixture: Fixture) -> list[str]: return problems +def rendered_digest(adapter: str, skill: Skill) -> str: + """The hash an install stamp records: the rendered skill, stamp excluded. + + Computed as skilldeck.stamp does -- the content, newline-terminated, as + UTF-8 -- so it equals the installed file's ``hash=``. + """ + content = ADAPTERS[adapter].render(skill) + if not content.endswith("\n"): + content += "\n" + return "sha256:" + hashlib.sha256(content.encode("utf-8")).hexdigest() + + def plan_fixture(fixture: Fixture, adapter: str) -> PlannedFixture: """Validate ``fixture`` for ``adapter`` and pin down its identity.""" problems = fixture_layout_problems(fixture) @@ -963,7 +1080,7 @@ def plan_fixture(fixture: Fixture, adapter: str) -> PlannedFixture: "name": skill.name, "version": skill.version, "canonical_sha256": canonical_skill_digest(meta_text, skill.body), - "rendered_sha256": sha256_text(ADAPTERS[adapter].render(skill)), + "rendered_sha256": rendered_digest(adapter, skill), }, }, ) @@ -1015,6 +1132,8 @@ class ReplaySpec: repeat: int timeout: int fixtures: tuple[FixtureIdentity, ...] + #: the prompt each fixture's runs were given, in ``fixtures`` order + prompts: tuple[str, ...] runner_sha256: str | None #: sha256 of the record itself, stored in the new record's replay_of record_sha256: str @@ -1054,8 +1173,12 @@ def _get_digest(where: str, mapping: object, key: str) -> str: return value -def load_replay(path: Path) -> ReplaySpec: - """Read the configuration and identities to replay from a run record.""" +def load_replay(path: Path, trust_command: bool = False) -> ReplaySpec: + """Read the configuration and identities to replay from a run record. + + A record is data, and replaying it runs its command: unless + ``trust_command``, the command must be the recorded preset's own. + """ try: text = path.read_text(encoding="utf-8") data = json.loads(text) @@ -1065,7 +1188,8 @@ def load_replay(path: Path) -> ReplaySpec: if _get(where, data, "record_type") != RECORD_TYPE: raise ConfigError(f"{where}: not a skilldeck eval run record") version = _get(where, data, "schema_version") - if version != RECORD_SCHEMA_VERSION: + # type check first: True == 1 in Python, but not in JSON + if type(version) is not int or version != RECORD_SCHEMA_VERSION: raise ConfigError( f"{where}: unsupported schema_version {version!r} " f"(this runner reads {RECORD_SCHEMA_VERSION})" @@ -1076,9 +1200,11 @@ def load_replay(path: Path) -> ReplaySpec: if not isinstance(raw_fixtures, list) or not raw_fixtures: raise ConfigError(f"{where}: 'fixtures' must be a non-empty list") identities: list[FixtureIdentity] = [] + prompts: list[str] = [] for i, entry in enumerate(raw_fixtures): at = f"{where}: fixtures[{i}]" skill = _get(at, entry, "skill") + prompts.append(_get_str(at, entry, "prompt")) identities.append( { "name": _get_str(at, entry, "name"), @@ -1099,17 +1225,31 @@ def load_replay(path: Path) -> ReplaySpec: if len(names) != len(set(names)): raise ConfigError(f"{where}: a fixture is listed twice") source = _get(where, data, "skilldeck") + harness = resolve_harness( + _get_str(f"{where}: harness", harness_data, "name"), + _get_str(f"{where}: harness", harness_data, "command"), + _get_str(where, data, "adapter"), + _get_optional_str(f"{where}: harness", harness_data, "model"), + ) + preset = HARNESSES.get(harness.name) + if not trust_command and ( + preset is None + or shlex.split(harness.command) + != shlex.split(preset.expected_command(harness.model)) + ): + raise ConfigError( + f"{where}: the record's {harness.name} harness command " + f"{harness.command!r} is not the built-in preset's, and replaying " + "runs it as-is; read it, and pass --trust-record-command if you " + "trust the record" + ) return ReplaySpec( - harness=resolve_harness( - _get_str(f"{where}: harness", harness_data, "name"), - _get_str(f"{where}: harness", harness_data, "command"), - _get_str(where, data, "adapter"), - _get_optional_str(f"{where}: harness", harness_data, "model"), - ), + harness=harness, harness_version=_get_optional_str(f"{where}: harness", harness_data, "version"), repeat=_get_count(f"{where}: config", config, "repeat"), timeout=_get_count(f"{where}: config", config, "timeout_s"), fixtures=tuple(identities), + prompts=tuple(prompts), runner_sha256=_get_optional_str(f"{where}: skilldeck", source, "runner_sha256"), record_sha256=sha256_text(text), ) @@ -1129,8 +1269,10 @@ def replay_fixtures(spec: ReplaySpec) -> list[Fixture]: return fixtures -def replay_problems(recorded: FixtureIdentity, planned: PlannedFixture) -> list[str]: - """How ``planned`` differs from the identity a run record pinned.""" +def replay_problems( + recorded: FixtureIdentity, prompt: str, planned: PlannedFixture +) -> list[str]: + """How ``planned`` differs from the identity and prompt a record pinned.""" current = planned.identity name = recorded["name"] @@ -1143,9 +1285,13 @@ def changed(what: str, before: str, after: str) -> str: ("skill", before["name"], after["name"]), ("skill version", before["version"], after["version"]), ("skill content", before["canonical_sha256"], after["canonical_sha256"]), - ("installed skill file", before["rendered_sha256"], after["rendered_sha256"]), + ("rendered skill", before["rendered_sha256"], after["rendered_sha256"]), ) - return [changed(what, old, new) for what, old, new in pairs if old != new] + problems = [changed(what, old, new) for what, old, new in pairs if old != new] + if prompt != planned.prompt: + # the prompt lives in this runner, or names the adapter's install path + problems.append(f"{name}: the review prompt changed since the record") + return problems # -- running and recording --------------------------------------------------- @@ -1197,6 +1343,15 @@ def run_status(run: AgentRun, passed: bool) -> str: return "passed" if passed else "failed" +def describe_error(exc: BaseException, workdir: Path) -> str: + """``exc`` as a record problem, with work dir paths made relative to it.""" + text = f"{type(exc).__name__}: {exc}" + for form in dict.fromkeys((str(workdir), workdir.as_posix())): + text = text.replace(form + "/", "").replace(form + os.sep, "") + text = text.replace(form, ".") + return text + + def attempt_run( planned: PlannedFixture, harness: Harness, @@ -1207,42 +1362,47 @@ def attempt_run( ) -> tuple[RunEntry, AgentRun | None]: """Build a fresh repo, run the agent in it, score it, and store its output. - Raises AgentStartError if the agent command can't be started at all. + Any failure other than the agent's is recorded as an ``error`` entry, so + one broken run never costs the record of the paid ones. Raises + AgentStartError if the agent command can't be started at all. """ fixture = planned.fixture try: repo = prepare_repo( fixture, workdir / "repos" / f"run-{attempt}", harness.adapter ) - except (OSError, subprocess.CalledProcessError) as exc: - problem = f"could not build the review repo: {exc}" + except Exception as exc: # a SkillError from the adapter, git, I/O, ... + problem = f"could not build the review repo: {describe_error(exc, workdir)}" return RunEntry(attempt, "error", [problem]), None started_at, clock = _now(), time.monotonic() run = run_agent(harness.command, planned.prompt, repo, timeout, harness.model) - duration = round(time.monotonic() - clock, 3) - finished_at = _now() - - # raw output lives beside the record, never in it (unless asked for) - out = workdir / "artifacts" / f"run-{attempt}" / fixture.name - out.mkdir(parents=True, exist_ok=True) - (out / "report.txt").write_text(run.stdout, encoding="utf-8") - (out / "stderr.txt").write_text(run.stderr, encoding="utf-8") - problems, passed = evaluate(fixture, run) entry = RunEntry( attempt=attempt, - status=run_status(run, passed), - problems=problems, + status="error", + problems=[], started_at=started_at, - finished_at=finished_at, - duration_s=duration, + finished_at=_now(), + duration_s=round(time.monotonic() - clock, 3), exit_code=run.returncode, - finding_count=len(parse_findings(run.stdout)), - artifacts={ - name: (out / f"{name}.txt").relative_to(workdir).as_posix() - for name in ("report", "stderr") - }, raw={"report": run.stdout, "stderr": run.stderr} if include_reports else None, ) + try: + # raw output lives beside the record, never in it (unless asked for) + out = workdir / "artifacts" / f"run-{attempt}" / fixture.name + out.mkdir(parents=True, exist_ok=True) + (out / "report.txt").write_text(run.stdout, encoding="utf-8") + (out / "stderr.txt").write_text(run.stderr, encoding="utf-8") + entry.artifacts = { + name: (out / f"{name}.txt").relative_to(workdir).as_posix() + for name in ("report", "stderr") + } + entry.finding_count = len(parse_findings(run.stdout)) + problems, passed = evaluate(fixture, run) + except Exception as exc: + problem = f"could not store or score the report: {describe_error(exc, workdir)}" + entry.problems = [problem] + return entry, run + entry.status, entry.problems = run_status(run, passed), problems return entry, run @@ -1254,10 +1414,22 @@ def build_record( planned: Sequence[PlannedFixture], entries: dict[str, list[RunEntry]], config: dict[str, object], + repeat: int, started_at: str, - stopped: bool, + stopped: str | None, ) -> dict[str, object]: - """The run record: provider-neutral, and free of raw agent output.""" + """The run record: provider-neutral, and free of raw agent output. + + ``entries`` may stop short of the plan; every planned run it lacks is + recorded as ``not_run``. + """ + reason = f"not run: {stopped}" if stopped else "not run" + for p in planned: + done = entries.setdefault(p.fixture.name, []) + done.extend( + RunEntry(attempt, "not_run", [reason]) + for attempt in range(len(done) + 1, repeat + 1) + ) runs = [entry for p in planned for entry in entries[p.fixture.name]] not_run = sum(entry.status == "not_run" for entry in runs) passed = sum(entry.passed for entry in runs) @@ -1369,8 +1541,14 @@ def _parser() -> argparse.ArgumentParser: "--replay", type=Path, metavar="RECORD", - help="re-run a run record's configuration, refusing if a skill or " - "fixture changed since", + help="re-run a run record's configuration, refusing if a skill, " + "fixture or prompt changed since, or if its command is not a preset's", + ) + parser.add_argument( + "--trust-record-command", + action="store_true", + help="with --replay: run the record's command even if it is not the " + "built-in preset's (a custom harness always needs this)", ) parser.add_argument( "--include-reports", @@ -1404,10 +1582,12 @@ def main(argv: Sequence[str] | None = None) -> int: "--replay takes its configuration from the record; drop " + ", ".join(clashes) ) - replay = load_replay(args.replay) + replay = load_replay(args.replay, args.trust_record_command) harness, repeat, timeout = replay.harness, replay.repeat, replay.timeout fixtures = replay_fixtures(replay) else: + if args.trust_record_command: + raise ConfigError("--trust-record-command only applies to --replay") harness = resolve_harness( args.harness, args.agent_cmd, args.adapter, args.model ) @@ -1424,8 +1604,9 @@ def main(argv: Sequence[str] | None = None) -> int: # adapter can't install -- or, replaying, on anything that changed planned, problems = plan_fixtures(fixtures, harness.adapter) if replay is not None and not problems: - for recorded, current in zip(replay.fixtures, planned, strict=True): - problems.extend(replay_problems(recorded, current)) + pairs = zip(replay.fixtures, replay.prompts, planned, strict=True) + for recorded, prompt, current in pairs: + problems.extend(replay_problems(recorded, prompt, current)) if problems: for problem in problems: print(f"error: {problem}", file=sys.stderr) @@ -1440,94 +1621,107 @@ def main(argv: Sequence[str] | None = None) -> int: file=sys.stderr, ) return 2 + if replay is not None and runner_digest() != replay.runner_sha256: + print("note: the eval runner (scorer or prompt) changed since the record") if args.dry_run: print("\ndry run: fixtures are valid; no agent was invoked") return 0 source = source_identity() version = harness_version(harness.version_command) - if replay is not None: - if version != replay.harness_version: - print( - f"note: harness version {version!r} differs from the record's " - f"{replay.harness_version!r}" - ) - if source["runner_sha256"] != replay.runner_sha256: - print("note: the eval runner (scorer or prompt) changed since the record") + if replay is not None and version != replay.harness_version: + print( + f"note: harness version {version!r} differs from the record's " + f"{replay.harness_version!r}" + ) workdir = Path(tempfile.mkdtemp(prefix="skilldeck-evals-")) print(f"work dir: {workdir}\n") started_at = _now() - entries: dict[str, list[RunEntry]] = {} - pass_counts: dict[str, int] = {} + entries: dict[str, list[RunEntry]] = {p.fixture.name: [] for p in planned} stop: str | None = None # why the remaining runs were not attempted status = 0 - for p in planned: - fixture = p.fixture - entries[fixture.name] = [] - passes = 0 - for attempt in range(1, repeat + 1): - if stop is not None: - entry = RunEntry(attempt, "not_run", [f"not run: {stop}"]) + in_flight: tuple[str, int] | None = None # the run under way, if any + + def record_in_flight(problem: str) -> None: + if in_flight is not None: + name, attempt = in_flight + if len(entries[name]) < attempt: + entries[name].append(RunEntry(attempt, "error", [problem])) + + try: + for p in planned: + fixture = p.fixture + for attempt in range(1, repeat + 1): + in_flight = (fixture.name, attempt) + label = fixture.name if repeat == 1 else f"{fixture.name} #{attempt}" + run: AgentRun | None = None + try: + entry, run = attempt_run( + p, harness, attempt, workdir, timeout, args.include_reports + ) + except AgentStartError as exc: + stop, status = str(exc), 2 + entry = RunEntry(attempt, "error", [stop]) entries[fixture.name].append(entry) - continue - label = fixture.name if repeat == 1 else f"{fixture.name} #{attempt}" - run: AgentRun | None = None - try: - entry, run = attempt_run( - p, harness, attempt, workdir, timeout, args.include_reports - ) - except AgentStartError as exc: - stop, status = str(exc), 2 - entry = RunEntry(attempt, "error", [stop]) - except KeyboardInterrupt: - stop, status = "interrupted", 130 - entry = RunEntry(attempt, "error", [stop]) - entries[fixture.name].append(entry) - count = entry.finding_count - findings = "" if count is None else f" ({count} findings)" - print(f"{'PASS' if entry.passed else 'FAIL'} {label}{findings}") - for problem in entry.problems: - print(f" {problem}") - if not entry.passed and run is not None and run.stderr.strip(): - print(" agent stderr:") - print(textwrap.indent(run.stderr.rstrip(), " " * 8)) - passes += entry.passed - pass_counts[fixture.name] = passes - - config: dict[str, object] = { - "repeat": repeat, - "timeout_s": timeout, - "max_runs": args.max_runs, - "jobs": 1, - "include_reports": args.include_reports, - "replay_of": None if replay is None else replay.record_sha256, - } - record = build_record( - source=source, - harness=harness, - version=version, - planned=planned, - entries=entries, - config=config, - started_at=started_at, - stopped=stop is not None, - ) - record_path = workdir / RECORD_NAME - write_record(record_path, record) + count = entry.finding_count + findings = "" if count is None else f" ({count} findings)" + print(f"{'PASS' if entry.passed else 'FAIL'} {label}{findings}") + for problem in entry.problems: + print(f" {problem}") + if not entry.passed and run is not None and run.stderr.strip(): + print(" agent stderr:") + print(textwrap.indent(run.stderr.rstrip(), " " * 8)) + if stop is not None: + break + if stop is not None: + break + except KeyboardInterrupt: + stop, status = "interrupted", 130 + record_in_flight(stop) + except BaseException as exc: + # still record the runs already paid for, then fail loudly + stop = f"runner error: {describe_error(exc, workdir)}" + record_in_flight(stop) + raise + finally: + config: dict[str, object] = { + "repeat": repeat, + "timeout_s": timeout, + "max_runs": args.max_runs, + "jobs": 1, + "include_reports": args.include_reports, + "replay_of": None if replay is None else replay.record_sha256, + } + record = build_record( + source=source, + harness=harness, + version=version, + planned=planned, + entries=entries, + config=config, + repeat=repeat, + started_at=started_at, + stopped=stop, + ) + record_path = workdir / RECORD_NAME + write_record(record_path, record) + print(f"run record: {record_path}") - runs = len(planned) * repeat - failed = runs - sum(pass_counts.values()) + pass_counts = { + name: sum(entry.passed for entry in runs) for name, runs in entries.items() + } + runs_total = len(planned) * repeat + failed = runs_total - sum(pass_counts.values()) if repeat > 1: print("\npass rate per fixture:") for name, passes in pass_counts.items(): print(f" {passes}/{repeat} ({passes / repeat:4.0%}) {name}") - print(f"\n{runs - failed}/{runs} runs passed") + print(f"\n{runs_total - failed}/{runs_total} runs passed") else: - print(f"\n{runs - failed}/{runs} fixtures passed") + print(f"\n{runs_total - failed}/{runs_total} fixtures passed") if stop is not None: print(f"stopped early: {stop}", file=sys.stderr) - print(f"run record: {record_path}") print(f"reports and stderr: {workdir / 'artifacts'}") if args.keep or failed: print(f"review repos kept in {workdir / 'repos'}") diff --git a/tests/conftest.py b/tests/conftest.py index 48e4712..d31650d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -5,6 +5,7 @@ """ import importlib.util +import subprocess import sys from collections.abc import Callable from pathlib import Path @@ -41,6 +42,25 @@ def _no_agent_home_overrides(monkeypatch: pytest.MonkeyPatch) -> None: monkeypatch.delenv(var, raising=False) +#: agent CLIs the eval runner's presets name; a real one costs money to run +_REAL_AGENT_CLIS = frozenset({"claude", "codex"}) + + +@pytest.fixture(autouse=True) +def _no_real_agent_cli(monkeypatch: pytest.MonkeyPatch) -> None: + # The eval runner's presets run `claude`/`codex` from PATH, where a real, + # logged-in CLI may be installed. Tests use stand-ins by absolute path; + # a bare preset name reaching subprocess.run is a test bug, not a run. + real_run = subprocess.run + + def guarded(cmd: object, *args: object, **kwargs: object) -> object: + if isinstance(cmd, list) and cmd and cmd[0] in _REAL_AGENT_CLIS: + raise AssertionError(f"a test tried to run the real agent CLI: {cmd}") + return real_run(cmd, *args, **kwargs) # type: ignore[call-overload] + + monkeypatch.setattr(subprocess, "run", guarded) + + @pytest.fixture def symlink() -> Callable[[Path, Path], None]: """Return ``make(link, target)``, which skips the test if symlinks are denied. diff --git a/tests/test_eval_runs.py b/tests/test_eval_runs.py index 8bad748..5870ed6 100644 --- a/tests/test_eval_runs.py +++ b/tests/test_eval_runs.py @@ -6,9 +6,11 @@ import itertools import json +import os import re import shlex import shutil +import stat import subprocess import sys import textwrap @@ -168,12 +170,18 @@ def _record(workdir): @pytest.fixture def no_subprocess(monkeypatch): - """Fail the test if anything -- an agent, git, a version probe -- runs.""" + """Fail the test if anything but git -- an agent, a version probe -- runs. - def forbidden(cmd, **_): - raise AssertionError(f"unexpected subprocess: {cmd}") + Listing a fixture's files asks git (read-only); nothing else may run. + """ + real_run = subprocess.run - monkeypatch.setattr(run_evals.subprocess, "run", forbidden) + def only_git(cmd, **kwargs): + if cmd[0] != "git": + raise AssertionError(f"unexpected subprocess: {cmd}") + return real_run(cmd, **kwargs) + + monkeypatch.setattr(run_evals.subprocess, "run", only_git) monkeypatch.setattr( run_evals.tempfile, "mkdtemp", @@ -363,6 +371,110 @@ def broken(*_args): assert run["problems"][0].startswith("could not build the review repo") +def test_a_skill_that_cannot_install_is_recorded_and_the_rest_run( + run_main, monkeypatch, tmp_path +): + # the second fixture ships an unstamped file where the skill installs: + # the adapter refuses (SkillError); that run errors, the record survives + fixtures = _copy_fixture(tmp_path) + broken = fixtures / "logging-unstamped" + shutil.copytree(fixtures / "logging", broken) + skill_file = broken / "base" / ".claude" / "skills" / "logging" / "SKILL.md" + skill_file.parent.mkdir(parents=True) + skill_file.write_text("hand-written\n", encoding="utf-8") + monkeypatch.setattr(run_evals, "FIXTURES", fixtures) + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + + status, workdir = run_main("--skill", "logging", "--agent-cmd", agent) + assert status == 1 + _, record = _record(workdir) + assert schema_errors(record) == [] + first, second = (f["runs"][0] for f in record["fixtures"]) + assert first["status"] == "passed" + assert second["status"] == "error" + (problem,) = second["problems"] + assert problem.startswith("could not build the review repo: SkillError: ") + # paths are relative to the work dir, not absolute temp paths + assert "repos/run-1/logging-unstamped/.claude/skills/logging/SKILL.md" in ( + problem.replace("\\", "/") + ) + assert str(tmp_path) not in problem and tmp_path.as_posix() not in problem + + +def test_a_scoring_error_keeps_the_agent_outcome(run_main, monkeypatch, tmp_path): + def broken(*_args): + raise ValueError("scorer bug") + + monkeypatch.setattr(run_evals, "evaluate", broken) + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + status, workdir = run_main("--skill", "logging", "--agent-cmd", agent) + assert status == 1 + _, record = _record(workdir) + (run,) = record["fixtures"][0]["runs"] + assert run["status"] == "error" + assert run["problems"] == [ + "could not store or score the report: ValueError: scorer bug" + ] + assert run["exit_code"] == 0 and run["duration_s"] is not None + assert (workdir / run["artifacts"]["report"]).is_file() + + +def test_an_unexpected_runner_error_still_writes_the_record( + run_main, monkeypatch, tmp_path +): + real_attempt = run_evals.attempt_run + calls = [] + + def second_one_breaks(*args, **kwargs): + calls.append(args) + if len(calls) == 2: + raise RuntimeError("runner bug") + return real_attempt(*args, **kwargs) + + monkeypatch.setattr(run_evals, "attempt_run", second_one_breaks) + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + with pytest.raises(RuntimeError, match="runner bug"): + run_main("--skill", "logging", "--agent-cmd", agent, "--repeat", "3") + (workdir,) = tmp_path.glob("work-*") + _, record = _record(workdir) + assert schema_errors(record) == [] + assert record["status"] == "incomplete" + runs = record["fixtures"][0]["runs"] + assert [run["status"] for run in runs] == ["passed", "error", "not_run"] + assert runs[1]["problems"] == ["runner error: RuntimeError: runner bug"] + assert runs[2]["problems"] == ["not run: runner error: RuntimeError: runner bug"] + + +def test_an_interrupt_mid_plan_records_what_ran(run_main, monkeypatch, tmp_path): + real_attempt = run_evals.attempt_run + calls = [] + + def interrupted_second(*args, **kwargs): + calls.append(args) + if len(calls) == 2: + raise KeyboardInterrupt + return real_attempt(*args, **kwargs) + + monkeypatch.setattr(run_evals, "attempt_run", interrupted_second) + agent = _stand_in_agent(tmp_path, PASSING_AGENT) + status, workdir = run_main( + "--skill", "logging", "--agent-cmd", agent, "--repeat", "3" + ) + assert status == 130 + _, record = _record(workdir) + runs = record["fixtures"][0]["runs"] + assert [run["status"] for run in runs] == ["passed", "error", "not_run"] + assert runs[2]["problems"] == ["not run: interrupted"] + + +def test_describe_error_relativises_work_dir_paths(tmp_path): + workdir = tmp_path / "work" + exc = OSError(f"cannot write {workdir / 'repos' / 'x.py'} or {workdir}") + assert run_evals.describe_error(exc, workdir) == ( + f"OSError: cannot write {Path('repos') / 'x.py'} or ." + ) + + def test_the_schema_rejects_a_malformed_record(run_main, tmp_path): agent = _stand_in_agent(tmp_path, PASSING_AGENT) _, workdir = run_main("--skill", "logging", "--agent-cmd", agent) @@ -433,6 +545,81 @@ def test_fixture_digest_covers_content_and_paths_but_not_line_endings(tmp_path): assert run_evals.fixture_digest(path) != original +def test_fixture_digest_ignores_junk_files(tmp_path): + path = _copy_fixture(tmp_path) / "logging" + original = run_evals.fixture_digest(path) + cache = path / "change" / "auth" / "__pycache__" + cache.mkdir() + (cache / "session.cpython-312.pyc").write_bytes(b"\x00junk") + (path / "base" / ".DS_Store").write_bytes(b"\x00junk") + (path / "change" / "auth" / "stray.pyc").write_bytes(b"\x00junk") + assert run_evals.fixture_digest(path) == original + # and the review repo is built from the same files + names = {f.relative for f in run_evals.fixture_files(path)} + assert not any("pycache" in n or n.endswith((".pyc", ".DS_Store")) for n in names) + + +@pytest.mark.skipif(os.name == "nt", reason="Windows has no exec bit") +def test_fixture_digest_covers_the_exec_bit(tmp_path): + path = _copy_fixture(tmp_path) / "logging" + original = run_evals.fixture_digest(path) + session = path / "change" / "auth" / "session.py" + session.chmod(session.stat().st_mode | stat.S_IXUSR) + assert run_evals.fixture_digest(path) != original + + +def _git_in(repo, *args): + subprocess.run( + ["git", "-c", "user.name=t", "-c", "user.email=t@localhost", *args], + cwd=repo, + check=True, + capture_output=True, + ) + + +def test_fixture_digest_in_git_follows_what_git_tracks(tmp_path): + repo = tmp_path / "repo" + fixtures = repo / "fixtures" + shutil.copytree(run_evals.FIXTURES / "logging", fixtures / "logging") + path = fixtures / "logging" + (repo / ".gitignore").write_text("*.log\n", encoding="utf-8") + _git_in(repo, "init", "-q") + _git_in(repo, "add", "-A") + _git_in(repo, "commit", "-q", "-m", "fixture") + original = run_evals.fixture_digest(path) + # the same files outside git hash the same + assert original == run_evals.fixture_digest(run_evals.FIXTURES / "logging") + + (path / "change" / "debug.log").write_text("ignored\n", encoding="utf-8") + assert run_evals.fixture_digest(path) == original # ignored by git + new = path / "change" / "notes.txt" + new.write_text("untracked but not ignored\n", encoding="utf-8") + assert run_evals.fixture_digest(path) != original # being authored + new.unlink() + + # the exec bit comes from the index, so every platform agrees + _git_in(repo, "update-index", "--chmod=+x", "fixtures/logging/expected.yaml") + assert run_evals.fixture_digest(path) != original + + +def test_rendered_digest_is_the_install_stamp_hash(tmp_path): + fixture = run_evals.load_fixture(run_evals.FIXTURES / "logging") + for adapter in ("claude", "codex"): + repo = run_evals.prepare_repo(fixture, tmp_path / adapter, adapter) + installed = repo / run_evals.skill_path(fixture, adapter) + (stamp_hash,) = re.findall( + r"hash=([0-9a-f]{64}) -->", installed.read_text(encoding="utf-8") + ) + identity = run_evals.plan_fixture(fixture, adapter).identity + assert identity["skill"]["rendered_sha256"] == f"sha256:{stamp_hash}" + + +def test_commit_ids_may_be_sha1_or_sha256(): + assert run_evals._COMMIT_RE.fullmatch("a" * 40) + assert run_evals._COMMIT_RE.fullmatch("a" * 64) + assert not run_evals._COMMIT_RE.fullmatch("a" * 50) + + # -- harness presets ----------------------------------------------------------- @@ -483,6 +670,8 @@ def test_agent_cmd_and_adapter_override_a_preset(): ({"agent_cmd": "agent -m {model} {prompt}"}, "pass --model"), ({"agent_cmd": "agent '{prompt}"}, "No closing quotation"), ({"name": "claude", "model": " "}, "--model must not be empty"), + ({"name": "claude", "model": "--yolo"}, "looks like an option"), + ({"name": "claude", "agent_cmd": ""}, "the agent command is empty"), ({"name": "gemini"}, "unknown harness 'gemini'"), ], ) @@ -491,6 +680,124 @@ def test_invalid_harness_options_are_config_errors(kwargs, message): run_evals.resolve_harness(**kwargs) +def test_agents_and_probes_get_no_stdin(monkeypatch, tmp_path): + calls = [] + + def fake_run(cmd, **kwargs): + calls.append(kwargs) + return subprocess.CompletedProcess(cmd, 0, stdout="v1\n", stderr="") + + monkeypatch.setattr(run_evals.subprocess, "run", fake_run) + run_evals.run_agent("agent {prompt}", "p", tmp_path, 5) + run_evals.harness_version(["agent", "--version"]) + assert [call["stdin"] for call in calls] == [subprocess.DEVNULL] * 2 + + +def test_an_agent_reading_stdin_does_not_block_on_an_open_pipe(tmp_path): + # the runner's own stdin is a pipe that never closes (as under a CI + # step or a wrapper script); an agent that reads stdin -- codex exec + # does when it isn't a TTY -- must see EOF at once, not hang or ingest it + agent = _stand_in_agent( + tmp_path, "import sys\nprint(repr(sys.stdin.read()))\n", "reader.py" + ) + script = tmp_path / "runner.py" + script.write_text( + textwrap.dedent( + f"""\ + import importlib.util, pathlib, sys + spec = importlib.util.spec_from_file_location( + "run_evals", {str(ROOT / "evals" / "run_evals.py")!r} + ) + module = importlib.util.module_from_spec(spec) + sys.modules["run_evals"] = module + spec.loader.exec_module(module) + run = module.run_agent({agent!r}, "p", pathlib.Path("."), 20) + print(run.returncode, run.stdout.strip()) + """ + ), + encoding="utf-8", + ) + runner = subprocess.Popen( + [sys.executable, str(script)], + cwd=tmp_path, + stdin=subprocess.PIPE, + stdout=subprocess.PIPE, + stderr=subprocess.PIPE, + text=True, + encoding="utf-8", + ) + try: + runner.stdin.write("secret piped input\n") + runner.stdin.flush() + # stdin stays open while the runner works + assert runner.wait(timeout=60) == 0, runner.stderr.read() + assert runner.stdout.read().strip() == "0 ''" + finally: + runner.stdin.close() + if runner.poll() is None: + runner.kill() + runner.stdout.close() + runner.stderr.close() + + +@pytest.mark.parametrize( + ("name", "command", "probe"), + [ + ("claude", "claude -p {prompt}", ["claude", "--version"]), + ( + "claude", + "/opt/bin/claude -p {prompt}", + ["/opt/bin/claude", "--version"], + ), + ("codex", "codex.exe exec {prompt}", ["codex.exe", "--version"]), + ( + "codex", + r"C:\\tools\\CODEX.CMD exec {prompt}", + [r"C:\tools\CODEX.CMD", "--version"], + ), + # a wrapper would credit its own version to the harness + ("claude", "env FOO=1 claude -p {prompt}", None), + ("codex", "npx @openai/codex exec {prompt}", None), + ("codex", "claude -p {prompt}", None), + ("custom", "claude -p {prompt}", None), + ], +) +def test_only_the_presets_own_executable_is_probed(name, command, probe): + harness = run_evals.Harness(name, command, "claude") + assert harness.version_command == probe + + +@pytest.mark.skipif(os.name == "nt", reason="needs an executable script") +def test_a_preset_records_the_version_its_executable_reports(run_main, tmp_path): + # a stand-in named like the preset's CLI, run by absolute path (never via + # PATH, where the real CLI may be): its --version answer is recorded + stand_in = tmp_path / "bin" / "codex" + stand_in.parent.mkdir() + stand_in.write_text( + f"#!{sys.executable}\n" + + textwrap.dedent( + """\ + import sys + if sys.argv[1:] == ["--version"]: + print("codex-cli 9.9.9-stand-in") + raise SystemExit(0) + assert sys.argv[1] == "exec" + print("Reviewed main..HEAD: no findings.") + """ + ), + encoding="utf-8", + ) + stand_in.chmod(0o755) + agent = f"{shlex.quote(str(stand_in))} exec {{prompt}}" + status, workdir = run_main( + "--skill", "logging", "--harness", "codex", "--agent-cmd", agent + ) + assert status == 1 # an empty review misses the plant + _, record = _record(workdir) + assert record["harness"]["version"] == "codex-cli 9.9.9-stand-in" + assert record["harness"]["version_command"] == [str(stand_in), "--version"] + + def test_harness_version_is_best_effort(): version = run_evals.harness_version([sys.executable, "--version"]) assert version is not None and version.startswith("Python ") @@ -500,10 +807,9 @@ def test_harness_version_is_best_effort(): assert run_evals.harness_version(None) is None -def test_a_preset_harness_records_its_version_and_model(run_main, tmp_path, capsys): +def test_a_preset_harness_records_its_model(run_main, tmp_path, capsys): # --harness codex with a stand-in in its place: the codex adapter installs - # the skill, the model reaches the command, and the version probe runs - # the command's own executable + # the skill and the model reaches the command agent = _stand_in_agent( tmp_path, """\ @@ -533,8 +839,9 @@ def test_a_preset_harness_records_its_version_and_model(run_main, tmp_path, caps assert record["adapter"] == "codex" assert record["harness"]["name"] == "codex" assert record["harness"]["model"] == "tiny-model" - assert record["harness"]["version_command"] == [sys.executable, "--version"] - assert record["harness"]["version"].startswith("Python ") + # the command runs python, not codex: no version is credited to codex + assert record["harness"]["version_command"] is None + assert record["harness"]["version"] is None (run,) = record["fixtures"][0]["runs"] assert run["status"] == "failed" assert ".agents/skills/logging/SKILL.md" in record["fixtures"][0]["prompt"] @@ -644,7 +951,7 @@ def _first_record(run_main, tmp_path, agent_body=PASSING_AGENT, *extra): def test_replay_reruns_the_recorded_configuration(run_main, tmp_path): path = _first_record(run_main, tmp_path, PASSING_AGENT, "--repeat", "2") original = json.loads(path.read_text(encoding="utf-8")) - status, workdir = run_main("--replay", str(path)) + status, workdir = run_main("--replay", str(path), "--trust-record-command") assert status == 0 _, replayed = _record(workdir) assert schema_errors(replayed) == [] @@ -676,7 +983,7 @@ def test_replay_refuses_a_changed_fixture(run_main, monkeypatch, tmp_path, capsy ) path.write_text(json.dumps(record), encoding="utf-8") - status, workdir = run_main("--replay", str(path)) + status, workdir = run_main("--replay", str(path), "--trust-record-command") assert status == 2 assert workdir is None # refused before a work dir, let alone an agent assert not marker.exists() @@ -685,16 +992,18 @@ def test_replay_refuses_a_changed_fixture(run_main, monkeypatch, tmp_path, capsy ) -def _minimal_record(tmp_path, **skill_changes): +def _minimal_record(tmp_path, harness=None, prompt=None, **skill_changes): """A hand-written record of the logging fixture's current identity.""" fixture = run_evals.load_fixture(run_evals.FIXTURES / "logging") - identity = run_evals.plan_fixture(fixture, "claude").identity + planned = run_evals.plan_fixture(fixture, "claude") + identity = {**planned.identity, "prompt": prompt or planned.prompt} identity["skill"].update(skill_changes) record = { "schema_version": 1, "record_type": "skilldeck-eval-run", "skilldeck": {"runner_sha256": None}, - "harness": { + "harness": harness + or { "name": "claude", "command": "claude -p {prompt}", "model": None, @@ -718,7 +1027,7 @@ def _minimal_record(tmp_path, **skill_changes): ), ( {"rendered_sha256": "sha256:" + "0" * 64}, - "logging: installed skill file changed since the record", + "logging: rendered skill changed since the record", ), ({"version": "9.9.9"}, "logging: skill version changed since the record"), ], @@ -737,6 +1046,66 @@ def test_replay_dry_run_verifies_digests_and_plans(no_subprocess, tmp_path, caps out = capsys.readouterr().out assert "plan: 1 fixture(s) x 1 repeat(s) = 1 run(s)" in out assert "harness: claude (adapter claude, model harness default)" in out + # the hand-written record names no runner digest: the drift is noted + assert "note: the eval runner (scorer or prompt) changed" in out + + +def test_replay_refuses_a_changed_prompt(no_subprocess, tmp_path, capsys): + path = _minimal_record(tmp_path, prompt="Review it with some other wording.") + assert run_evals.main(["--replay", str(path), "--dry-run"]) == 2 + assert "logging: the review prompt changed since the record" in ( + capsys.readouterr().err + ) + + +@pytest.mark.parametrize( + "harness", + [ + # a preset's name on a command that isn't the preset's + {"name": "claude", "command": "sh -c 'touch PWNED' {prompt}"}, + {"name": "codex", "command": "claude -p {prompt}"}, + # a custom command is always the record's own + {"name": "custom", "command": "my-agent {prompt}"}, + ], + ids=["claude-name", "codex-name", "custom"], +) +def test_replay_refuses_a_command_that_is_not_the_preset( + no_subprocess, tmp_path, capsys, harness +): + harness = {**harness, "model": None, "version": None} + path = _minimal_record(tmp_path, harness=harness) + assert run_evals.main(["--replay", str(path), "--dry-run"]) == 2 + err = capsys.readouterr().err + assert f"harness command {harness['command']!r} is not the built-in" in err + assert "--trust-record-command" in err + # an explicit opt-in accepts it (the dry run still runs nothing) + argv = ["--replay", str(path), "--dry-run", "--trust-record-command"] + assert run_evals.main(argv) == 0 + assert f"command: {harness['command']}" in capsys.readouterr().out + + +@pytest.mark.parametrize( + ("name", "model", "command"), + [ + ("claude", None, "claude -p {prompt}"), + ("claude", "opus", "claude --model {model} -p {prompt}"), + ("codex", None, "codex exec {prompt}"), + ("codex", "gpt-5", "codex exec --model {model} {prompt}"), + ], +) +def test_replay_accepts_a_preset_command(no_subprocess, tmp_path, name, model, command): + harness = {"name": name, "command": command, "model": model, "version": None} + path = _minimal_record(tmp_path, harness=harness) + spec = run_evals.load_replay(path) + assert spec.harness.command == command + assert spec.harness.model == model + + +def test_trust_record_command_needs_replay(capsys): + assert run_evals.main(["--trust-record-command", "--dry-run"]) == 2 + assert "--trust-record-command only applies to --replay" in ( + capsys.readouterr().err + ) def test_replay_takes_its_configuration_only_from_the_record(tmp_path, capsys): @@ -756,6 +1125,10 @@ def test_replay_takes_its_configuration_only_from_the_record(tmp_path, capsys): '{"record_type": "skilldeck-eval-run", "schema_version": 99}', "unsupported schema_version 99", ), + ( + '{"record_type": "skilldeck-eval-run", "schema_version": true}', + "unsupported schema_version True", + ), ], ) def test_replay_rejects_an_unreadable_record(tmp_path, capsys, text, message): @@ -770,7 +1143,8 @@ def test_replay_refuses_a_fixture_that_no_longer_exists(run_main, tmp_path, caps record = json.loads(path.read_text(encoding="utf-8")) record["fixtures"][0]["name"] = "../logging" path.write_text(json.dumps(record), encoding="utf-8") - assert run_evals.main(["--replay", str(path)]) == 2 + argv = ["--replay", str(path), "--trust-record-command"] + assert run_evals.main(argv) == 2 assert "fixture '../logging' from the record no longer exists" in ( capsys.readouterr().err ) From 5e39da340516d73389ca7a8fbbfef5dc1c125802 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 24 Sep 2026 09:48:51 +0000 Subject: [PATCH 3/3] Reuse stamp.content_hash for the eval rendered digest Now that main has skilldeck.stamp.content_hash (#131), the run record's rendered_sha256 comes from it instead of a local copy of the formula, and a test checks it against both the installed file's stamp and skilldeck catalog's rendered_sha256 for the claude and codex adapters. Part of #74. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01HtiCGzpikMrkDYBkfQG5CX --- evals/README.md | 3 ++- evals/run_evals.py | 10 ++++------ tests/test_eval_runs.py | 7 +++++++ 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/evals/README.md b/evals/README.md index b4e8631..9841aa0 100644 --- a/evals/README.md +++ b/evals/README.md @@ -119,7 +119,8 @@ platform: exactly those files. Then the skill's name, version, canonical digest (the one in `src/skilldeck/_content_manifest.json`) and `rendered_sha256`: the rendered skill content the adapter installs, excluding the install stamp - (it equals the stamp's `hash=`); plus the exact prompt; + (it equals the stamp's `hash=` and `skilldeck catalog`'s + `rendered_sha256` for that adapter); plus the exact prompt; - **how**: the harness name, the exact command template, the model (if requested), the harness version (the probe's first line, `null` if it failed), the adapter, and the repeat, timeout and budget; diff --git a/evals/run_evals.py b/evals/run_evals.py index dfd7660..47a92ca 100644 --- a/evals/run_evals.py +++ b/evals/run_evals.py @@ -84,6 +84,7 @@ sha256_text, ) from skilldeck.registry import Skill, discover_skills # noqa: E402 +from skilldeck.stamp import content_hash # noqa: E402 from skilldeck.targets import Scope # noqa: E402 FIXTURES = ROOT / "evals" / "fixtures" @@ -1054,13 +1055,10 @@ def fixture_layout_problems(fixture: Fixture) -> list[str]: def rendered_digest(adapter: str, skill: Skill) -> str: """The hash an install stamp records: the rendered skill, stamp excluded. - Computed as skilldeck.stamp does -- the content, newline-terminated, as - UTF-8 -- so it equals the installed file's ``hash=``. + It equals the installed file's ``hash=`` and ``skilldeck catalog``'s + ``rendered_sha256`` for the same adapter. """ - content = ADAPTERS[adapter].render(skill) - if not content.endswith("\n"): - content += "\n" - return "sha256:" + hashlib.sha256(content.encode("utf-8")).hexdigest() + return "sha256:" + content_hash(ADAPTERS[adapter].render(skill)) def plan_fixture(fixture: Fixture, adapter: str) -> PlannedFixture: diff --git a/tests/test_eval_runs.py b/tests/test_eval_runs.py index 5870ed6..5d8fec6 100644 --- a/tests/test_eval_runs.py +++ b/tests/test_eval_runs.py @@ -20,7 +20,10 @@ import run_evals # loaded from evals/run_evals.py by conftest.py from skilldeck import __version__ +from skilldeck.adapters import ADAPTERS +from skilldeck.catalog import build_catalog from skilldeck.provenance import canonical_json, sha256_text +from skilldeck.registry import discover_skills ROOT = Path(__file__).resolve().parent.parent SCHEMA = json.loads( @@ -604,6 +607,7 @@ def test_fixture_digest_in_git_follows_what_git_tracks(tmp_path): def test_rendered_digest_is_the_install_stamp_hash(tmp_path): fixture = run_evals.load_fixture(run_evals.FIXTURES / "logging") + catalog = build_catalog(discover_skills(known_agents=set(ADAPTERS))) for adapter in ("claude", "codex"): repo = run_evals.prepare_repo(fixture, tmp_path / adapter, adapter) installed = repo / run_evals.skill_path(fixture, adapter) @@ -612,6 +616,9 @@ def test_rendered_digest_is_the_install_stamp_hash(tmp_path): ) identity = run_evals.plan_fixture(fixture, adapter).identity assert identity["skill"]["rendered_sha256"] == f"sha256:{stamp_hash}" + # the same value skilldeck catalog publishes for the adapter + (entry,) = [e for e in catalog["skills"] if e["name"] == "logging"] + assert entry["rendered_sha256"][adapter] == f"sha256:{stamp_hash}" def test_commit_ids_may_be_sha1_or_sha256():