diff --git a/.github/workflows/selftest.yml b/.github/workflows/selftest.yml index 5a68438..adb4bf2 100644 --- a/.github/workflows/selftest.yml +++ b/.github/workflows/selftest.yml @@ -44,6 +44,116 @@ jobs: - name: Run the suite run: python -m unittest discover -s tests -t . --verbose + engine: + name: Inspect engine (${{ matrix.os }}) + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + # Both sandboxes a run can get. Ubuntu has Docker, which is the default + # and what CI uses; Windows has none, which is the other supported + # shape. The Windows leg is the only place the cross-platform tools are + # exercised at all -- inspect's own assume a POSIX guest. + os: [ubuntu-latest, windows-latest] + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-python@v5 + with: + python-version: "3.12" + # inspect-ai pulls in the order of eighty packages, so the download + # is most of this step. Keyed on pyproject, which is where the extra + # is declared and the only thing that changes what gets installed. + cache: pip + cache-dependency-path: pyproject.toml + + - name: Install with the inspect extra + run: python -m pip install --upgrade pip && python -m pip install ".[inspect]" + + # `mockllm` reaches no provider, so this needs no key and runs on a pull + # request from a fork. It cannot tell us whether a skill is any good -- + # the mock does no work and every judged expectation fails. What it does + # tell us is whether the machinery around the model still holds together + # on both platforms, which is where every bug so far has been. + - name: Build a repo to test + shell: bash + run: | + set -euo pipefail + mkdir -p fixture/demo-skill/evals fixture/demo-skill/fixtures + cat > fixture/demo-skill/SKILL.md <<'EOF' + --- + name: demo-skill + description: Does demonstrable things, for a test that needs a skill. + --- + EOF + echo "seeded by the case, not produced by the agent" \ + > fixture/demo-skill/fixtures/seeded.txt + cat > fixture/demo-skill/evals/evals.json <<'EOF' + { + "evaluations": [ + {"id": "demo-a", "skill_should_trigger": true, "prompt": "do the demo thing", + "workspace": "fixtures", "files_exist": ["seeded.txt"]}, + {"id": "demo-b", "skill_should_trigger": true, "prompt": "another way to ask"}, + {"id": "demo-c", "skill_should_trigger": true, "prompt": "a third phrasing"}, + {"id": "demo-d", "skill_should_trigger": false, "prompt": "something adjacent"}, + {"id": "demo-e", "skill_should_trigger": false, "prompt": "something else entirely"} + ] + } + EOF + + # `SKILLSCOPE_REPO` rather than `working-directory`, because a `--skills-dir` + # glob that was passed is resolved against the repo root and `find_root` + # takes that from the nearest `.git`. Running from a subdirectory of a + # checkout would therefore still glob the checkout. Same thing `repo:` + # does for the action. + # + # Exits non-zero because the mock satisfies nothing; the report is the + # artifact under test, not the exit code. + - name: Grade the fixture on the inspect engine + continue-on-error: true + env: + SKILLSCOPE_REPO: fixture + run: > + python -m skillscope behavioral --engine inspect + --model mockllm/model --skills-dir '*' --skill demo-skill + --output engine-report.json + + - name: Check the machinery held + shell: python + env: + EXPECTED_SANDBOX: ${{ matrix.os == 'windows-latest' && 'local' || 'docker' }} + run: | + import json + import os + + report = json.load(open("engine-report.json", encoding="utf-8")) + meta, totals = report["meta"], report["totals"] + print(json.dumps(meta, indent=2)) + + # An infrastructure failure is not a graded result. This is what + # catches a sandbox that would not start. + assert totals["errors"] == 0, report["cases"] + + expected = os.environ["EXPECTED_SANDBOX"] + assert meta["sandbox"] == expected, f"ran in {meta['sandbox']!r}, wanted {expected!r}" + + checks = [c for case in report["cases"] for c in case["checks"]] + assert checks, "nothing was graded, so nothing was proven" + + # A sandbox that cannot be listed reports the same shape as an agent + # that produced nothing, so the difference is asserted explicitly. + broken = [c for c in checks if "could not list the sandbox" in (c["detail"] or "")] + assert not broken, broken + + # The seeded file exists because the case put it there, not because + # the mock did anything. It passing proves the whole path: the fixture + # was staged into the working directory, the directory was listed, and + # the listing was matched against what the case asked for. + seeded = [c for c in checks if c["kind"] == "files_exist"] + assert seeded, "the seeded-file check did not run" + assert all(c["passed"] for c in seeded), seeded + print(f"{len(checks)} checks graded, seeded fixture found, sandbox {expected}.") + action: name: Action against a throwaway repo runs-on: ubuntu-latest diff --git a/docs/usage.md b/docs/usage.md index 68164ef..8a47d86 100644 --- a/docs/usage.md +++ b/docs/usage.md @@ -208,6 +208,69 @@ Legs with a scoped environment run as a separate job, because a job's credentials are fixed before its matrix expands. A repo that declares no scoped environment gets one matrix, labels and all. +## Which engine grades a run + +`--engine` chooses what actually runs the cases. The dataset, the CLI and the +reports are identical whichever you pick; only the thing driving the agent +changes. + +| `--engine` | What runs | Needs | +| --- | --- | --- | +| `legacy` (default) | The `claude` CLI, driven directly | the CLI on `PATH` | +| `inspect` | A harness-independent agent through `inspect_ai` | `pip install 'skillscope[inspect]'` | +| `claude-code` | Real Claude Code inside the sandbox, to cross-check the other two | `skillscope[verify]`, Linux only | + +`inspect` grades a skill on whether its *instructions* work rather than on how +one product reads them, which is the stronger claim and the one a product repo +can adopt. It is also much cheaper: a routing case is a single model call, +because the decision is visible in the first reply and nothing needs executing. + +`claude-code` is a reporting leg, never a gate. Harness runs are +nondeterministic and the harness is not what is being graded, so a divergence +there is a question about the skill rather than a build failure. + +### Where an `inspect` run is sandboxed + +Two separate decisions, made by different people. + +**Which provider** is a property of the runner, chosen with +`SKILLSCOPE_SANDBOX`. Docker by default; `podman` on a host that has that +instead; `local` to skip the container. `local` is for working locally rather +than for CI, because a graded run that quietly dropped its sandbox would report +the same numbers with none of the isolation. + +Podman needs three things, and each was discovered by the next one failing: + +* `pip install 'skillscope[podman]'`. The provider is registered by a separate + package through an entry point, so the podman binary alone is not enough. +* `podman-compose`, and `INSPECT_PODMAN_COMPOSE=podman-compose`. Bare + `podman compose` is a shim that delegates to whichever compose provider it + finds, which on a host that also has Docker is Docker's -- and that then + talks to a daemon podman was chosen to avoid. +* A search registry, because podman will not guess one. Docker assumes Docker + Hub for an image name with no registry; podman refuses, and the default + sandbox image is named without one. `unqualified-search-registries = + ["docker.io"]` in `/etc/containers/registries.conf`. + +Podman is worth the setup where the runner's user cannot reach the Docker +socket, since it is daemonless and rootless and needs neither that nor group +membership. + +**What the sandbox must provide** is a property of the skill, declared as +`sandbox: compose.yaml` in its `evals/machine.yml`. Skills get a container with +no network by default; one that installs a server or pulls a model cannot run +that way and says so. Selecting a provider does not discard what a skill asked +for -- the compose file rides along. + +Windows is the exception to both: inspect's sandbox layer and every tool built +on it assume a POSIX guest, so those legs run unsandboxed and trade isolation +for running on the platform they are meant to test. + +To see what changing engine would do to your own datasets before changing it, +[`tools/benchmark_engines.py`](../tools/benchmark_engines.py) runs the same +cases through two engines and reports per-case agreement, measured against how +much one engine already disagrees with itself. + ## In CI: one job [`reusable.yml`](../.github/workflows/reusable.yml) grades a repo's skills with diff --git a/pyproject.toml b/pyproject.toml index d05674d..e849eb6 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -15,16 +15,34 @@ requires-python = ">=3.10" license = { text = "MIT" } authors = [{ name = "Advanced Micro Devices, Inc." }] -# The runner itself is standard library only, so a run needs no wheels beyond -# this package. PyYAML is the one exception: it reads the optional +# The legacy runner is standard library only, so a graded run needs no wheels +# beyond this package. PyYAML is the one exception: it reads the optional # evals/machine.yml, which only CI planning touches. dependencies = ["pyyaml>=6.0"] +# The inspect-backed engine (`--engine inspect`). Kept an extra so the default +# path keeps the stdlib-only property while both engines are shipped: a repo +# that has not migrated installs nothing new. +[project.optional-dependencies] +# `anthropic` is listed explicitly: inspect-ai treats every model provider as +# optional, so installing it alone gets you a harness that cannot reach a model. +inspect = ["inspect-ai>=0.3.263", "anthropic>=0.40"] + +# For a runner that has podman rather than docker. The provider registers +# itself through an `inspect_ai` entry point, so installing it is the whole +# setup; `SKILLSCOPE_SANDBOX=podman` then selects it. +podman = ["skillscope[inspect]", "inspect-podman"] + +# The Claude Code verification leg (`--engine claude-code`). Separate from +# `inspect` because it is a reporting-only cross-check, not something a graded +# run needs -- and because it only works on a POSIX guest. +verify = ["skillscope[inspect]", "inspect-swe>=0.2.70"] + [project.scripts] skillscope = "skillscope.cli:main" [tool.setuptools] -packages = ["skillscope"] +packages = ["skillscope", "skillscope.engine"] [tool.setuptools.package-data] skillscope = ["data/*.json", "schema/*.json"] diff --git a/skillscope/agent.py b/skillscope/agent.py index 53bce6d..d5ebca5 100644 --- a/skillscope/agent.py +++ b/skillscope/agent.py @@ -41,7 +41,7 @@ from dataclasses import dataclass from pathlib import Path -from . import datasets, deadline +from . import datasets, deadline, usage DEFAULT_MODEL = os.environ.get("SKILLSCOPE_MODEL", "opus") DEFAULT_EFFORT = os.environ.get("SKILLSCOPE_EFFORT", "high") @@ -70,10 +70,18 @@ def is_automated_env() -> bool: ) +# Model providers that reach no cloud service. The CI pin exists to keep paid +# runs comparable between runs; one of these grades nothing and costs nothing, +# so pinning it only turns a free wiring check into a run that needs a key. +NO_PROVIDER_PREFIXES = ("mockllm",) + + def enforce_model_policy(model: str | None) -> str | None: """Coerce non-opus models to opus in CI; pass through otherwise.""" if model is None or not is_automated_env() or "opus" in model.lower(): return model + if model.lower().startswith(NO_PROVIDER_PREFIXES): + return model _safe_print( f"[skillscope] automated run: coercing model '{model}' -> " f"'{AUTOMATED_MODEL}' to pin the CI model." @@ -387,6 +395,9 @@ def __init__(self, *, workspace: Path, events: list[dict], judge_model: str | No result_text = "" for ev in events: + # Recording what the run spent is what lets it be compared against + # the same cases on the other engine. + usage.record_stream_event(ev) if ev.get("type") == "result" and isinstance(ev.get("result"), str): result_text = ev["result"] diff --git a/skillscope/behavior.py b/skillscope/behavior.py index 154f48c..cac3a15 100644 --- a/skillscope/behavior.py +++ b/skillscope/behavior.py @@ -210,6 +210,25 @@ def summarize(outcomes: list[BehaviorOutcome], meta: dict) -> dict: } +def _isolation_note(meta: dict) -> str: + """One line saying whether the agent was contained while it worked. + + Behavioral runs the agent to completion with permissions bypassed, so + whether it was isolated changes what the numbers cost to obtain. A report + that omits it reads as though it were, and the answer differs per platform: + the Windows legs have no sandbox available at all. + """ + where = meta.get("sandbox") + if where is None: + return "" + if meta.get("sandbox_isolated"): + return f"Cases ran isolated, in `{where}`." + return ( + f"**Cases ran unsandboxed** (`{where}`): the agent worked directly in " + "the harness's own filesystem, with permissions bypassed." + ) + + def render_markdown(summary: dict) -> str: totals = summary["totals"] meta = summary["meta"] @@ -220,6 +239,8 @@ def render_markdown(summary: dict) -> str: f"({totals['checks_passed']}/{totals['checks']} individual expectations) " f"on `{meta['model']}` (effort `{meta['effort']}`).", "", + _isolation_note(meta), + "", "| Skill | Cases | Passed | Expectations | Met |", "| --- | --- | --- | --- | --- |", ] diff --git a/skillscope/cli.py b/skillscope/cli.py index f9d93b4..182b3de 100644 --- a/skillscope/cli.py +++ b/skillscope/cli.py @@ -72,7 +72,17 @@ from concurrent.futures import ThreadPoolExecutor from pathlib import Path -from . import behavior, config, datasets, deadline, references, routing, structure +from . import ( + behavior, + config, + datasets, + deadline, + engine, + references, + routing, + structure, + usage, +) from . import selection as select_module from .agent import check_api_reachable, enforce_model_policy @@ -334,6 +344,19 @@ def _prepare_graded_run( selected = _selected_skills(args.skill) _structural_or_exit(selected if scope is None else sorted(set(scope))) args.model = enforce_model_policy(args.model) or args.model + if getattr(args, "engine", "legacy") in ("inspect", "claude-code"): + # The CLI-based reachability probe tests something these engines do not + # use, but they still need one of their own: a graded run starts + # containers and installs skills before it first reaches a provider, so + # without this a bad key surfaces as a task that failed after all that. + engine.require() + if not args.skip_preflight: + from .engine import models as engine_models + + ok, detail = engine_models.check_reachable(engine_models.resolve(args.model)) + if not ok: + raise SystemExit(f"error: model not reachable -- {detail}") + return selected if not args.skip_preflight: ok, detail = check_api_reachable(args.model) if not ok: @@ -341,6 +364,67 @@ def _prepare_graded_run( return selected +def _sandbox_meta(args: argparse.Namespace) -> dict: + """What contained this run, recorded so the report does not have to imply it. + + The legacy engine runs the agent on the host with permissions bypassed, and + saying so in the artifact is the point: the same numbers mean different + things depending on whether anything was isolated. + """ + if getattr(args, "engine", "legacy") == "legacy": + return {"sandbox": "host", "sandbox_isolated": False} + from .engine import sandbox as engine_sandbox + + return engine_sandbox.describe() + + +def _finish_routing( + args: argparse.Namespace, + outcomes: list, + routing_set: dict, + started: float, + *, + isolated: bool, + extra: dict | None = None, +) -> int: + """Summarize, report, and gate a routing run. Shared by both engines.""" + summary = routing.summarize( + outcomes, + list(routing_set), + { + "model": args.model, + "engine": args.engine, + "effort": args.effort, + "skills": list(routing_set), + "extended": args.extended, + "wall_time_s": round(time.time() - started, 1), + "timeout": args.timeout, + "isolated_config_dir": isolated, + "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), + # Routing under the inspect engine executes nothing -- the skill + # tool is offered and never called -- so there is no sandbox and + # nothing to isolate. Saying "none" is not the same as saying the + # run was unprotected. + **( + {"sandbox": "none", "sandbox_isolated": None} + if args.engine == "inspect" + else _sandbox_meta(args) + ), + **(extra or {}), + }, + ) + _write_report(summary, routing.render_markdown(summary), args, "routing") + + if (code := _fail_if_expired()) is not None: + return code + reason = routing_gate(summary["totals"], args.min_accuracy) + if reason: + print(f"[routing] {reason}", file=sys.stderr) + return 1 + return 0 + + def cmd_routing(args: argparse.Namespace) -> int: # Who is in the room decides what the structural gate covers, so it is # settled before anything is checked or any token is spent. @@ -362,6 +446,15 @@ def cmd_routing(args: argparse.Namespace) -> int: elif args.skill: cases = datasets.filter_cases(cases, args.skill) + if args.engine == "inspect": + from .engine import models as engine_models + from .engine import routing as inspect_routing + + outcomes = inspect_routing.run( + cases, routing_set, engine_models.resolve(args.model) + ) + return _finish_routing(args, outcomes, routing_set, started, isolated=True) + routing_config = routing.RoutingConfig( model=args.model, effort=args.effort, @@ -392,34 +485,20 @@ def cmd_routing(args: argparse.Namespace) -> int: else: outcomes = [routing.run_case(case, routing_set, routing_config) for case in cases] - summary = routing.summarize( + return _finish_routing( + args, outcomes, - list(routing_set), - { - "model": args.model, - "effort": args.effort, - "skills": list(routing_set), - "extended": args.extended, - "wall_time_s": round(time.time() - started, 1), - "timeout": args.timeout, + routing_set, + started, + isolated=routing_config.isolate_config, + extra={ "case_timeout": args.case_timeout, "max_tool_calls": args.max_tool_calls, "max_inspection_calls": args.max_inspection_calls, - "isolated_config_dir": routing_config.isolate_config, "max_budget_usd": args.max_budget_usd, "optional_cli_flags_used": sorted(routing_config.available_flags), - "github_run_id": os.environ.get("GITHUB_RUN_ID"), }, ) - _write_report(summary, routing.render_markdown(summary), args, "routing") - - if (code := _fail_if_expired()) is not None: - return code - reason = routing_gate(summary["totals"], args.min_accuracy) - if reason: - print(f"[routing] {reason}", file=sys.stderr) - return 1 - return 0 def cmd_behavioral(args: argparse.Namespace) -> int: @@ -442,17 +521,33 @@ def cmd_behavioral(args: argparse.Namespace) -> int: ) return 0 - outcomes = behavior.run(skills, gradable, args.model, args.effort) + if args.engine in ("inspect", "claude-code"): + from .engine import models as engine_models + + if args.engine == "inspect": + from .engine import behavioral as runner + else: + from .engine import verify as runner + + outcomes = runner.run( + skills, gradable, engine_models.resolve(args.model), args.effort + ) + else: + outcomes = behavior.run(skills, gradable, args.model, args.effort) + summary = behavior.summarize( outcomes, { "model": args.model, + "engine": args.engine, "effort": args.effort, "skills": skills, "extended": args.extended, "wall_time_s": round(time.time() - started, 1), "timeout": args.timeout, "github_run_id": os.environ.get("GITHUB_RUN_ID"), + **usage.snapshot().as_meta(), + **_sandbox_meta(args), }, ) _write_report(summary, behavior.render_markdown(summary), args, "behavioral") @@ -565,6 +660,17 @@ def _add_graded_arguments(parser: argparse.ArgumentParser) -> None: parser.add_argument( "--skip-preflight", action="store_true", help="Skip the API reachability check." ) + parser.add_argument( + "--engine", + default=os.environ.get("SKILLSCOPE_ENGINE", "legacy"), + choices=["legacy", "inspect", "claude-code"], + help=( + "Which eval engine runs the cases. `legacy` drives the claude CLI " + "directly; `inspect` runs a harness-independent agent through " + "inspect_ai (needs `pip install 'skillscope[inspect]'`). Default: " + "legacy, or $SKILLSCOPE_ENGINE." + ), + ) _add_timeout_argument(parser) diff --git a/skillscope/datasets.py b/skillscope/datasets.py index 17a75b8..a8ada70 100644 --- a/skillscope/datasets.py +++ b/skillscope/datasets.py @@ -535,7 +535,7 @@ def tier0_errors(skill: str, cases: list[Case]) -> list[str]: return errors -MACHINE_KEYS = {"os", "labels"} +MACHINE_KEYS = {"os", "labels", "sandbox"} def _read_machine(skill: str) -> dict: @@ -573,11 +573,17 @@ def machine_plan(skill: str) -> dict: An absent ``evals/machine.yml`` is the common case: the everyday runners, on the platforms the repo runs on by default. A skill ships one to drop a - platform it cannot support (``os``) or to ask for a runner label its work - requires (``labels``):: + platform it cannot support (``os``), to ask for a runner label its work + requires (``labels``), or to name a compose file for the sandbox its cases + need (``sandbox``, read by the inspect engine):: os: [Linux] labels: [mi300x] + sandbox: compose.yaml + + ``sandbox`` is how a skill that must reach the network to pull a model, or + that needs a device bound in, opts out of the default no-network container + instead of every skill paying for what one of them needs. Labels rather than a class name, because a class name has to be defined somewhere and that somewhere is a second file to keep in step. A label is diff --git a/skillscope/engine/__init__.py b/skillscope/engine/__init__.py new file mode 100644 index 0000000..13b2207 --- /dev/null +++ b/skillscope/engine/__init__.py @@ -0,0 +1,38 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The inspect-backed eval engine (``--engine inspect``). + +The legacy engine drives the `claude` CLI directly; this one hands the work to +`inspect_ai`. Both produce the same outcome objects, so everything downstream -- +`summarize`, `render_markdown`, the report writers -- is shared. + +`inspect_ai` is an optional dependency, so nothing here is imported at module +scope by the rest of the package. Call `require()` before touching a submodule +to turn a missing wheel into an actionable message rather than a traceback. +""" + +from __future__ import annotations + +INSTALL_HINT = ( + "error: --engine inspect needs the inspect extra. Install it with:\n" + " pip install 'skillscope[inspect]'" +) + + +def require() -> None: + """Raise SystemExit with an install hint when `inspect_ai` is missing.""" + try: + import inspect_ai # noqa: F401 + except ModuleNotFoundError as exc: # pragma: no cover -- environment shape + raise SystemExit(INSTALL_HINT) from exc + + +def available() -> bool: + """Whether the inspect extra is installed (for diagnostics, not control flow).""" + try: + import inspect_ai # noqa: F401 + except ModuleNotFoundError: + return False + return True diff --git a/skillscope/engine/behavioral.py b/skillscope/engine/behavioral.py new file mode 100644 index 0000000..8ac6afd --- /dev/null +++ b/skillscope/engine/behavioral.py @@ -0,0 +1,178 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Behavioral evals on the inspect engine. + +`run()` matches `behavior.run()` -- same arguments, same `BehaviorOutcome` +list -- so swapping engines is a one-line substitution in the CLI and every +report path downstream is untouched. +""" + +from __future__ import annotations + +from pathlib import Path + +from .. import agent, config, deadline, usage +from ..behavior import BehaviorOutcome +from ..datasets import Case +from . import convert, models, sandbox as sandbox_spec, scorers, stats, tools + +# An agent that never decides it is finished must still stop. The legacy engine +# bounded this with `--case-timeout` and a process kill; inspect expresses it +# declaratively, and a message cap catches the loop a wall-clock cap only ends +# after paying for it. +MESSAGE_LIMIT = 120 + +# A model that reaches no provider never calls the submit tool, so it loops to +# whatever cap it is given -- and every turn is a real sandbox round trip. The +# wiring run proves the machinery in a handful of turns; the rest is the mock +# failing to finish, slowly. +MOCK_MESSAGE_LIMIT = 6 + + +def message_limit_for(model: str) -> int: + """How many turns this model should be allowed before the case is stopped.""" + if model.lower().startswith(agent.NO_PROVIDER_PREFIXES): + return MOCK_MESSAGE_LIMIT + return MESSAGE_LIMIT + + +def _tools(skill_dir: Path) -> list: + """Tools the agent gets for a behavioral run. + + The skill under test, plus the cross-platform set from `engine/tools.py` -- + inspect's own `bash()` and `text_editor()` assume a POSIX guest, which the + Windows legs do not have. + """ + from inspect_ai.tool import skill + + return [skill([skill_dir]), *tools.toolset()] + + +def _prompt() -> str | None: + """Tell the agent where its work belongs, when that is not obvious. + + A container sandbox starts at `/`, and an agent left to guess reasonably + tries `/app`, then `~`, and scatters its output. What a case produced then + depends on where the agent happened to `cd`, which is not something the + dataset should have to predict. + """ + if not tools.containerized(): + return None + return ( + f"Your working directory is {tools.WORKDIR}. Create and edit files " + "there, using paths relative to it, so the work you produce can be " + "found afterwards." + ) + + +def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = None): + """One inspect `Task` per skill: its cases, its skill installed, its scorer.""" + from inspect_ai import Task + from inspect_ai.agent import react + + skill_dir = config.active().skill_path(skill) + samples = [convert.sample_from_case(c, skill_dir, ctx) for c in cases] + + bound = deadline.active() + return Task( + name=f"behavioral-{skill}", + dataset=samples, + solver=react(prompt=_prompt(), tools=_tools(skill_dir)), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=message_limit_for(model), + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +def _outcomes(log, skill: str, cases: list[Case]) -> list[BehaviorOutcome]: + """Map one inspect `EvalLog` back onto skillscope's outcome objects. + + A task that failed outright reports one failed outcome per case rather than + an empty list: an infrastructure failure that produced no samples must not + render as "every expectation met". + """ + prompts = {c.id: c.prompt for c in cases} + outcomes: list[BehaviorOutcome] = [] + + if log.status == "error" or not log.samples: + detail = getattr(log.error, "message", None) or "the task produced no samples" + return [ + BehaviorOutcome( + id=case.id, + skill=skill, + prompt=case.prompt, + passed=False, + elapsed_s=0.0, + error=f"inspect task failed: {detail}", + ) + for case in cases + ] + + for sample in log.samples: + case_id = str(sample.id) + checks: list[dict] = [] + error: str | None = None + + for score in (sample.scores or {}).values(): + checks.extend((score.metadata or {}).get(scorers.CHECKS, [])) + + if sample.error is not None: + error = f"{sample.error.message}" + elif not checks: + error = "case has no behavioral assertions to grade" + + outcomes.append( + BehaviorOutcome( + id=case_id, + skill=skill, + prompt=prompts.get(case_id, ""), + passed=error is None and bool(checks) and all(c["passed"] for c in checks), + elapsed_s=round(getattr(sample, "total_time", None) or 0.0, 2), + checks=checks, + error=error, + ) + ) + return outcomes + + +def run( + skills: list[str], cases: list[Case], model: str, effort: str +) -> list[BehaviorOutcome]: + """Run every behavioral case, grouped by skill. Mirrors `behavior.run`.""" + from inspect_ai import eval as inspect_eval + + sandbox_spec.require_provider() + + outcomes: list[BehaviorOutcome] = [] + for skill in skills: + skill_cases = [c for c in cases if c.skill == skill and c.has_behavior] + if not skill_cases: + continue + + print(f"[behavioral] {skill}: {len(skill_cases)} case(s)", flush=True) + logs = inspect_eval( + build_task(skill, skill_cases, model), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + # skillscope's own progress lines are the report; inspect's rich + # display takes over the terminal and produces nothing useful when + # a CI job pipes stdout to a file. + display="plain", + ) + for log in logs: + stats.record_log(log) + outcomes.extend(_outcomes(log, skill, skill_cases)) + + for outcome in outcomes: + passed = sum(1 for c in outcome.checks if c["passed"]) + print( + f" [{'PASS' if outcome.passed else 'FAIL'}] {outcome.id}: " + f"{passed}/{len(outcome.checks)} checks in {outcome.elapsed_s}s" + + (f" -- {outcome.error}" if outcome.error else ""), + flush=True, + ) + return outcomes diff --git a/skillscope/engine/convert.py b/skillscope/engine/convert.py new file mode 100644 index 0000000..75b9a63 --- /dev/null +++ b/skillscope/engine/convert.py @@ -0,0 +1,99 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Turn skillscope's dataset into inspect samples. + +`evals.json` is the frozen contract: this module is the only place that knows +how a `Case` becomes an inspect `Sample`, so the dataset format and the engine +can move independently. + +Expectations ride along in `Sample.metadata` rather than `Sample.target`. A case +asserts several unrelated things at once (files produced, phrases in the +transcript, judged behaviors), which is a poor fit for the single `target` +string inspect scorers conventionally compare against; the scorers in +`engine/scorers.py` read them back out by name. +""" + +from __future__ import annotations + +from pathlib import Path + +from ..behavior import expand +from ..datasets import Case + +# Keys written into `Sample.metadata`. Named here so scorers and tasks agree on +# the spelling without importing each other. +SKILL = "skill" +SHOULD_TRIGGER = "skill_should_trigger" +CATEGORY = "category" +EXPECTED = "expected_behavior" +UNEXPECTED = "unexpected_behavior" +LOGS_CONTAIN = "logs_contain" +FILES_EXIST = "files_exist" +EXTENDED = "extended" + + +def seed_files(seed: Path) -> dict[str, str]: + """Map a case's ``workspace`` fixture directory onto `Sample.files`. + + The *contents* land at the sandbox working directory, matching what the + legacy `_stage_workspace` did -- a case hands the agent a starting file to + edit rather than describing one in prose. + """ + if not seed.is_dir(): + raise FileNotFoundError(f"workspace fixture directory not found: {seed}") + + from . import tools + + # Seeded under the same directory the tools resolve against. inspect writes + # these relative to the sandbox's own working directory, which for a + # container is `/` -- so without this a case's fixture would land beside + # `/etc` while the agent worked somewhere else. + prefix = f"{tools.WORKDIR.lstrip('/')}/" if tools.containerized() else "" + + files: dict[str, str] = {} + for path in sorted(seed.rglob("*")): + if path.is_file(): + target = path.relative_to(seed).as_posix() + files[prefix + target] = str(path) + return files + + +def sample_from_case(case: Case, skill_dir: Path, ctx: dict | None = None) -> "object": + """Build one inspect `Sample` from a `Case`. + + `ctx` supplies `{name}` template variables, expanded with the same + substitution the legacy engine uses so a prompt containing literal braces + (JSON snippets, regex quantifiers) survives unchanged. + """ + from inspect_ai.dataset import Sample + + ctx = ctx or {} + files = seed_files(skill_dir / case.workspace) if case.workspace else {} + + return Sample( + id=case.id, + input=expand(case.prompt, ctx), + files=files or None, + metadata={ + SKILL: case.skill, + SHOULD_TRIGGER: case.skill_should_trigger, + CATEGORY: case.category, + EXPECTED: list(case.expected_behavior), + UNEXPECTED: list(case.unexpected_behavior), + LOGS_CONTAIN: [expand(t, ctx) for t in case.logs_contain], + FILES_EXIST: [expand(p, ctx) for p in case.files_exist], + EXTENDED: case.extended, + }, + ) + + +def samples_from_cases( + cases: list[Case], skill_dir_for: "object", ctx: dict | None = None +) -> list: + """Convert many cases. `skill_dir_for` maps a skill name to its directory.""" + return [ + sample_from_case(case, skill_dir_for(case.skill), ctx) + for case in cases + ] diff --git a/skillscope/engine/judge.py b/skillscope/engine/judge.py new file mode 100644 index 0000000..c329396 --- /dev/null +++ b/skillscope/engine/judge.py @@ -0,0 +1,287 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The LLM judge for `expected_behavior` / `unexpected_behavior`. + +Two properties of the legacy judge are load-bearing and preserved here. + +**Polarity is never inverted.** The judge is shown the requirement as written, +including "must not" ones, and reports whether the requirement is *satisfied*. +A caller that negates the verdict turns a correct run into a failure, which is +why `agent._grade_with_llm` carries the same warning. + +**The judge sees what the agent produced, not what it said about it.** Evidence +is the tool calls, the tool output, and the artifacts themselves -- an agent +writing "I won't call the cloud API" must not satisfy an expectation that it +avoided doing so, and must not fail one either. Text artifacts are included +inline and images are attached, so "did it actually generate a picture of a +cat" is answerable rather than inferred from a filename. +""" + +from __future__ import annotations + +from pathlib import PurePosixPath + +# Bounds on the evidence packet. A behavioral run can leave a model cache or a +# multi-megabyte log in the workspace; the judge needs the artifacts a case is +# about, not everything on disk. +MAX_FILES = 20 +MAX_FILE_BYTES = 20_000 +MAX_TRANSCRIPT = 12_000 + +# Tool *calls* are short and every one of them matters -- they are the record of +# what the agent did. Tool *results* are what grow without bound (a directory +# listing, a validator's output, a file echoed back), so they are capped +# individually and the calls are always kept whole. +MAX_RESULT = 800 + +IMAGE_SUFFIXES = {".png", ".jpg", ".jpeg", ".gif", ".webp"} +BINARY_SUFFIXES = {".zip", ".gz", ".tar", ".bin", ".safetensors", ".onnx", ".pt"} + +VERDICT_INSTRUCTIONS = """\ +Answer with a single line of JSON and nothing else: +{"pass": true|false, "reason": ""} +""" + + +def is_image(path: str) -> bool: + return PurePosixPath(path).suffix.lower() in IMAGE_SUFFIXES + + +def is_probably_binary(path: str) -> bool: + return PurePosixPath(path).suffix.lower() in BINARY_SUFFIXES + + +def requirement_text(statement: str, *, must_happen: bool) -> str: + """The requirement as the judge sees it, with its polarity spelled out.""" + if must_happen: + return ( + f"The agent MUST have done this:\n{statement}\n\n" + 'Set "pass" to true if the agent did it, false if it did not.' + ) + return ( + f"The agent MUST NOT have done this:\n{statement}\n\n" + 'Set "pass" to true if the agent avoided it, false if the agent did it ' + "anyway. Absence of evidence that the agent did it counts as avoiding " + "it, so the default verdict is true." + ) + + +def parse_verdict(text: str) -> tuple[bool, str] | None: + """Read the last verdict-shaped JSON object out of a chatty reply. + + A reason can itself contain braces -- a regex quantifier, a quoted snippet -- + so the decoder finds object boundaries rather than matching them textually. + """ + import json + + decoder = json.JSONDecoder() + verdict = None + for index, char in enumerate(text): + if char != "{": + continue + try: + parsed, _ = decoder.raw_decode(text[index:]) + except ValueError: + continue + if isinstance(parsed, dict) and "pass" in parsed: + verdict = parsed + + if verdict is None: + return None + reason = str(verdict.get("reason", "")).strip() or "(no reason given)" + return bool(verdict.get("pass")), reason + + +def final_message_of(state) -> str: + """What the agent last said to the user. + + Kept apart from the transcript on purpose. Some expectations are about what + the agent *told* the user -- "output the curl commands they need" -- and are + unanswerable without it. Others are about what it *did*, and for those a + claim in the final message is not evidence: an agent writing "I won't call + the cloud API" must neither satisfy nor fail an expectation that it avoided + doing so. The prompt says which to use for which. + """ + # A `react` agent delivers its answer through the submit tool, and that is + # what lands in `output.completion`. The last assistant *message* can be + # the preamble that introduces it -- "the commands are below" -- so reading + # only that can show the judge a description of an answer instead of the + # answer. + completion = getattr(getattr(state, "output", None), "completion", None) + if isinstance(completion, str) and completion.strip(): + return completion.strip()[:MAX_TRANSCRIPT] + + for message in reversed(state.messages): + if getattr(message, "role", None) != "assistant": + continue + content = getattr(message, "content", None) + if isinstance(content, str) and content.strip(): + return content.strip()[:MAX_TRANSCRIPT] + if isinstance(content, list): + texts = [ + part.text + for part in content + if isinstance(getattr(part, "text", None), str) + ] + if any(t.strip() for t in texts): + return "\n".join(texts).strip()[:MAX_TRANSCRIPT] + return "(the agent said nothing)" + + +def _elide_middle(text: str, limit: int) -> str: + """Trim the middle, never the end. + + Cutting the tail drops the most recent actions, and those are usually the + ones a check turns on -- an agent writes a file, then validates it, and the + validation is what the expectation is about. Losing it makes the run look + like the agent claimed something it never did. + """ + if len(text) <= limit: + return text + head = text[: limit // 2] + tail = text[-(limit // 2) :] + return f"{head}\n...[middle of transcript elided]...\n{tail}" + + +def transcript_of(state) -> str: + """What the agent did: tool calls and their results, never its prose.""" + parts: list[str] = [] + for message in state.messages: + for call in getattr(message, "tool_calls", None) or []: + parts.append(f"$ {call.function} {call.arguments}") + if getattr(message, "role", "") == "tool": + content = getattr(message, "content", None) + if isinstance(content, str): + body = content.strip() + if len(body) > MAX_RESULT: + body = body[:MAX_RESULT] + " ...[output truncated]" + parts.append(body) + return _elide_middle("\n".join(parts), MAX_TRANSCRIPT) + + +async def artifacts(paths: list[str]) -> tuple[list[str], list[tuple[str, bytes]]]: + """Read what the agent produced: text inline, images as attachments.""" + from inspect_ai.util import sandbox + + from . import tools + + described: list[str] = [] + images: list[tuple[str, bytes]] = [] + + for path in paths[:MAX_FILES]: + # `list_paths` reports paths relative to the case's working directory, + # but `read_file` resolves against the sandbox's own -- which for a + # container is `/`. Without this the judge is told every artifact is + # unreadable and concludes the agent produced nothing. + target = await tools.resolve(path) + if is_image(path): + try: + images.append((path, await sandbox().read_file(target, text=False))) + except Exception as exc: # noqa: BLE001 -- an unreadable file is evidence too + described.append(f"--- {path} (image, unreadable: {exc}) ---") + continue + if is_probably_binary(path): + described.append(f"--- {path} (binary) ---") + continue + try: + body = await sandbox().read_file(target, text=True) + except Exception as exc: # noqa: BLE001 + described.append(f"--- {path} (unreadable: {exc}) ---") + continue + if len(body) > MAX_FILE_BYTES: + body = body[:MAX_FILE_BYTES] + "\n...[truncated]..." + described.append(f"--- {path} ---\n{body}") + + if len(paths) > MAX_FILES: + described.append(f"...and {len(paths) - MAX_FILES} more files") + return described, images + + +async def grade( + statement: str, + state, + *, + must_happen: bool, + grader: str | None = None, +) -> tuple[bool, str]: + """Ask the grader whether one requirement was satisfied.""" + from inspect_ai.model import ( + ChatMessageUser, + ContentImage, + ContentText, + get_model, + ) + + from . import tools + + try: + paths = await tools_list_paths() + except tools.ListingFailed as exc: + # Say so rather than presenting an empty workspace as fact. A judge told + # "no files" will confidently report the agent did nothing, which reads + # as the skill failing when the sandbox is what failed. + return False, f"judge skipped: could not list the sandbox -- {exc}" + + described, images = await artifacts(paths) + + evidence = "\n".join( + [ + f"Files the agent left behind: {paths or 'none'}", + "", + "--- what the agent DID (tool calls and their results) ---", + transcript_of(state), + "", + "--- what the agent SAID to the user (its final message) ---", + final_message_of(state), + "", + "--- artifacts it produced ---", + *described, + ] + ) + + content: list = [ + ContentText( + text=( + "You are grading whether a coding agent's run satisfied one " + "requirement. Judge only from the evidence below.\n\n" + "For a requirement about what the agent DID, use the tool " + "calls and the artifacts: the agent claiming in its message " + "that it did or avoided something is not evidence either way. " + "For a requirement about what the agent TOLD the user, its " + "final message is the evidence.\n\n" + f"REQUIREMENT:\n{requirement_text(statement, must_happen=must_happen)}\n\n" + f"EVIDENCE:\n{evidence}\n\n" + "Do not invert the verdict for any reason.\n" + f"{VERDICT_INSTRUCTIONS}" + ) + ) + ] + for path, data in images: + content.append(ContentText(text=f"--- {path} ---")) + content.append(ContentImage(image=_data_uri(path, data))) + + model = get_model(grader) if grader else get_model() + output = await model.generate([ChatMessageUser(content=content)]) + + parsed = parse_verdict(output.completion or "") + if parsed is None: + return False, f"judge gave no JSON verdict: {(output.completion or '')[:200]!r}" + satisfied, reason = parsed + return satisfied, f"judge: {reason}" + + +def _data_uri(path: str, data: bytes) -> str: + import base64 + + suffix = PurePosixPath(path).suffix.lower().lstrip(".") + mime = "jpeg" if suffix in {"jpg", "jpeg"} else suffix + return f"data:image/{mime};base64,{base64.b64encode(data).decode()}" + + +async def tools_list_paths() -> list[str]: + """Indirection so `judge` does not import `tools` at module scope.""" + from . import tools + + return await tools.list_paths() diff --git a/skillscope/engine/models.py b/skillscope/engine/models.py new file mode 100644 index 0000000..9c2fbff --- /dev/null +++ b/skillscope/engine/models.py @@ -0,0 +1,114 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Model names: skillscope aliases to inspect model strings. + +`--model opus` is the `claude` CLI's alias vocabulary. inspect wants a +provider-qualified name (`anthropic/claude-opus-5`), so the two have to be +translated at the boundary rather than either side changing its spelling -- +`--model` is part of the frozen CLI surface. + +Anything already carrying a provider prefix passes through untouched, which is +what makes `--model mockllm/model` work for the no-cost wiring runs. +""" + +from __future__ import annotations + +import os + +ALIASES = { + "opus": "anthropic/claude-opus-5", + "sonnet": "anthropic/claude-sonnet-5", + "haiku": "anthropic/claude-haiku-4-5-20251001", +} + +# `claude` reads per-request headers from this; nothing in inspect does, so +# skillscope parses it and hands the result to the provider instead. An +# enterprise gateway in front of the Anthropic API is the reason it exists. +CUSTOM_HEADERS_ENV = "ANTHROPIC_CUSTOM_HEADERS" +AUTH_TOKEN_ENV = "ANTHROPIC_AUTH_TOKEN" + + +def resolve(model: str) -> str: + """Translate a skillscope model alias into an inspect model string.""" + if "/" in model: + return model + return ALIASES.get(model.lower(), f"anthropic/{model}") + + +async def _probe(model: str): + from inspect_ai.model import get_model + + resolved = get_model(model, **model_args(model)) + return await resolved.generate("Reply with the single word: ok") + + +def check_reachable(model: str) -> tuple[bool, str]: + """Confirm the model answers before anything expensive starts. + + A graded run starts containers and installs skills before it ever reaches a + provider, so a misconfigured gateway surfaces as a task that failed after + all that work rather than as a credentials problem. One tiny call up front + turns a 401 buried in a sample error into a message on the first line. + + Costs a handful of tokens. `mockllm` reaches no provider, so it is skipped + rather than charged for a round trip that proves nothing. + """ + if model.startswith("mockllm"): + return True, "mockllm (no provider)" + + import anyio + + try: + output = anyio.run(_probe, model) + except Exception as exc: # noqa: BLE001 -- the reason is the return value + return False, f"{type(exc).__name__}: {exc}"[:400] + return True, (output.completion or "").strip()[:40] + + +def custom_headers() -> dict[str, str]: + """Parse ``ANTHROPIC_CUSTOM_HEADERS`` (newline-separated ``Key: value``).""" + headers: dict[str, str] = {} + for line in (os.environ.get(CUSTOM_HEADERS_ENV) or "").splitlines(): + if ":" not in line: + continue + name, _, value = line.partition(":") + if name.strip(): + headers[name.strip()] = value.strip() + return headers + + +def model_args(model: str) -> dict: + """Provider arguments for the configured gateway, if any. + + inspect passes these straight to `AsyncAnthropic`, so custom headers ride in + as `default_headers`. Empty when no gateway headers are configured, which is + the ordinary api.anthropic.com case. + + Scoped to Anthropic models on purpose. The free `mockllm/model` wiring run + reaches no provider at all, and refusing it because the shell happens to + hold both Anthropic variables would break the one check that costs nothing + -- on exactly the machines most likely to have an OAuth token lying around. + """ + if not model.startswith("anthropic/"): + return {} + + headers = custom_headers() + if not headers: + return {} + + if os.environ.get(AUTH_TOKEN_ENV): + # The same rule `credentials.resolve` enforces when it hands a job its + # environment: a federated token is only good at api.anthropic.com, so + # it never travels with a gateway's base URL or headers. Caught here + # too because an environment can be assembled by hand, and inspect's + # OAuth path also sets `default_headers` itself -- passing ours would + # surface as a duplicate keyword argument from inside the SDK. + raise SystemExit( + f"error: both {AUTH_TOKEN_ENV} and {CUSTOM_HEADERS_ENV} are set. " + "A federated token only works at api.anthropic.com; reaching a " + "gateway needs ANTHROPIC_API_KEY instead. Unset one of them." + ) + + return {"default_headers": headers} diff --git a/skillscope/engine/routing.py b/skillscope/engine/routing.py new file mode 100644 index 0000000..1d6ed3b --- /dev/null +++ b/skillscope/engine/routing.py @@ -0,0 +1,183 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Routing evals on the inspect engine. + +Routing asks one question: with the whole catalog installed, does this prompt +activate the skill it should, and stay quiet when it should not? The room +matters -- a skill tested alone happily answers its neighbour's prompts -- so +every skill is offered on every case, exactly as `routing.stage_workspace` did. + +Two things get much simpler than the legacy engine. + +**Activation is observed, not inferred.** With the `skill()` tool the agent +names the skill it wants, so the decision is a tool call rather than something +reconstructed from a stream of events. `routing.detect_activation` and its +helpers exist because that signal was not available. + +**Stopping is free.** A routing decision is visible in the first assistant turn, +so a case is exactly one model call: the skills are offered as tools and the +reply either names one or does not. Nothing is executed, so there is no agent +loop to bound and no sandbox to start -- the legacy engine's stream reader, +process group and SIGKILL all exist to end a turn it had already paid for. + +Keeping the scaffolding out is also a measurement decision: no submit tool and +no agent system prompt sit between the descriptions and the decision, which is +what makes the result about the descriptions. + +What this measures is how well a description discriminates against its +neighbours, which is the part a skill author controls. It is not a measurement +of any particular product harness's discovery machinery, and the numbers are +not interchangeable with one. +""" + +from __future__ import annotations + +import time +from pathlib import Path + +from .. import config, deadline +from ..datasets import Case +from ..routing import PASSING_VERDICTS, Outcome, classify +from . import convert, models, stats + +SKILL_TOOL = "skill" + + +def activation_of(messages) -> str | None: + """The skill the agent asked for, or None if it never asked for one.""" + for message in messages: + for call in getattr(message, "tool_calls", None) or []: + if call.function != SKILL_TOOL: + continue + named = (call.arguments or {}).get("command") + if isinstance(named, str) and named.strip(): + return named.strip() + return None + + +def tool_call_count(messages) -> int: + return sum(len(getattr(m, "tool_calls", None) or []) for m in messages) + + +def decide(routing_set: dict[str, Path]): + """Solver: offer the room as tools, take one turn, record what was named. + + The tool is never executed -- only its definition matters, which is the + skill's name and description. That is the whole input to a routing decision. + """ + from inspect_ai.solver import solver + from inspect_ai.tool import skill + + @solver + def _decide(): + tools = [skill(list(routing_set.values()))] + + async def solve(state, generate): + from inspect_ai.model import get_model + + output = await get_model().generate(input=state.messages, tools=tools) + state.messages.append(output.message) + state.output = output + return state + + return solve + + return _decide() + + +def build_task(cases: list[Case], routing_set: dict[str, Path]): + """One task holding every routing case, with the whole room offered.""" + from inspect_ai import Task + + cfg = config.active() + samples = [ + convert.sample_from_case( + case, cfg.skill_path(case.skill) if case.skill else Path(".") + ) + for case in cases + ] + + bound = deadline.active() + return Task( + name="routing", + dataset=samples, + solver=decide(routing_set), + # No sandbox: nothing is executed, so there is nothing to isolate. + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +def _outcome(sample, case: Case, room: list[str]) -> Outcome: + """Map one inspect sample onto the outcome `routing.summarize` expects.""" + messages = sample.messages or [] + observed = activation_of(messages) + calls = tool_call_count(messages) + + if sample.error is not None: + verdict, error, stop_reason = "error", sample.error.message, "error" + else: + verdict, error = classify(case.expect_skill, observed), None + stop_reason = "decided" if observed else "completed" + + return Outcome( + id=case.id, + category=case.category, + skill=case.skill, + prompt=case.prompt, + expect=case.expect_skill, + observed=observed, + verdict=verdict, + passed=verdict in PASSING_VERDICTS, + stop_reason=stop_reason, + elapsed_s=round(getattr(sample, "total_time", None) or 0.0, 2), + tool_calls=calls, + # The legacy engine counted reads of a skill body separately because a + # skill could be inspected without being activated. The tool makes that + # distinction disappear: naming the skill *is* the activation. + inspection_calls=0, + visible_skills=room, + # Nothing beyond the room can leak in: the tool is constructed from the + # room, so there is no user-level config dir for a stray skill to arrive + # from. That is why `routing.can_isolate_config` has no counterpart here. + extra_skills=[], + error=error, + ) + + +def run(cases: list[Case], routing_set: dict[str, Path], model: str) -> list[Outcome]: + """Run every routing case against the whole room.""" + from inspect_ai import eval as inspect_eval + + if not cases: + return [] + + room = list(routing_set) + print(f"[routing] installed together: {', '.join(room) or '(none)'}") + print(f"[routing] {len(cases)} cases, model={model}", flush=True) + + started = time.perf_counter() + logs = inspect_eval( + build_task(cases, routing_set), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + display="plain", + ) + + by_id = {case.id: case for case in cases} + outcomes: list[Outcome] = [] + for log in logs: + stats.record_log(log) + if log.status == "error" or not log.samples: + detail = getattr(log.error, "message", None) or "no samples" + raise SystemExit(f"error: routing task failed: {detail}") + for sample in log.samples: + case = by_id.get(str(sample.id)) + if case is not None: + outcomes.append(_outcome(sample, case, room)) + + elapsed = round(time.perf_counter() - started, 1) + print(f"[routing] {len(outcomes)} decisions in {elapsed}s", flush=True) + return outcomes diff --git a/skillscope/engine/sandbox.py b/skillscope/engine/sandbox.py new file mode 100644 index 0000000..011a91e --- /dev/null +++ b/skillscope/engine/sandbox.py @@ -0,0 +1,138 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Which sandbox a skill's cases run in. + +Two decisions, kept apart because they are made by different people. + +**Which provider** is a property of the machine: Docker by default, `podman` on +a host that has that instead, `local` where there is no container at all. +`SKILLSCOPE_SANDBOX` selects it, because whoever runs the job knows what the +runner has and a skill does not. Any provider inspect can resolve works -- +`podman` comes from `inspect-podman`, which registers itself through an +`inspect_ai` entry point, so installing it is the whole setup. + +**What the sandbox has to provide** is a property of the skill, declared in +`evals/machine.yml` with a `sandbox:` key naming a compose file. A skill that +must reach the network to pull a model, or that needs a device bound in, says +so there instead of every skill paying for what one of them needs. + +Windows is the exception to both: inspect's sandbox layer, and every tool built +on it, assumes a POSIX guest, so the Windows legs run `local` and trade +isolation for running on the platform they are meant to test. Ephemeral, +off-network runners are what covers that gap. +""" + +from __future__ import annotations + +import os +import sys + +from .. import datasets + +# Which provider to use. Set it to what the runner actually has: `podman` on a +# host without Docker, `local` to skip the container entirely. `local` is for +# working locally, not for CI -- a graded run that quietly dropped its sandbox +# would report the same numbers with none of the isolation. +SANDBOX_ENV = "SKILLSCOPE_SANDBOX" + +DEFAULT_PROVIDER = "docker" + +# Providers that take no configuration, so a skill's compose file cannot apply. +UNCONFIGURED = {"local"} + +# `local` runs in the same filesystem as the harness: the sandbox API works, but +# nothing is isolated. Named so a report can say which it was. +NOT_ISOLATED = {"local"} + + +def describe() -> dict: + """What the report should say about isolation. + + A report that shows the same numbers whether or not a case was contained + invites the reader to assume it was. Both engines say it outright instead, + so "these ran isolated and those did not" is answerable from the artifact + rather than from whoever remembers how the job was configured. + """ + name = provider() + return {"sandbox": name, "sandbox_isolated": name not in NOT_ISOLATED} + + +# Providers that live in another package. inspect resolves these through an +# entry point, so the binary being installed proves nothing -- the Python +# package has to be there too, and the failure otherwise is a ValueError from +# inspect's registry that says nothing about how to fix it. +PROVIDER_PACKAGES = {"podman": "skillscope[podman]"} + + +def is_windows() -> bool: + return sys.platform.startswith("win") + + +def require_provider(resolve=None) -> None: + """Fail early, and legibly, when the chosen provider cannot be resolved. + + `resolve` is injectable so this can be tested without the inspect extra + installed, which the unit suite deliberately runs without. + """ + name = provider() + if resolve is None: + from inspect_ai.util._sandbox.registry import registry_find_sandboxenv + + resolve = registry_find_sandboxenv + + try: + resolve(name) + except Exception as exc: # noqa: BLE001 -- inspect raises a bare ValueError + hint = PROVIDER_PACKAGES.get(name) + install = f"\n pip install '{hint}'" if hint else "" + raise SystemExit( + f"error: {SANDBOX_ENV}={name!r} but inspect cannot resolve that " + f"sandbox provider.{install}\n" + f" ({exc})" + ) from exc + + +def provider() -> str: + """The sandbox provider for this run.""" + override = os.environ.get(SANDBOX_ENV, "").strip() + if override: + return override + if is_windows(): + return "local" + return DEFAULT_PROVIDER + + +def for_skill(skill: str): + """The `sandbox` spec for a skill's task. + + Returns a `(provider, config)` tuple when the skill declares a compose file + and the provider can take one, a bare provider name otherwise -- both are + accepted as `Task(sandbox=...)`. + """ + name = provider() + if name in UNCONFIGURED: + return name + + # The provider is the machine's choice and the compose file is the skill's, + # so selecting a provider must not silently discard what the skill asked + # for: a skill that needs network egress would otherwise run without it and + # fail for a reason nothing in the report explains. + compose = _declared_compose(skill) + return (name, str(compose)) if compose is not None else name + + +def _declared_compose(skill: str): + """Path to the compose file a skill's `machine.yml` names, if any.""" + name = (datasets._read_machine(skill) or {}).get("sandbox") + if not name: + return None + + path = datasets.skill_path(skill) / name + if not path.is_file(): + raise SystemExit( + f"error: {skill}: evals/machine.yml names sandbox '{name}', " + f"but {path} does not exist." + ) + return path diff --git a/skillscope/engine/scorers.py b/skillscope/engine/scorers.py new file mode 100644 index 0000000..e465f57 --- /dev/null +++ b/skillscope/engine/scorers.py @@ -0,0 +1,119 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Grading for the inspect engine. + +One scorer grades every expectation a case carries and reports them all, rather +than one scorer per kind. A behavioral run costs minutes and real tokens, so a +run that fails should not have to be repeated to discover the second thing wrong +with it -- the same reason the legacy `Run.evaluate` reports instead of raising. + +The per-expectation results ride in `Score.metadata["checks"]` in the shape +`agent.Check` uses, so `behavior.render_markdown` keeps working unchanged. +""" + +from __future__ import annotations + +import os + +from ..agent import _find_file +from . import convert, judge, tools + +CHECKS = "checks" + + +def _check(kind: str, expectation: str, passed: bool, detail: str = "") -> dict: + return { + "kind": kind, + "expectation": expectation, + "passed": passed, + "detail": detail, + } + + +def searchable(state) -> str: + """Everything in the run, for `logs_contain` to search. + + Deliberately broader than what the judge sees. The legacy engine searched + the whole raw transcript, so a case can pin down a tool name, a command + string, or a phrase the agent used -- and cases were written against that. + `judge.transcript_of` is the narrower, prose-free view, because an agent + *claiming* it avoided something is not evidence that it did. + """ + parts: list[str] = [] + for message in state.messages: + parts.append(f"{getattr(message, 'role', '')}:") + for call in getattr(message, "tool_calls", None) or []: + parts.append(f"{call.function} {call.arguments}") + content = getattr(message, "content", None) + if isinstance(content, str): + parts.append(content) + elif isinstance(content, list): + for part in content: + text = getattr(part, "text", None) + if isinstance(text, str): + parts.append(text) + return "\n".join(parts) + + +def expectations(): + """Grade every expectation on the case and report each one.""" + from inspect_ai.scorer import CORRECT, INCORRECT, Score, accuracy, scorer, stderr + + @scorer(metrics=[accuracy(), stderr()]) + def _expectations(): + async def score(state, target) -> "Score": + meta = state.metadata or {} + checks: list[dict] = [] + + transcript = searchable(state) + for text in meta.get(convert.LOGS_CONTAIN, []): + checks.append( + _check("logs_contain", text, text.lower() in transcript.lower()) + ) + + wanted = meta.get(convert.FILES_EXIST, []) + if wanted: + try: + files = await tools.list_paths() + except tools.ListingFailed as exc: + # Report the sandbox, not the skill. "Nothing was produced" + # would blame the agent for the harness's failure. + for path in wanted: + checks.append( + _check("files_exist", path, False, f"could not list the sandbox: {exc}") + ) + files = None + else: + for path in wanted: + found = _find_file(files, path) + detail = "" + if found is None: + detail = f"sandbox holds: {files or 'nothing'}" + elif found != path: + detail = f"found at {found}" + checks.append( + _check("files_exist", path, found is not None, detail) + ) + + # Judged expectations last: the deterministic results are on screen + # before the grader calls, which take a few seconds each, begin. + for statement in meta.get(convert.EXPECTED, []): + ok, reason = await judge.grade(statement, state, must_happen=True) + checks.append(_check("expected_behavior", statement, ok, reason)) + + for statement in meta.get(convert.UNEXPECTED, []): + ok, reason = await judge.grade(statement, state, must_happen=False) + checks.append(_check("unexpected_behavior", statement, ok, reason)) + + passed = bool(checks) and all(c["passed"] for c in checks) + return Score( + value=CORRECT if passed else INCORRECT, + answer=f"{sum(c['passed'] for c in checks)}/{len(checks)} checks", + metadata={CHECKS: checks}, + ) + + return score + + return _expectations() diff --git a/skillscope/engine/stats.py b/skillscope/engine/stats.py new file mode 100644 index 0000000..c28160c --- /dev/null +++ b/skillscope/engine/stats.py @@ -0,0 +1,41 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Read what an inspect run spent out of its `EvalLog`. + +inspect already counts this per model in `log.stats.model_usage`; skillscope +just has to move it somewhere the report can see. Kept apart from the engine +modules so `usage` stays the only shared vocabulary between the two engines. +""" + +from __future__ import annotations + +from .. import usage + + +def record_log(log) -> None: + """Add one `EvalLog`'s token and cost totals to the run.""" + stats = getattr(log, "stats", None) + for model_usage in (getattr(stats, "model_usage", None) or {}).values(): + usage.record( + input_tokens=getattr(model_usage, "input_tokens", 0) or 0, + output_tokens=getattr(model_usage, "output_tokens", 0) or 0, + # Populated only when the provider supplies pricing; a gateway + # generally does not, so this stays None and the report omits it. + cost_usd=getattr(model_usage, "total_cost", None), + calls=0, + ) + + # Count assistant messages, not samples. The legacy engine records one call + # per assistant event in its stream, so counting per sample here would be + # the same number only for routing -- where each case is a single turn -- + # and a large undercount for behavioral, where the agent loops. The two + # columns sit side by side in the benchmark, so they have to mean the same + # thing. + responses = 0 + for sample in getattr(log, "samples", None) or []: + for message in getattr(sample, "messages", None) or []: + if getattr(message, "role", None) == "assistant": + responses += 1 + usage.record(calls=responses) diff --git a/skillscope/engine/tools.py b/skillscope/engine/tools.py new file mode 100644 index 0000000..eb9d873 --- /dev/null +++ b/skillscope/engine/tools.py @@ -0,0 +1,287 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""A tool set that works on a non-POSIX guest. + +inspect's own tools assume one: `bash()` execs `["bash", "--login", "-c", ...]`, +`text_editor()` needs a Linux-only helper binary, and `list_files()`/`grep()` +shell out to `find`/`grep`. On a Windows host with the `local` sandbox that +leaves an agent that can read and think but cannot write a file or run a +command -- not enough to grade a behavioral case with. + +Everything here is built on `SandboxEnvironment.exec` / `read_file` / +`write_file`, which are provider-level and platform-neutral. Only the shell +invocation differs, and that is probed once per sample rather than assumed: +the same Docker sandbox is POSIX whichever host started it, so the host's own +platform is not the answer. +""" + +from __future__ import annotations + +SHELL_KEY = "skillscope_shell" +WORKDIR_KEY = "skillscope_workdir" + +# A container sandbox starts at `/`, so a relative path lands beside `/proc` and +# `/etc` and a recursive listing walks the whole image. Everything the case does +# happens here instead: fixtures are seeded into it, tools resolve against it, +# and it is what gets listed. inspect_swe resolves the same problem the same way +# -- its agent cwd falls back to the home directory when the sandbox default is +# `/`. +WORKDIR = "/workspace" + +POSIX_SHELL = ["bash", "-lc"] +WINDOWS_SHELL = ["powershell", "-NoProfile", "-Command"] + +# Listing is the one thing `SandboxEnvironment` has no method for, so it stays a +# shell command -- but only one, defined here, used by both the tools and the +# scorers. +POSIX_LIST = "find . -type f" +WINDOWS_LIST = "Get-ChildItem -Recurse -File | Resolve-Path -Relative" + + +async def shell_prefix() -> list[str]: + """The argv prefix that runs a shell command in this sample's sandbox. + + Probed once and remembered: a probe per tool call would double the round + trips on the slowest part of a run. + """ + from inspect_ai.util import sandbox, store + + cached = store().get(SHELL_KEY) + if cached: + return list(cached) + + probe = await sandbox().exec(["bash", "-lc", "exit 0"], concurrency=False) + prefix = POSIX_SHELL if probe.success else WINDOWS_SHELL + store().set(SHELL_KEY, prefix) + return list(prefix) + + +def containerized() -> bool: + """Whether this run has a sandbox of its own to work in.""" + from . import sandbox as sandbox_spec + + return sandbox_spec.provider() not in sandbox_spec.NOT_ISOLATED + + +async def workdir() -> str | None: + """The directory a case works in, or None to use the sandbox's own. + + `local` needs none: the harness's working directory is already a sensible + place and creating `/workspace` on someone's machine would not be. + """ + if not containerized(): + return None + + from inspect_ai.util import sandbox, store + + cached = store().get(WORKDIR_KEY) + if cached: + return cached + + prefix = await shell_prefix() + await sandbox().exec(prefix + [f"mkdir -p {WORKDIR}"], concurrency=False) + store().set(WORKDIR_KEY, WORKDIR) + return WORKDIR + + +async def resolve(path: str) -> str: + """A case-relative path, as the sandbox should see it.""" + base = await workdir() + if base is None or path.startswith("/"): + return path + return f"{base}/{path.lstrip('./')}" + + +async def run(command: str, timeout: int | None = None): + """Run `command` through whichever shell the sandbox has, in the workdir.""" + from inspect_ai.util import sandbox + + prefix = await shell_prefix() + return await sandbox().exec( + prefix + [command], cwd=await workdir(), timeout=timeout + ) + + +def normalize_listing(stdout: str) -> list[str]: + """Turn a directory listing into relative POSIX-style paths. + + `find` and `Get-ChildItem` disagree about separators and prefixes, so this + normalises both: backslashes become slashes and a leading `./` or `.\\` is + dropped. The harness's own furniture is filtered out -- an installed skill + is not something the case produced, and `files_exist` must not be satisfied + by one. + """ + paths: list[str] = [] + for line in stdout.splitlines(): + rel = line.strip().replace("\\", "/") + while rel.startswith("./"): + rel = rel[2:] + if not rel or rel.startswith(".claude/") or rel.startswith("skills/"): + continue + paths.append(rel) + return sorted(paths) + + +class ListingFailed(RuntimeError): + """The sandbox could not be listed, which is not the same as it being empty. + + Returning an empty list here would make a broken sandbox look exactly like + an idle agent: `files_exist` fails, and the judge -- which builds its + evidence from the same listing -- reports that nothing was produced. Both + read as the skill's fault. Raising keeps the two apart. + """ + + +async def list_paths() -> list[str]: + """Files in the sandbox working directory, as relative POSIX-style paths.""" + prefix = await shell_prefix() + listing = WINDOWS_LIST if prefix == WINDOWS_SHELL else POSIX_LIST + result = await run(listing) + if not result.success: + raise ListingFailed( + f"`{listing}` failed in the sandbox (exit {result.returncode}). " + f"stderr: {result.stderr.strip()[:200] or '(none)'}" + ) + return normalize_listing(result.stdout) + + +def _text(result) -> str: + """`stdout` plus `stderr`, which is where a failing command says why.""" + parts = [result.stdout.strip(), result.stderr.strip()] + body = "\n".join(p for p in parts if p) + if result.success: + return body or "(no output)" + return f"exit code {result.returncode}\n{body}".strip() + + +def shell(timeout: int = 300): + """Run shell commands in the sandbox, on whichever platform it is.""" + from inspect_ai.tool import Tool, tool + + @tool(name="shell") + def _shell() -> Tool: + async def execute(command: str) -> str: + """Run a command in the sandbox and return its output. + + Uses bash on Linux and macOS, and PowerShell on Windows, so write + commands for the platform you find yourself on. Check with `uname` + or `$PSVersionTable` if you are unsure. + + Args: + command: The command line to run. + + Returns: + The command's combined output, or its exit code and error output + when it fails. + """ + return _text(await run(command, timeout=timeout)) + + return execute + + return _shell() + + +def write_file(): + """Create or overwrite a file, without going through a shell.""" + from inspect_ai.tool import Tool, tool + + @tool(name="write_file") + def _write_file() -> Tool: + async def execute(path: str, content: str) -> str: + """Write text to a file in the sandbox, replacing it if it exists. + + Prefer this over shell redirection: it needs no quoting or escaping + and behaves the same on every platform. + + Args: + path: File to write, relative to the working directory. + content: The full text the file should contain. + + Returns: + Confirmation of what was written. + """ + from inspect_ai.util import sandbox + + await sandbox().write_file(await resolve(path), content) + return f"wrote {len(content)} characters to {path}" + + return execute + + return _write_file() + + +def edit_file(): + """Replace one exact occurrence of a string in a file.""" + from inspect_ai.tool import Tool, tool + + @tool(name="edit_file") + def _edit_file() -> Tool: + async def execute(path: str, old_text: str, new_text: str) -> str: + """Replace an exact snippet in a file. + + `old_text` must appear exactly once, so include enough surrounding + context to make it unique. To create a file, use write_file. + + Args: + path: File to edit, relative to the working directory. + old_text: The exact text to replace. + new_text: What to put in its place. + + Returns: + Confirmation, or an explanation of why the edit was refused. + """ + from inspect_ai.util import sandbox + + target = await resolve(path) + current = await sandbox().read_file(target, text=True) + found = current.count(old_text) + if found == 0: + return f"no edit made: {path} does not contain that text" + if found > 1: + return ( + f"no edit made: that text appears {found} times in {path}. " + "Include more surrounding context so it matches once." + ) + await sandbox().write_file(target, current.replace(old_text, new_text, 1)) + return f"edited {path}" + + return execute + + return _edit_file() + + +def list_files(): + """List the files the sandbox working directory holds.""" + from inspect_ai.tool import Tool, tool + + @tool(name="list_files") + def _list_files() -> Tool: + async def execute() -> str: + """List every file in the working directory, recursively. + + Returns: + One relative path per line. + """ + try: + paths = await list_paths() + except ListingFailed as exc: + return f"could not list the directory: {exc}" + return "\n".join(paths) if paths else "(no files)" + + return execute + + return _list_files() + + +def toolset() -> list: + """The tools a behavioral run gives the agent. + + inspect's `think()` is reused as-is -- it never touches the sandbox, so it + is already platform-neutral. Its `bash()`, `text_editor()`, `list_files()` + and `grep()` are the ones replaced above. + """ + from inspect_ai.tool import think + + return [shell(), write_file(), edit_file(), list_files(), think()] diff --git a/skillscope/engine/verify.py b/skillscope/engine/verify.py new file mode 100644 index 0000000..caf8a82 --- /dev/null +++ b/skillscope/engine/verify.py @@ -0,0 +1,115 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""The Claude Code verification leg (`--engine claude-code`). + +The `inspect` engine grades a skill with a harness-independent agent, which is +a deliberate choice: it tests whether a skill's *instructions* work rather than +how one product reads them. This leg exists to answer the question that choice +raises -- do the results still hold under the real thing? + +It runs actual Claude Code inside the sandbox, via `inspect_swe`, and produces +the same outcome objects as the other two engines, so the benchmark tool can +diff its report against theirs with nothing new. + +**Reporting only.** It is not a gate. Harness runs are nondeterministic and the +harness is not what we are grading; a divergence here is a question about the +skill, not a build failure. + +**Linux only.** `inspect_swe` shells `bash -c` merely to locate the CLI, and the +model proxy it starts in the guest is a Linux binary, so a Windows guest cannot +run this leg at all. That is the whole reason the primary engine does not depend +on it. +""" + +from __future__ import annotations + +import sys +from pathlib import Path + +from .. import config, deadline +from ..behavior import BehaviorOutcome +from ..datasets import Case +from . import behavioral, convert, models, sandbox as sandbox_spec, scorers, stats + +INSTALL_HINT = ( + "error: --engine claude-code needs the verify extra. Install it with:\n" + " pip install 'skillscope[verify]'" +) + + +def require() -> None: + """Fail early and legibly rather than at the first sandbox call.""" + if sys.platform.startswith("win"): + raise SystemExit( + "error: --engine claude-code cannot run on Windows. inspect_swe " + "requires a POSIX guest, both to locate the CLI and to run the " + "model proxy it installs in the sandbox." + ) + try: + import inspect_swe # noqa: F401 + except ModuleNotFoundError as exc: # pragma: no cover -- environment shape + raise SystemExit(INSTALL_HINT) from exc + + +def build_task(skill: str, cases: list[Case], model: str, ctx: dict | None = None): + """One task per skill, solved by real Claude Code rather than our agent.""" + from inspect_ai import Task + from inspect_swe import claude_code + + skill_dir = config.active().skill_path(skill) + samples = [convert.sample_from_case(c, skill_dir, ctx) for c in cases] + + bound = deadline.active() + return Task( + name=f"claude-code-{skill}", + dataset=samples, + # `skills=` installs into .claude/skills inside the sandbox, which is + # where the real harness looks -- the point of this leg is that its + # discovery machinery, not ours, decides what happens. + solver=claude_code(skills=[skill_dir]), + scorer=scorers.expectations(), + sandbox=sandbox_spec.for_skill(skill), + message_limit=behavioral.message_limit_for(model), + time_limit=int(bound.remaining()) if bound is not None else None, + ) + + +def run( + skills: list[str], cases: list[Case], model: str, effort: str +) -> list[BehaviorOutcome]: + """Mirrors `behavior.run`, so the CLI and the benchmark treat it the same.""" + from inspect_ai import eval as inspect_eval + + require() + + sandbox_spec.require_provider() + + outcomes: list[BehaviorOutcome] = [] + for skill in skills: + skill_cases = [c for c in cases if c.skill == skill and c.has_behavior] + if not skill_cases: + continue + + print(f"[claude-code] {skill}: {len(skill_cases)} case(s)", flush=True) + logs = inspect_eval( + build_task(skill, skill_cases, model), + model=model, + model_args=models.model_args(model), + log_dir=str(Path(".skillscope") / "logs"), + display="plain", + ) + for log in logs: + stats.record_log(log) + outcomes.extend(behavioral._outcomes(log, skill, skill_cases)) + + for outcome in outcomes: + passed = sum(1 for c in outcome.checks if c["passed"]) + print( + f" [{'PASS' if outcome.passed else 'FAIL'}] {outcome.id}: " + f"{passed}/{len(outcome.checks)} checks in {outcome.elapsed_s}s" + + (f" -- {outcome.error}" if outcome.error else ""), + flush=True, + ) + return outcomes diff --git a/skillscope/routing.py b/skillscope/routing.py index fbaac65..6b28d83 100644 --- a/skillscope/routing.py +++ b/skillscope/routing.py @@ -49,7 +49,7 @@ from dataclasses import asdict, dataclass, field from pathlib import Path -from . import deadline +from . import deadline, usage from .agent import claude_env from .datasets import Case @@ -524,6 +524,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> except json.JSONDecodeError: continue events.append(event) + usage.record_stream_event(event) reported = _init_skills(event, skills) if reported is not None: @@ -543,6 +544,7 @@ def run_case(case: Case, routing_set: dict[str, Path], config: RoutingConfig) -> if event.get("type") == "result": stop_reason = "result" + usage.record_stream_event(event) if event.get("is_error"): error = str(event.get("result") or "result event reported an error")[:400] break diff --git a/skillscope/schema/machine.schema.json b/skillscope/schema/machine.schema.json index 8f5997e..f3f55a5 100644 --- a/skillscope/schema/machine.schema.json +++ b/skillscope/schema/machine.schema.json @@ -19,6 +19,12 @@ "items": { "type": "string", "minLength": 1 }, "description": "Extra `runs-on` labels the behavioral cases need, added to the base labels the workflow supplies. Name the hardware, not the pool: `mi300x` says what the skill requires and lands it on any runner registered with that label. A leg that asks for labels is treated as scoped, so it is also what the repo may hold behind a gate label and pay for from a separate environment. Keep the list as short as the runners allow -- every label is a condition a pool has to satisfy, and a label no runner carries is a job that queues forever rather than an error.", "examples": [["mi300x"]] + }, + "sandbox": { + "type": "string", + "minLength": 1, + "description": "Compose file, relative to the skill directory, describing the sandbox the behavioral cases need under the inspect engine. Absent means the default container with no network, which is what a skill that only reads and writes files should want. Name one to opt into network egress -- a skill that installs a server or pulls a model cannot run without it -- or to bind a device in. Ignored on Windows, where inspect's sandbox layer assumes a POSIX guest and cases run unsandboxed on the host instead.", + "examples": ["compose.yaml"] } } } diff --git a/skillscope/usage.py b/skillscope/usage.py new file mode 100644 index 0000000..2160a1c --- /dev/null +++ b/skillscope/usage.py @@ -0,0 +1,97 @@ +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""What a graded run spent, recorded by whichever engine ran it. + +Both engines know their own cost and neither reported it: the legacy engine +discards the `total_cost_usd` the CLI hands back with every result, and inspect +keeps usage in its own `.eval` log where skillscope's report never looks. That +was fine while there was one engine and nothing to compare it against. + +Accumulated in module state rather than threaded through return values, because +the engines' entry points return outcome lists and that signature is what lets +the CLI swap one for the other in a single line. A run is a process, so the +scope is right even if the shape is blunt; `reset()` exists for tests. +""" + +from __future__ import annotations + +from dataclasses import dataclass, field + + +@dataclass +class Usage: + """Totals for one graded run. Fields are None when the engine cannot say.""" + + input_tokens: int = 0 + output_tokens: int = 0 + cost_usd: float | None = None + calls: int = 0 + + @property + def total_tokens(self) -> int: + return self.input_tokens + self.output_tokens + + def as_meta(self) -> dict: + """The shape that goes into a report's `meta`, omitting what is unknown.""" + meta: dict = { + "model_calls": self.calls, + "input_tokens": self.input_tokens, + "output_tokens": self.output_tokens, + "total_tokens": self.total_tokens, + } + if self.cost_usd is not None: + meta["cost_usd"] = round(self.cost_usd, 4) + return meta + + +_current: Usage = Usage() + + +def reset() -> None: + global _current + _current = Usage() + + +def snapshot() -> Usage: + return _current + + +def record( + *, + input_tokens: int = 0, + output_tokens: int = 0, + cost_usd: float | None = None, + calls: int = 1, +) -> None: + """Add one model interaction's cost to the run.""" + _current.input_tokens += int(input_tokens or 0) + _current.output_tokens += int(output_tokens or 0) + _current.calls += int(calls or 0) + if cost_usd is not None: + _current.cost_usd = (_current.cost_usd or 0.0) + float(cost_usd) + + +def record_stream_event(event: dict) -> None: + """Record what one `claude` stream-json event says the run has spent. + + Shared by both legacy commands, because they read the same stream and a + column that means "responses" in one and "cases" in the other is worse than + no column at all. + + Tokens and responses come from assistant events, one per model reply. Cost + comes only from the result event, where it is a run total -- and a routing + case is normally killed before that event arrives, so it reports responses + with no cost. That is what the legacy engine can actually observe. + """ + kind = event.get("type") + if kind == "assistant": + message = event.get("message") + counts = (message or {}).get("usage") if isinstance(message, dict) else None + record( + input_tokens=(counts or {}).get("input_tokens", 0), + output_tokens=(counts or {}).get("output_tokens", 0), + ) + elif kind == "result": + record(cost_usd=event.get("total_cost_usd"), calls=0) diff --git a/tests/test_skillscope.py b/tests/test_skillscope.py index 11cdee6..fae7a4e 100644 --- a/tests/test_skillscope.py +++ b/tests/test_skillscope.py @@ -48,6 +48,12 @@ ) from skillscope import selection as select_module from skillscope.datasets import EVALUATIONS_KEY, TRIGGER_KEY +from skillscope.engine import behavioral as engine_behavioral +from skillscope.engine import judge as engine_judge +from skillscope.engine import models as engine_models +from skillscope.engine import routing as engine_routing +from skillscope.engine import sandbox as engine_sandbox +from skillscope.engine import tools as engine_tools REPO_ROOT = datasets.PACKAGE_DIR.parent SCHEMA_DIR = datasets.PACKAGE_DIR / "schema" @@ -328,13 +334,16 @@ def setUp(self) -> None: def test_documented_keys_match_the_parser(self) -> None: self.assertEqual(set(self.schema["properties"]), datasets.MACHINE_KEYS) - def test_neither_key_is_enumerated_in_the_schema(self) -> None: - # Neither can be: a label means whatever a repo registered its runners - # with, so the schema documents what the key is for and the workflow - # supplies the labels around it. - for key in datasets.MACHINE_KEYS: + def test_no_list_key_is_enumerated_in_the_schema(self) -> None: + # Neither `os` nor `labels` can be: a label means whatever a repo + # registered its runners with, so the schema documents what the key is + # for and the workflow supplies the labels around it. Scoped to the + # list-valued keys, since `sandbox` names a file rather than a set. + for key, spec in self.schema["properties"].items(): + if spec.get("type") != "array": + continue with self.subTest(key=key): - self.assertNotIn("enum", self.schema["properties"][key]["items"]) + self.assertNotIn("enum", spec["items"]) def test_every_machine_yml_in_the_repo_resolves(self) -> None: for skill in datasets.declared_skills(): @@ -2501,5 +2510,394 @@ def test_an_empty_room_leaves_only_the_shared_pool(self) -> None: self.assertTrue(all(case.skill is None for case in cases)) +class TestCiModelPin(unittest.TestCase): + """The pin keeps paid runs comparable; a mock is neither paid nor graded.""" + + def test_a_real_model_is_pinned_under_ci(self) -> None: + with mock.patch.dict(os.environ, {"CI": "true"}): + self.assertEqual(agent.enforce_model_policy("sonnet"), "opus") + + def test_a_mock_is_left_alone_under_ci(self) -> None: + # Otherwise the free wiring run becomes a run that needs a key, in the + # one place where not needing a key is the whole point. + with mock.patch.dict(os.environ, {"CI": "true"}): + self.assertEqual( + agent.enforce_model_policy("mockllm/model"), "mockllm/model" + ) + + def test_nothing_is_pinned_outside_ci(self) -> None: + with mock.patch.dict(os.environ, {"CI": "", "GITHUB_ACTIONS": ""}): + self.assertEqual(agent.enforce_model_policy("sonnet"), "sonnet") + + +class TestEngineMessageLimit(unittest.TestCase): + """A model that cannot finish should not be given a hundred turns to prove it.""" + + def test_a_real_model_gets_the_full_budget(self) -> None: + self.assertEqual( + engine_behavioral.message_limit_for("anthropic/claude-opus-5"), + engine_behavioral.MESSAGE_LIMIT, + ) + + def test_a_mock_gets_a_short_one(self) -> None: + # It never calls submit, so it loops to whatever cap it is given, and + # every turn is a real sandbox round trip. + self.assertEqual( + engine_behavioral.message_limit_for("mockllm/model"), + engine_behavioral.MOCK_MESSAGE_LIMIT, + ) + + +class TestEngineModelNames(unittest.TestCase): + """`--model` speaks the claude CLI's aliases; inspect wants provider names.""" + + def test_an_alias_becomes_a_provider_qualified_name(self) -> None: + self.assertEqual(engine_models.resolve("opus"), "anthropic/claude-opus-5") + + def test_an_alias_is_case_insensitive(self) -> None: + self.assertEqual(engine_models.resolve("Opus"), "anthropic/claude-opus-5") + + def test_a_qualified_name_passes_through(self) -> None: + # What makes `--model mockllm/model` work for the no-cost wiring runs. + self.assertEqual(engine_models.resolve("mockllm/model"), "mockllm/model") + + def test_an_unknown_bare_name_is_assumed_to_be_anthropic(self) -> None: + self.assertEqual(engine_models.resolve("claude-x"), "anthropic/claude-x") + + +class TestEngineGatewayHeaders(unittest.TestCase): + """`ANTHROPIC_CUSTOM_HEADERS` is a claude CLI variable; inspect ignores it.""" + + def setUp(self) -> None: + for var in (engine_models.CUSTOM_HEADERS_ENV, engine_models.AUTH_TOKEN_ENV): + self.addCleanup(os.environ.pop, var, None) + os.environ.pop(var, None) + + def test_no_headers_configured_means_no_provider_arguments(self) -> None: + self.assertEqual(engine_models.model_args("anthropic/claude-opus-5"), {}) + + def test_headers_are_parsed_into_default_headers(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = ( + "X-Subscription-Key: secret\nuser: ci-runner\n" + ) + self.assertEqual( + engine_models.model_args("anthropic/claude-opus-5"), + { + "default_headers": { + "X-Subscription-Key": "secret", + "user": "ci-runner", + } + }, + ) + + def test_a_value_containing_a_colon_survives(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "Referer: https://example.com/x" + self.assertEqual( + engine_models.custom_headers(), {"Referer": "https://example.com/x"} + ) + + def test_blank_and_malformed_lines_are_skipped(self) -> None: + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "\nnot-a-header\n\nk: v\n" + self.assertEqual(engine_models.custom_headers(), {"k": "v"}) + + def test_a_non_anthropic_model_needs_no_gateway_arguments(self) -> None: + # The free wiring run reaches no provider, so a shell that happens to + # hold both Anthropic variables must not break the one check that costs + # nothing -- and those are exactly the machines that have an OAuth token. + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "k: v" + os.environ[engine_models.AUTH_TOKEN_ENV] = "token" + self.assertEqual(engine_models.model_args("mockllm/model"), {}) + + def test_oauth_and_gateway_headers_together_are_refused(self) -> None: + # inspect's OAuth path sets `default_headers` itself, so ours would be a + # duplicate keyword argument deep inside the SDK. Fail with the reason. + os.environ[engine_models.CUSTOM_HEADERS_ENV] = "k: v" + os.environ[engine_models.AUTH_TOKEN_ENV] = "token" + with self.assertRaises(SystemExit) as caught: + engine_models.model_args("anthropic/claude-opus-5") + self.assertIn(engine_models.AUTH_TOKEN_ENV, str(caught.exception)) + + +class TestEngineListingNormalisation(unittest.TestCase): + """`find` and `Get-ChildItem` disagree about separators and prefixes.""" + + def test_posix_output(self) -> None: + listing = "./out.png\n./docs/plan.md\n" + self.assertEqual( + engine_tools.normalize_listing(listing), ["docs/plan.md", "out.png"] + ) + + def test_windows_output(self) -> None: + listing = ".\\out.png\r\n.\\docs\\plan.md\r\n" + self.assertEqual( + engine_tools.normalize_listing(listing), ["docs/plan.md", "out.png"] + ) + + def test_the_installed_skill_does_not_satisfy_files_exist(self) -> None: + # The harness put it there, so a case asserting SKILL.md was produced + # would otherwise pass without the agent doing anything. + listing = "./skills/demo/SKILL.md\n./.claude/settings.json\n./out.png\n" + self.assertEqual(engine_tools.normalize_listing(listing), ["out.png"]) + + def test_blank_lines_are_dropped(self) -> None: + self.assertEqual(engine_tools.normalize_listing("\n\n \n"), []) + + +class TestEngineJudgeVerdicts(unittest.TestCase): + """A grader is chatty and its reasons contain punctuation.""" + + def test_a_bare_verdict(self) -> None: + self.assertEqual( + engine_judge.parse_verdict('{"pass": true, "reason": "it did"}'), + (True, "it did"), + ) + + def test_a_verdict_wrapped_in_prose(self) -> None: + text = 'Looking at the evidence...\n{"pass": false, "reason": "no file"}\nDone.' + self.assertEqual(engine_judge.parse_verdict(text), (False, "no file")) + + def test_a_reason_containing_braces(self) -> None: + # A regex quantifier or a quoted snippet in the reason must not confuse + # the scan, which is why boundaries are decoded rather than matched. + text = '{"pass": true, "reason": "matched a{2,3} in the output"}' + self.assertEqual( + engine_judge.parse_verdict(text), (True, "matched a{2,3} in the output") + ) + + def test_the_last_verdict_wins(self) -> None: + text = '{"pass": true, "reason": "first"}\n{"pass": false, "reason": "second"}' + self.assertEqual(engine_judge.parse_verdict(text), (False, "second")) + + def test_no_verdict_at_all(self) -> None: + self.assertIsNone(engine_judge.parse_verdict("I could not decide.")) + + def test_a_missing_reason_still_yields_a_verdict(self) -> None: + self.assertEqual( + engine_judge.parse_verdict('{"pass": true}'), (True, "(no reason given)") + ) + + +class TestEngineJudgePolarity(unittest.TestCase): + """The judge grades the requirement; callers must never negate the verdict.""" + + def test_a_must_requirement_asks_whether_it_happened(self) -> None: + text = engine_judge.requirement_text("generate an image", must_happen=True) + self.assertIn("MUST have done this", text) + self.assertIn("true if the agent did it", text) + + def test_a_must_not_requirement_asks_whether_it_was_avoided(self) -> None: + # Read as a pass when the agent avoided it: negating this verdict is + # what turns a correct run into a failure. + text = engine_judge.requirement_text("call a cloud API", must_happen=False) + self.assertIn("MUST NOT have done this", text) + self.assertIn("true if the agent avoided it", text) + self.assertIn("default verdict is true", text) + + +class TestEngineJudgeTruncation(unittest.TestCase): + """What settles a check is usually the last thing the agent did.""" + + def test_short_transcripts_are_untouched(self) -> None: + self.assertEqual(engine_judge._elide_middle("abc", 100), "abc") + + def test_the_end_survives(self) -> None: + # Cutting the tail would drop the validator run that a "did it verify + # its work" expectation turns on, making the agent look like it lied. + text = "START" + ("x" * 5000) + "VALIDATED" + trimmed = engine_judge._elide_middle(text, 400) + self.assertTrue(trimmed.startswith("START")) + self.assertTrue(trimmed.endswith("VALIDATED")) + self.assertIn("elided", trimmed) + self.assertLess(len(trimmed), 600) + + +class _State: + def __init__(self, messages, output=None) -> None: + self.messages = messages + self.output = output + + +class _Output: + def __init__(self, completion: str) -> None: + self.completion = completion + + +class _Assistant: + role = "assistant" + + def __init__(self, content: str) -> None: + self.content = content + + +class TestEngineJudgeFinalMessage(unittest.TestCase): + """A `react` agent answers through submit, not through a chat message.""" + + def test_the_submitted_answer_wins(self) -> None: + # The last assistant message is often the preamble that introduces the + # answer. Grading that instead shows the judge a description of the + # work rather than the work. + state = _State( + [_Assistant("Here are the commands you need:")], + _Output("curl -X POST /api/v1/pull -d '{...}'"), + ) + self.assertIn("curl -X POST", engine_judge.final_message_of(state)) + + def test_it_falls_back_to_the_last_assistant_message(self) -> None: + state = _State([_Assistant("no submit tool in this agent")], None) + self.assertEqual( + engine_judge.final_message_of(state), "no submit tool in this agent" + ) + + def test_silence_is_reported_rather_than_guessed_at(self) -> None: + self.assertEqual( + engine_judge.final_message_of(_State([], None)), "(the agent said nothing)" + ) + + +class TestEngineJudgeArtifacts(unittest.TestCase): + def test_images_are_recognised_by_suffix(self) -> None: + self.assertTrue(engine_judge.is_image("out.PNG")) + self.assertTrue(engine_judge.is_image("art/cat.jpeg")) + self.assertFalse(engine_judge.is_image("notes.md")) + + def test_known_binaries_are_not_read_as_text(self) -> None: + self.assertTrue(engine_judge.is_probably_binary("model.safetensors")) + self.assertFalse(engine_judge.is_probably_binary("report.md")) + + +class _Call: + def __init__(self, function: str, arguments: dict) -> None: + self.function = function + self.arguments = arguments + + +class _Message: + def __init__(self, tool_calls: list | None = None) -> None: + self.tool_calls = tool_calls + + +class TestEngineRoutingActivation(unittest.TestCase): + """Naming a skill through the tool *is* the activation, so it is observed.""" + + def test_a_skill_call_is_the_decision(self) -> None: + messages = [_Message([_Call("skill", {"command": "local-ai-use"})])] + self.assertEqual(engine_routing.activation_of(messages), "local-ai-use") + + def test_no_tool_call_means_nothing_activated(self) -> None: + self.assertIsNone(engine_routing.activation_of([_Message(), _Message([])])) + + def test_another_tool_is_not_an_activation(self) -> None: + messages = [_Message([_Call("think", {"thought": "skill demo-skill?"})])] + self.assertIsNone(engine_routing.activation_of(messages)) + + def test_the_first_skill_named_wins(self) -> None: + messages = [ + _Message([_Call("skill", {"command": "first"})]), + _Message([_Call("skill", {"command": "second"})]), + ] + self.assertEqual(engine_routing.activation_of(messages), "first") + + def test_a_blank_command_is_not_an_activation(self) -> None: + messages = [_Message([_Call("skill", {"command": " "})])] + self.assertIsNone(engine_routing.activation_of(messages)) + + def test_tool_calls_are_counted_across_messages(self) -> None: + messages = [ + _Message([_Call("think", {}), _Call("skill", {"command": "x"})]), + _Message(), + ] + self.assertEqual(engine_routing.tool_call_count(messages), 2) + + +class TestEngineSandboxSelection(unittest.TestCase): + """The provider is the machine's choice; the compose file is the skill's.""" + + def setUp(self) -> None: + self.addCleanup(os.environ.pop, engine_sandbox.SANDBOX_ENV, None) + os.environ.pop(engine_sandbox.SANDBOX_ENV, None) + self.repo = Repo(self) + # Pinned, because the answer depends on the platform and the suite runs + # on both. Without this these assertions quietly mean something + # different on a Windows runner than on a Linux one. + self._posix_host() + + def _posix_host(self) -> None: + patch = mock.patch.object(engine_sandbox, "is_windows", lambda: False) + patch.start() + self.addCleanup(patch.stop) + + def _windows_host(self) -> None: + patch = mock.patch.object(engine_sandbox, "is_windows", lambda: True) + patch.start() + self.addCleanup(patch.stop) + + def _skill(self, machine: str | None = None, compose: bool = False) -> None: + folder = self.repo.skill( + "boxed", dataset=tier0_dataset("boxed"), machine=machine + ) + if compose: + (folder / "compose.yaml").write_text("services: {}\n", encoding="utf-8") + self.repo.activate() + + def test_docker_by_default(self) -> None: + self._skill() + self.assertEqual(engine_sandbox.for_skill("boxed"), "docker") + + def test_the_env_var_selects_the_provider(self) -> None: + self._skill() + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + self.assertEqual(engine_sandbox.for_skill("boxed"), "podman") + + def test_a_declared_compose_file_rides_along(self) -> None: + self._skill(machine="sandbox: compose.yaml\n", compose=True) + provider, config = engine_sandbox.for_skill("boxed") + self.assertEqual(provider, "docker") + self.assertTrue(config.endswith("compose.yaml")) + + def test_selecting_a_provider_keeps_the_skill_s_compose_file(self) -> None: + # The skill asked for network egress; choosing podman must not drop it, + # or the case runs without what it needs and fails unexplainably. + self._skill(machine="sandbox: compose.yaml\n", compose=True) + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + provider, config = engine_sandbox.for_skill("boxed") + self.assertEqual(provider, "podman") + self.assertTrue(config.endswith("compose.yaml")) + + def test_local_takes_no_configuration(self) -> None: + self._skill(machine="sandbox: compose.yaml\n", compose=True) + os.environ[engine_sandbox.SANDBOX_ENV] = "local" + self.assertEqual(engine_sandbox.for_skill("boxed"), "local") + + def test_windows_has_no_sandbox_available(self) -> None: + # inspect's sandbox layer assumes a POSIX guest, so those legs run + # unsandboxed -- and a compose file the skill declared cannot apply, + # because there is no container to apply it to. + self._windows_host() + self._skill(machine="sandbox: compose.yaml\n", compose=True) + self.assertEqual(engine_sandbox.for_skill("boxed"), "local") + + def test_an_unresolvable_provider_says_what_to_install(self) -> None: + # The binary being present proves nothing: inspect resolves a + # third-party provider through an entry point, so the Python package + # has to be installed too. Its own error names neither the variable + # nor the package. + os.environ[engine_sandbox.SANDBOX_ENV] = "podman" + + def unresolvable(name: str): + raise ValueError(f"SandboxEnvironment type {name!r} not recognized.") + + with self.assertRaises(SystemExit) as caught: + engine_sandbox.require_provider(resolve=unresolvable) + message = str(caught.exception) + self.assertIn(engine_sandbox.SANDBOX_ENV, message) + self.assertIn("skillscope[podman]", message) + + def test_a_named_compose_file_that_is_missing_is_an_error(self) -> None: + self._skill(machine="sandbox: nope.yaml\n") + with self.assertRaises(SystemExit) as caught: + engine_sandbox.for_skill("boxed") + self.assertIn("nope.yaml", str(caught.exception)) + + if __name__ == "__main__": unittest.main(verbosity=2) diff --git a/tools/benchmark_engines.py b/tools/benchmark_engines.py new file mode 100644 index 0000000..2064e00 --- /dev/null +++ b/tools/benchmark_engines.py @@ -0,0 +1,256 @@ +#!/usr/bin/env python3 +# Copyright Advanced Micro Devices, Inc. +# +# SPDX-License-Identifier: MIT + +"""Compare the legacy and inspect engines on the same dataset. + +Answers the two questions a migration has to answer before it can be trusted. + +**Does the new engine agree?** Per case, not in aggregate: an accuracy figure +can match exactly while individual cases flip in both directions and cancel +out. Flips are reported by direction, and against a measured noise floor -- +routing and behavioral are both nondeterministic, so "these two runs differ" +means nothing until you know how much one engine differs from itself. + +**Does it pay for itself?** Wall clock and tokens per run, from the report +`meta` both engines now populate. + +Runs through the `skillscope` CLI rather than importing either engine, so what +is measured is what CI executes. + + tools/benchmark_engines.py routing --routing-room my-skill --noise + tools/benchmark_engines.py behavioral --skill my-skill + tools/benchmark_engines.py --compare legacy.json inspect.json +""" + +from __future__ import annotations + +import argparse +import json +import subprocess +import sys +import tempfile +from pathlib import Path + +AGREE = "agree" +NEW_PASSES = "only the inspect engine passes" +NEW_FAILS = "only the legacy engine passes" + + +def run_leg(leg: str, engine: str, passthrough: list[str], label: str) -> dict: + """Run one leg on one engine and return its JSON report.""" + out = Path(tempfile.mkdtemp(prefix="benchmark-")) / f"{label}.json" + cmd = [ + sys.executable, "-m", "skillscope", leg, + "--engine", engine, "--output", str(out), *passthrough, + ] + print(f"[benchmark] {label}: {' '.join(cmd)}", flush=True) + # A failing leg is a result, not an error: a run where cases fail still + # produces the report this compares. + subprocess.run(cmd, check=False) + if not out.is_file(): + raise SystemExit(f"error: {label} produced no report at {out}") + return json.loads(out.read_text(encoding="utf-8")) + + +def cases_by_id(report: dict) -> dict[str, dict]: + return {str(case["id"]): case for case in report.get("cases", [])} + + +def compare(baseline: dict, candidate: dict) -> dict: + """Per-case comparison of two reports of the same dataset.""" + left, right = cases_by_id(baseline), cases_by_id(candidate) + shared = sorted(set(left) & set(right)) + + rows = [] + for case_id in shared: + a, b = left[case_id], right[case_id] + if a["passed"] == b["passed"]: + direction = AGREE + else: + direction = NEW_PASSES if b["passed"] else NEW_FAILS + rows.append( + { + "id": case_id, + "direction": direction, + "baseline_passed": a["passed"], + "candidate_passed": b["passed"], + # Routing carries the decision itself, which says more than + # pass/fail: two engines can both fail a case for different + # reasons, and that is not agreement. + "baseline_observed": a.get("observed"), + "candidate_observed": b.get("observed"), + "baseline_verdict": a.get("verdict"), + "candidate_verdict": b.get("verdict"), + } + ) + + agreed = sum(1 for r in rows if r["direction"] == AGREE) + return { + "compared": len(rows), + "agreed": agreed, + "agreement": round(agreed / len(rows), 4) if rows else None, + "flips": [r for r in rows if r["direction"] != AGREE], + "only_in_baseline": sorted(set(left) - set(right)), + "only_in_candidate": sorted(set(right) - set(left)), + "rows": rows, + } + + +def spend(report: dict) -> dict: + meta = report.get("meta", {}) + return { + "engine": meta.get("engine", "legacy"), + "wall_time_s": meta.get("wall_time_s"), + "model_calls": meta.get("model_calls"), + "total_tokens": meta.get("total_tokens"), + "cost_usd": meta.get("cost_usd"), + } + + +def _cell(value) -> str: + return "n/a" if value is None else str(value) + + +def _spend_caveats(spend: dict) -> list[str]: + """Say which columns are comparable, because not all of them are. + + The two engines count different things and silently tabulating them side by + side invites the wrong conclusion. Wall time is always comparable. Tokens + are not: the legacy engine reads them from assistant events, which exclude + the system prompt and cached input, and a routing case is killed before the + totals arrive -- so its figure is a floor, not a total. Cost is the legacy + engine's trustworthy number, and inspect only has one when the provider + supplies pricing, which a gateway generally does not. + """ + engines = {spend[label]["engine"] for label in ("baseline", "candidate")} + notes = [] + if "legacy" in engines: + notes.append( + "> Legacy token counts are a floor: they omit the system prompt and " + "cached input, and a killed case never reports its totals. Compare " + "cost and wall time, not tokens." + ) + if any(spend[label]["cost_usd"] is None for label in ("baseline", "candidate")): + notes.append( + "> One engine reported no cost -- inspect only has one when the " + "model provider supplies pricing, which a gateway generally does " + "not. Wall time and model calls are comparable on both sides; " + "model calls in particular is the like-for-like measure of how " + "much work each engine asks of the model per case." + ) + return notes + + +def render(result: dict) -> str: + comparison = result["comparison"] + lines = [ + "## Engine benchmark", + "", + f"**{comparison['agreed']}/{comparison['compared']} cases agree** " + f"between the legacy and inspect engines.", + "", + ] + + noise = result.get("noise") + if noise is not None: + lines += [ + f"Noise floor: the legacy engine agrees with itself on " + f"{noise['agreed']}/{noise['compared']} cases. Treat any difference " + "at or below that as run-to-run variance rather than engine drift.", + "", + ] + else: + lines += [ + "_No noise floor measured; re-run with `--noise` before reading the " + "flips below as engine differences._", + "", + ] + + lines += ["| Run | Wall time | Model calls | Tokens | Cost |", "| --- | --- | --- | --- | --- |"] + for label in ("baseline", "candidate"): + s = result["spend"][label] + lines.append( + f"| {label} (`{s['engine']}`) | {_cell(s['wall_time_s'])}s | " + f"{_cell(s['model_calls'])} | {_cell(s['total_tokens'])} | " + f"{_cell(s['cost_usd'])} |" + ) + lines += ["", *_spend_caveats(result["spend"])] + + lines += ["", "### Cases that flipped", ""] + if not comparison["flips"]: + lines.append("None. Every shared case reached the same verdict on both engines.") + else: + lines += [ + "| Case | Direction | Legacy | Inspect |", + "| --- | --- | --- | --- |", + ] + for flip in comparison["flips"]: + left = flip["baseline_verdict"] or ("pass" if flip["baseline_passed"] else "fail") + right = flip["candidate_verdict"] or ("pass" if flip["candidate_passed"] else "fail") + lines.append(f"| `{flip['id']}` | {flip['direction']} | {left} | {right} |") + + for key, heading in ( + ("only_in_baseline", "Only the legacy run produced these cases"), + ("only_in_candidate", "Only the inspect run produced these cases"), + ): + missing = comparison[key] + if missing: + lines += ["", f"### {heading}", "", ", ".join(f"`{m}`" for m in missing)] + + return "\n".join(lines) + "\n" + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__.splitlines()[0]) + parser.add_argument("leg", nargs="?", choices=["routing", "behavioral"]) + parser.add_argument( + "--compare", + nargs=2, + metavar=("LEGACY", "INSPECT"), + help="Compare two reports that already exist instead of running the legs.", + ) + parser.add_argument( + "--noise", + action="store_true", + help=( + "Run the legacy engine twice to measure how much it disagrees with " + "itself. Without this the flip list cannot be read as engine drift." + ), + ) + parser.add_argument("--output", default="", help="Write the JSON result here.") + args, passthrough = parser.parse_known_args(argv) + + if args.compare: + baseline = json.loads(Path(args.compare[0]).read_text(encoding="utf-8")) + candidate = json.loads(Path(args.compare[1]).read_text(encoding="utf-8")) + noise = None + else: + if not args.leg: + parser.error("give a leg to run (routing or behavioral), or --compare") + baseline = run_leg(args.leg, "legacy", passthrough, "legacy") + noise_run = ( + run_leg(args.leg, "legacy", passthrough, "legacy-again") + if args.noise + else None + ) + candidate = run_leg(args.leg, "inspect", passthrough, "inspect") + noise = compare(baseline, noise_run) if noise_run is not None else None + + result = { + "comparison": compare(baseline, candidate), + "noise": noise, + "spend": {"baseline": spend(baseline), "candidate": spend(candidate)}, + } + + report = render(result) + print(report) + if args.output: + Path(args.output).write_text(json.dumps(result, indent=2), encoding="utf-8") + print(f"[benchmark] JSON result: {args.output}") + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())