diff --git a/src/agent_env/bundle/run.py b/src/agent_env/bundle/run.py index 8bcec78d..839df618 100644 --- a/src/agent_env/bundle/run.py +++ b/src/agent_env/bundle/run.py @@ -29,7 +29,7 @@ from agent_env.store.routing import namespace_routing, run_scope from agent_env.task import Task, record_task_cancelled from agent_env.task.interrupts import Interrupts -from agent_env.task.task import recorded_scores +from agent_env.task.task import FailedCriterion, failed_criteria, recorded_scores from agent_env.task.teardown import TeardownReport, teardown_run from agent_env.task_step.context import TaskStepContext from agent_env.utils.docker_build import DEFAULT_BUILD_PLATFORM @@ -83,6 +83,11 @@ def scores(self) -> dict[str, float]: when that isn't a step's verifier id.""" return recorded_scores(self.task.steps, self.context) + @property + def failed_criteria(self) -> list[FailedCriterion]: + """Each criterion a verification scoring below 1 recorded as failed, with the reason it gave.""" + return failed_criteria(self.task.steps, self.context) + @property def outcome(self) -> Outcome: if self.cancelled: @@ -112,6 +117,11 @@ class BundleRun: def failed(self) -> bool: return any(run.outcome is Outcome.FAILED for run in self.runs) + def below(self, threshold: float) -> tuple[TaskRun, ...]: + """The runs that finished but scored below ``threshold``, or recorded no score, which can't show it passed.""" + return tuple(run for run in self.runs if run.outcome in (Outcome.PASSED, Outcome.BELOW_ONE, Outcome.UNSCORED) + and (not run.scores or min(run.scores.values()) < threshold)) + def path(self, entry: BundleEntry) -> str: """``entry``'s path in the bundle, as the summary names it (``tasks/hello.json``).""" return relative(self.materialization.plan.bundle.bundle.root, entry.path) diff --git a/src/agent_env/cli/run.py b/src/agent_env/cli/run.py index a57137f7..4f5c1bfb 100644 --- a/src/agent_env/cli/run.py +++ b/src/agent_env/cli/run.py @@ -4,11 +4,13 @@ import os import shlex +import sys import time import traceback from pathlib import Path import click +from click.core import ParameterSource from agent_env.bundle import (BundleError, BundleKind, BundleRun, DryRun, Outcome, RunInterrupted, TaskRun, dry_run_bundle, run_bundle) @@ -25,6 +27,8 @@ _ENV_ENDPOINTS = (("mcp", "mcp_url"), ("gateway", "gateway_url"), ("pgweb", "db_web_url"), ("db-mcp", "db_mcp_url"), ("vnc", "vnc_url"), ("expires", "expires_at_utc")) _DRY_RUN = "Dry run: nothing is written or run." +_SHOWN_CRITERIA = 5 +_REASON_CHARS = 160 @click.command() @@ -39,41 +43,48 @@ help="Keep the sandboxes up after the runs, print their endpoints, and tear them down on Ctrl-C.") @click.option("--dry-run", is_flag=True, help="Check the bundle as the run does and show what it would write and run; write and run nothing.") +@click.option("--fail-under", type=click.FloatRange(0, 1), default=None, metavar="SCORE", + help="Exit 1 when a task scored below SCORE, or recorded no score; --fail-under 1 fails on any miss.") @click.pass_context def run(ctx: click.Context, bundle: str | None, tasks: tuple[str, ...], evals: tuple[str, ...], model: str | None, - sandbox: str | None, keep: bool, dry_run: bool): + sandbox: str | None, keep: bool, dry_run: bool, fail_under: float | None): """Run a bundle's tasks and evals, or list the installed bundles. BUNDLE is a folder, or the name of a bundle an installed package provides (PACKAGE/NAME when several packages provide that name). Without BUNDLE, it lists the installed bundles. With no --task or --eval, it runs every eval or, in a bundle without evals, every task. Each task runs - once, at most four at a time, and the run exits 1 if any of them failed. With --verbose, a run that - raised prints its traceback. + once, at most four at a time, and the run exits 1 if any of them failed. A task that scored below 1 lists + the criteria that failed and why, and exits 1 only under --fail-under, which also fails a task that + recorded no score. With --verbose, a run that raised prints its traceback. --dry-run makes the checks the run makes before its first task, reading the stores as the run does, and prints what it would write and run, writing and running nothing. It exits 1 on a problem the run would - stop at, and 0 otherwise; --model and --keep change nothing it shows. It takes no lock, so another run - can write first and change what it shows. + stop at, and 0 otherwise; --model, --keep and --fail-under change nothing it shows. It takes no lock, so + another run can write first and change what it shows. Each run's sandboxes are torn down as it ends; its instance and outputs stay. --keep holds them up until Ctrl-C instead. Ctrl-C or SIGTERM mid-run cancels the runs, tears them down, prints what ran and exits 130 (143 for SIGTERM); a second one stops the teardown and prints what is still up. """ if bundle is None: - if tasks or evals or model or sandbox or keep or dry_run: - raise click.UsageError("--task, --eval, --model, --sandbox, --keep and --dry-run need a BUNDLE to run") + given = [param.opts[0] for param in ctx.command.params if isinstance(param, click.Option) + and ctx.get_parameter_source(param.name) not in (None, ParameterSource.DEFAULT)] + if given: + *rest, last = given + raise click.UsageError(f"{', '.join(rest)} and {last} need a BUNDLE to run" if rest + else f"{last} needs a BUNDLE to run") _list_installed() return root, id_root = _locate(bundle) verbose = ctx.find_root().params.get("verbose") try: if dry_run: - click.echo(_DRY_RUN) - _report_dry_run(dry_run_bundle(root, tasks=tasks, evals=evals, sandbox=sandbox, on_progress=click.echo, - id_root=id_root)) + _echo(_DRY_RUN) + dry = dry_run_bundle(root, tasks=tasks, evals=evals, sandbox=sandbox, on_progress=_echo, id_root=id_root) + _report_dry_run(dry) return - result = run_bundle(root, tasks=tasks, evals=evals, model=model, sandbox=sandbox, on_progress=click.echo, + result = run_bundle(root, tasks=tasks, evals=evals, model=model, sandbox=sandbox, on_progress=_echo, id_root=id_root, keep=keep) except BundleError as e: _note_an_installed_namesake(e, bundle, root, id_root) @@ -88,10 +99,30 @@ def run(ctx: click.Context, bundle: str | None, tasks: tuple[str, ...], evals: t if keep: result = _hold(result, interrupts) _report_teardown(result) - if result.failed: + below = result.below(fail_under) if fail_under is not None else () + if below: + _report_below(result, below, fail_under) + if result.failed or below: raise SystemExit(1) +def _echo(message: str | None = None, err: bool = False) -> None: + """``click.echo``, which drops the line once its stream's reader has gone, as when piped into head, so the run + still finishes and tears down.""" + try: + click.echo(message, err=err) + except BrokenPipeError: + devnull = os.open(os.devnull, os.O_WRONLY) + os.dup2(devnull, (sys.stderr if err else sys.stdout).fileno()) + os.close(devnull) + + +def _report_below(result: BundleRun, below: tuple[TaskRun, ...], threshold: float) -> None: + shown = ", ".join(f"{result.path(run.entry)} ({min(run.scores.values()):g})" if run.scores + else f"{result.path(run.entry)} (no score)" for run in below) + _echo(f"\nBelow --fail-under {threshold:g}: {shown}") + + def _path_like(text: str) -> bool: """Text that names a folder even when there is none there.""" return text.startswith((".", "/", "~")) @@ -134,21 +165,21 @@ def _installed() -> tuple[InstalledBundle, ...]: def _list_installed() -> None: bundles = _installed() if not bundles: - click.echo("No bundles are installed. agent-env run PATH runs a folder.") + _echo("No bundles are installed. agent-env run PATH runs a folder.") return header = ("NAME", "PACKAGE", "CONTENTS", "DESCRIPTION") rows = [(run_name(bundle, bundles), f"{bundle.package} {bundle.version}", *_contents(bundle)) for bundle in bundles] widths = [max(len(row[i]) for row in (header, *rows)) for i in range(3)] def echo(name: str, package: str, contents: str, description: str) -> None: - click.echo(f"{name:<{widths[0]}} {package:<{widths[1]}} {contents:<{widths[2]}} {description}".rstrip()) + _echo(f"{name:<{widths[0]}} {package:<{widths[1]}} {contents:<{widths[2]}} {description}".rstrip()) echo(*header) for bundle, row in zip(bundles, rows): echo(*row) if bundle.root is not None: - click.echo(f"{'':<{widths[0]}} {bundle.root}") - click.echo("\nRun one with agent-env run NAME. To change one, copy its folder and run the copy.") + _echo(f"{'':<{widths[0]}} {bundle.root}") + _echo("\nRun one with agent-env run NAME. To change one, copy its folder and run the copy.") def _contents(bundle: InstalledBundle) -> tuple[str, str]: @@ -169,67 +200,84 @@ def _count(entries, kind: BundleKind, noun: str) -> str: def _summarize(result: BundleRun, verbose: bool) -> None: - click.echo() - click.echo("Tasks:") + _echo() + _echo("Tasks:") for run in result.runs: - click.echo(click.style(f" {_described(result, run)}", fg=_COLORS[run.outcome])) + _echo(click.style(f" {_described(result, run)}", fg=_COLORS[run.outcome])) + for line in _criteria_lines(run): + _echo(f" {line}") if result.evals: - click.echo("Evals:") + _echo("Evals:") for eval_run in result.evals: passed, failed, cancelled = (sum(run.outcome is outcome for run in eval_run.runs) for outcome in (Outcome.PASSED, Outcome.FAILED, Outcome.CANCELLED)) line = f" {result.path(eval_run.entry)} v{eval_run.version}: {passed} of {len(eval_run.runs)} passed" rest = [f"{failed} failed" if failed else "", f"{cancelled} cancelled" if cancelled else ""] - click.echo(", ".join([line, *filter(None, rest)])) + _echo(", ".join([line, *filter(None, rest)])) for entry in result.skipped: - click.echo(f"{result.path(entry)} isn't named by any eval, so it didn't run; run it with " - f"--task {shlex.quote(entry.name)}") + _echo(f"{result.path(entry)} isn't named by any eval, so it didn't run; run it with " + f"--task {shlex.quote(entry.name)}") if verbose: _print_tracebacks(result) +def _criteria_lines(run: TaskRun) -> list[str]: + """A line for each criterion the run failed, its step, the criterion and the reason it gave, up to + ``_SHOWN_CRITERIA``, then a count of the rest.""" + failed = run.failed_criteria + lines = [] + for criterion in failed[:_SHOWN_CRITERIA]: + reason = " ".join(criterion.reason.split()) + if len(reason) > _REASON_CHARS: + reason = reason[:_REASON_CHARS - 1].rstrip() + "…" + lines.append(f"{criterion.step}: {criterion.criterion}" + (f": {reason}" if reason else "")) + if len(failed) > _SHOWN_CRITERIA: + lines.append(f"and {len(failed) - _SHOWN_CRITERIA} more") + return lines + + def _report_dry_run(dry: DryRun) -> None: materialization = dry.materialization plan = materialization.plan - click.echo() + _echo() if plan.store_refs: - click.echo("Store refs:") + _echo("Store refs:") for ref in plan.store_refs: if ref.version is None: - click.echo(f" {ref.kind} {ref.id} v{plan.store_latest[ref.kind, ref.id]}, the latest") + _echo(f" {ref.kind} {ref.id} v{plan.store_latest[ref.kind, ref.id]}, the latest") else: - click.echo(f" {ref.kind} {ref.id} v{ref.version}") + _echo(f" {ref.kind} {ref.id} v{ref.version}") if images := dry.images(): - click.echo("Images:") + _echo("Images:") for line in images: - click.echo(f" {line}") + _echo(f" {line}") if dry.infra: - click.echo("Would build first:") + _echo("Would build first:") for build in dry.infra: - click.echo(f" {build}") - click.echo("Would run:") + _echo(f" {build}") + _echo("Would run:") for entry in dry.runs: - click.echo(f" {dry.path(entry)} v{materialization.version_of('task', entry.id)}") + _echo(f" {dry.path(entry)} v{materialization.version_of('task', entry.id)}") if plan.evals: - click.echo("Evals:") + _echo("Evals:") for entry in plan.evals: named = ", ".join(dry.path(ref.local) for ref in entry.references) - click.echo(f" {dry.path(entry.entry)} v{materialization.version_of('eval', entry.entry.id)}: {named}") + _echo(f" {dry.path(entry.entry)} v{materialization.version_of('eval', entry.entry.id)}: {named}") for entry in dry.skipped: - click.echo(f"{dry.path(entry)} isn't named by any eval, so it wouldn't run; run it with " - f"--task {shlex.quote(entry.name)}") + _echo(f"{dry.path(entry)} isn't named by any eval, so it wouldn't run; run it with " + f"--task {shlex.quote(entry.name)}") if materialization.not_preflighted: - click.echo("Not preflighted, since each reads what the run would write first:") + _echo("Not preflighted, since each reads what the run would write first:") for write, step in materialization.not_preflighted: - click.echo(f" {dry.path(write.source.entry)}: step {step.id!r} ({step.type})") - click.echo(_DRY_RUN) + _echo(f" {dry.path(write.source.entry)}: step {step.id!r} ({step.type})") + _echo(_DRY_RUN) def _print_tracebacks(result: BundleRun) -> None: for run in result.runs: if run.error is not None: - click.echo(f"\n{result.path(run.entry)} raised:", err=True) - click.echo("".join(traceback.format_exception(run.error)).rstrip(), err=True) + _echo(f"\n{result.path(run.entry)} raised:", err=True) + _echo("".join(traceback.format_exception(run.error)).rstrip(), err=True) def _described(result: BundleRun, run: TaskRun) -> str: @@ -254,22 +302,22 @@ def _hold(result: BundleRun, interrupts: Interrupts) -> BundleRun: kept = [run for run in result.runs if run.torn_down is None] count = sum(len(TeardownReport.skipped(run.context).left) for run in kept) if not count: - click.echo("\nNothing to keep up: no run left a sandbox.") + _echo("\nNothing to keep up: no run left a sandbox.") return result - click.echo("\nKept up:") + _echo("\nKept up:") for run in kept: lines = _kept_up(run.context) if lines: - click.echo(f" {result.path(run.entry)}") + _echo(f" {result.path(run.entry)}") for line in lines: - click.echo(f" {line}") + _echo(f" {line}") them = "it" if count == 1 else "them" - click.echo(f"\nHolding {_sandboxes(count)} up; Ctrl-C tears {them} down.") + _echo(f"\nHolding {_sandboxes(count)} up; Ctrl-C tears {them} down.") while not interrupts.count: time.sleep(0.2) if interrupts.count > 1: return result - click.echo(f"\nTearing down {_sandboxes(count)} (Ctrl-C again to stop now)") + _echo(f"\nTearing down {_sandboxes(count)} (Ctrl-C again to stop now)") try: return result.teardown() except RunInterrupted as stopped: @@ -318,13 +366,13 @@ def _report_teardown(result: BundleRun) -> None: still_up = [sandbox for report in reports for sandbox in report.still_up if sandbox.sandbox_type != "local" or LocalSandbox.find_work_dir(sandbox.sandbox_id)] if terminated or still_up: - click.echo() + _echo() if terminated: - click.echo(f"Tore down {_sandboxes(terminated)}.") - if still_up: - click.echo(click.style(f"Still up, {_sandboxes(len(still_up))}:", fg="yellow")) + _echo(f"Tore down {_sandboxes(terminated)}.") + if still_up: # on stderr, which still reaches the user when stdout is piped away + _echo(click.style(f"Still up, {_sandboxes(len(still_up))}:", fg="yellow"), err=True) for sandbox in still_up: - click.echo(f" {_still_up(sandbox)}") + _echo(f" {_still_up(sandbox)}", err=True) def _still_up(sandbox: RecordedSandbox) -> str: diff --git a/src/agent_env/task/task.py b/src/agent_env/task/task.py index 46960671..5038e996 100644 --- a/src/agent_env/task/task.py +++ b/src/agent_env/task/task.py @@ -11,6 +11,7 @@ import logging import time import uuid +from collections.abc import Iterator from dataclasses import dataclass from datetime import datetime, timezone from typing import TYPE_CHECKING, Callable, ClassVar, Optional @@ -120,13 +121,61 @@ def _sub(value): def recorded_scores(steps: list["TaskStep"], context: TaskStepContext) -> dict[str, float]: """Every score a run of ``steps`` recorded, by the id of the step that recorded it, or by its verification's key when that isn't a step's verifier id.""" + return {name: float(verification["score"]) for name, verification in _verifications(steps, context) + if isinstance(verification.get("score"), (int, float))} + + +@dataclass(frozen=True) +class FailedCriterion: + """A criterion a verification recorded as failed.""" + + step: str # the id of the step that recorded it, or its verification's key + criterion: str + reason: str # the verifier's justification or message, "" when it gave none + + +def failed_criteria(steps: list["TaskStep"], context: TaskStepContext) -> list[FailedCriterion]: + """Each criterion that failed in a verification scoring below 1, named as ``recorded_scores`` names its score.""" + failed = [] + for name, verification in _verifications(steps, context): + score = verification.get("score") + if not isinstance(score, (int, float)) or score >= 1: + continue + for index, result in enumerate(verification.get("results") or []): + if isinstance(result, dict) and _failed(result): + reason = _first_text(result, ("justification", "message", "reason", "description")) + failed.append(FailedCriterion(name, _criterion(result, index), reason)) + return failed + + +def _verifications(steps: list["TaskStep"], context: TaskStepContext) -> Iterator[tuple[str, dict]]: + """Each verification a run of ``steps`` recorded, by the id of the step that recorded it, or by its key when + that isn't a step's verifier id.""" step_ids = {step.verifier_id: step.id for step in steps if getattr(step, "verifier_id", None)} - scores = {} for key, verification in (context.metadata.get("verifications") or {}).items(): - score = verification.get("score") - if isinstance(score, (int, float)): - scores[step_ids.get(key, key)] = float(score) - return scores + yield step_ids.get(key, key), verification + + +def _failed(result: dict) -> bool: + """A result fails when its result is false or, recording none, its score is below 1; a skipped one never does.""" + if result.get("skipped"): + return False + if "result" in result: + return not result["result"] + score = result.get("score") + return isinstance(score, (int, float)) and score < 1 + + +def _criterion(result: dict, index: int) -> str: + """How a result names its criterion: verifiers name it differently, and some only by its type.""" + if name := _first_text(result, ("criterion", "id", "name")): + return name + return f"{result['type']} #{index}" if isinstance(result.get("type"), str) else f"#{index}" + + +def _first_text(result: dict, keys: tuple[str, ...]) -> str: + """The first of ``keys`` that holds text in ``result``, or "" when none does.""" + return next((result[key] for key in keys if isinstance(result.get(key), str) and result[key].strip()), "") def _dependency_ids(steps: list["TaskStep"]) -> dict[str, set[str]]: diff --git a/tst/integration/cli/run_bundle_test.py b/tst/integration/cli/run_bundle_test.py index 7d95138c..0f16ae94 100644 --- a/tst/integration/cli/run_bundle_test.py +++ b/tst/integration/cli/run_bundle_test.py @@ -120,10 +120,66 @@ def test_a_bundle_runs_from_its_folder_and_a_rerun_writes_only_what_changed(bund third = _run(bundle) assert "artifacts/greeting: v2 (files changed: hello.txt)" in third assert "tasks/hello.json v1: scored below 1 (hello: 0)" in third + assert "\n hello: greets: does not contain 'hello'\n" in third (instance_id,) = re.findall(r"instance (\S+)", third) with namespace_routing(): results = Task.get_instance(instance_id).context["metadata"]["verifications"]["hello"]["results"] - assert [(result["criterion"], result["result"]) for result in results] == [("greets", False), ("check passes", False)] + assert [(result["criterion"], result["result"]) for result in results] == [ + ("greets", False), ("check passes", False)] + + gated = CliRunner().invoke(cli, ["run", str(bundle), "--fail-under", "1"]) + assert gated.exit_code == 1, gated.output + assert gated.output.endswith("\nBelow --fail-under 1: tasks/hello.json (0)\n"), gated.output + + +def test_a_run_whose_reader_leaves_finishes_quietly_and_tears_down(bundle): + """As when piped into head: the reader closes the pipe after one line, and the run goes on to its end.""" + code, errors = _reader_leaves([bundle], "") + + assert code == 0, errors + assert "BrokenPipeError" not in errors, errors + + +def test_a_kept_run_whose_reader_leaves_still_tears_down_on_ctrl_c(bundle, sigint_handled): + """As when piped into tee, which Ctrl-C stops too: the reader leaves while the run holds its sandbox up.""" + code, errors = _reader_leaves([bundle, "--keep"], "Holding 1 sandbox up", signal.SIGINT) + + assert code == 0, errors + assert "BrokenPipeError" not in errors, errors + + +def test_a_stream_whose_reader_has_gone_drops_its_lines_and_the_other_stream_still_prints(): + """As with --verbose piped from, when only stderr's reader leaves.""" + read, write = os.pipe() + os.close(read) + echo = "from agent_env.cli.run import _echo; _echo('a'); _echo('x', err=True); _echo('b'); _echo('y', err=True)" + try: + result = subprocess.run([sys.executable, "-c", echo], stdout=subprocess.PIPE, stderr=write, text=True, + timeout=120) + finally: + os.close(write) + + assert (result.returncode, result.stdout) == (0, "a\nb\n") + + +def _reader_leaves(args, when, signum=None): + """Run ``agent-env run ARGS`` in its own process group, close its stdout once a line containing ``when`` is + printed, then send ``signum`` to the group, if given. Returns the exit code and what it printed on stderr.""" + process = subprocess.Popen([sys.executable, "-m", "agent_env.cli", "run", *map(str, args)], stdout=subprocess.PIPE, + stderr=subprocess.PIPE, text=True, start_new_session=True) + watchdog = threading.Timer(120, process.kill) + watchdog.start() + try: + for line in process.stdout: + if when in line: + break + process.stdout.close() + if signum is not None: + os.killpg(process.pid, signum) + _, errors = process.communicate() + return process.returncode, errors + finally: + watchdog.cancel() @skip_without_gnu_stat() diff --git a/tst/unit/bundle/installed_test.py b/tst/unit/bundle/installed_test.py index 9af92eb7..bfd780ee 100644 --- a/tst/unit/bundle/installed_test.py +++ b/tst/unit/bundle/installed_test.py @@ -238,7 +238,10 @@ def test_the_cli_says_so_when_no_bundles_are_installed_and_needs_a_bundle_for_it for option in (["--task", "t"], ["--keep"], ["--dry-run"]): usage = CliRunner().invoke(cli, ["run", *option]) assert usage.exit_code == 2 - assert "--task, --eval, --model, --sandbox, --keep and --dry-run need a BUNDLE to run" in usage.output + assert f"{option[0]} needs a BUNDLE to run" in usage.output + several = CliRunner().invoke(cli, ["run", "--task", "t", "--model", "", "--keep"]) + assert several.exit_code == 2 + assert "--task, --model and --keep need a BUNDLE to run" in several.output def test_the_cli_runs_an_installed_bundle_by_name_with_ids_rooted_at_its_package(site, monkeypatch, quiet_logs, diff --git a/tst/unit/bundle/run_teardown_test.py b/tst/unit/bundle/run_teardown_test.py index 916dbaf2..442544fc 100644 --- a/tst/unit/bundle/run_teardown_test.py +++ b/tst/unit/bundle/run_teardown_test.py @@ -287,6 +287,7 @@ def test_the_cli_prints_a_sandbox_it_couldnt_tear_down_as_still_up(bundle_dir, t assert result.exit_code == 0, result.output assert "[tasks/ok.json] couldn't tear down sb-ok: RuntimeError: stuck\n" in result.output assert result.output.endswith("\nStill up, 1 sandbox:\n sb-ok fake\n") + assert result.stderr.endswith("Still up, 1 sandbox:\n sb-ok fake\n") def test_the_cli_keep_prints_the_endpoints_holds_and_tears_down_on_ctrl_c(bundle_dir, terminated, quiet_logs, diff --git a/tst/unit/bundle/run_test.py b/tst/unit/bundle/run_test.py index 56a472c9..4815a70e 100644 --- a/tst/unit/bundle/run_test.py +++ b/tst/unit/bundle/run_test.py @@ -41,16 +41,17 @@ class _Scored(TaskStep): peak = 0 seen: list[TaskStepContext] = [] - def __init__(self, score=1.0, verifier_id=None, **base): + def __init__(self, score=1.0, verifier_id=None, results=None, **base): super().__init__(**base) - self.score, self.verifier_id = score, verifier_id + self.score, self.verifier_id, self.results = score, verifier_id, results @classmethod def from_dict(cls, data): - return cls(**cls._base_from_dict(data), score=data.get("score"), verifier_id=data.get("verifier_id")) + return cls(**cls._base_from_dict(data), score=data.get("score"), verifier_id=data.get("verifier_id"), + results=data.get("results")) def to_dict(self): - return {**super().to_dict(), "score": self.score, "verifier_id": self.verifier_id} + return {**super().to_dict(), "score": self.score, "verifier_id": self.verifier_id, "results": self.results} async def execute(self, context): _Scored.seen.append(context) @@ -61,7 +62,9 @@ async def execute(self, context): finally: _Scored.active -= 1 if self.verifier_id is not None: - context.metadata.setdefault("verifications", {})[self.verifier_id] = {"score": self.score} + verification = {"score": self.score} if self.results is None else {"score": self.score, + "results": self.results} + context.metadata.setdefault("verifications", {})[self.verifier_id] = verification return context @@ -408,6 +411,71 @@ def test_the_cli_exits_0_when_no_run_failed_even_if_one_scored_below_1(bundle_di assert "tasks/b.json v1: scored below 1" in result.output +FAILED = [ + {"criterion": "greets", "result": False, "justification": "does not contain 'hello'"}, + {"criterion": "builds", "result": True, "justification": "exit=0"}, + {"id": "loaded", "result": False, "description": "list_items lists ['a']: {'items': []}"}, + {"type": "response_contains", "result": False, "justification": "missing:\n 'Echo: hi'"}, + {"id": "row-1", "score": 0.0, "message": "x" * 300}, + {"criterion": "slow", "result": False, "skipped": True, "justification": "not run"}, + {"id": "exit_code", "result": False, "message": "exit_code=2 timed_out=False"}, + {"id": "late", "result": False}, +] + + +def _criteria_bundle(root): + layout(root, {"tasks/a.json": _scored(1.0), "tasks/t.json": json.dumps([ + {"id": "check", "type": "scored_run_test", "verifier_id": "v", "score": 0.2, "results": FAILED}])}) + return root + + +def test_the_summary_names_each_failed_criterion_and_why_up_to_five(bundle_dir, quiet_logs): + result = CliRunner().invoke(cli, ["run", str(_criteria_bundle(bundle_dir)), "--task", "t"]) + + assert result.exit_code == 0, result.output + lines = result.output.split("tasks/t.json v1: scored below 1 (check: 0.2)", 1)[1].splitlines()[1:7] + assert lines == [ + " check: greets: does not contain 'hello'", + " check: loaded: list_items lists ['a']: {'items': []}", + " check: response_contains #3: missing: 'Echo: hi'", + f" check: row-1: {'x' * 159}…", + " check: exit_code: exit_code=2 timed_out=False", + " and 1 more", + ] + + +@pytest.mark.parametrize("args, code, below", [ + (["--task", "a", "--task", "b", "--fail-under", "1"], 1, "Below --fail-under 1: tasks/b.json (0.5)"), + (["--task", "a", "--task", "b", "--fail-under", "0.5"], 0, None), + (["--task", "a", "--fail-under", "1"], 0, None), + (["--task", "a", "--task", "b"], 0, None), +], ids=["below", "at the threshold", "all passed", "no threshold"]) +def test_fail_under_exits_1_when_a_task_scored_below_it(bundle_dir, quiet_logs, args, code, below): + result = CliRunner().invoke(cli, ["run", str(bundle_dir), *args]) + + assert result.exit_code == code, result.output + assert (result.output.rstrip().rsplit("\n", 1)[-1] == below) if below else "Below --fail-under" not in result.output + + +def test_fail_under_fails_a_task_that_recorded_no_score(bundle_dir, quiet_logs): + layout(bundle_dir, {"tasks/quiet.json": json.dumps([{"id": "step", "type": "scored_run_test"}])}) + + result = CliRunner().invoke(cli, ["run", str(bundle_dir), "--task", "quiet", "--fail-under", "0"]) + + assert result.exit_code == 1, result.output + assert "tasks/quiet.json v1: unscored" in result.output + assert result.output.endswith("\nBelow --fail-under 0: tasks/quiet.json (no score)\n") + + +def test_fail_under_needs_a_bundle_and_changes_nothing_a_dry_run_shows(bundle_dir, quiet_logs): + bare = CliRunner().invoke(cli, ["run", "--fail-under", "1"]) + dry = CliRunner().invoke(cli, ["run", str(bundle_dir), "--task", "b", "--fail-under", "1", "--dry-run"]) + + assert bare.exit_code == 2 and "--fail-under needs a BUNDLE to run" in bare.output + assert dry.exit_code == 0, dry.output + assert "Below --fail-under" not in dry.output + + def test_the_cli_prints_an_unknown_sandbox_as_one_line(bundle_dir, quiet_logs): result = CliRunner().invoke(cli, ["run", str(bundle_dir), "--sandbox", "nowhere"]) diff --git a/tst/unit/task/failed_criteria_test.py b/tst/unit/task/failed_criteria_test.py new file mode 100644 index 00000000..66d5dd48 --- /dev/null +++ b/tst/unit/task/failed_criteria_test.py @@ -0,0 +1,48 @@ +"""The criteria a run's verifications recorded as failed, read from its context the way its scores are.""" + +from agent_env.task.task import FailedCriterion, failed_criteria +from agent_env.task_step.context import TaskStepContext + + +class _Step: + def __init__(self, id, verifier_id): + self.id, self.verifier_id = id, verifier_id + + +def _failed(verifications, steps=()): + return failed_criteria(list(steps), TaskStepContext(metadata={"verifications": verifications})) + + +def test_a_failed_criterion_is_named_by_the_step_that_recorded_it_or_else_by_its_key(): + verifications = {"v": {"score": 0, "results": [{"criterion": "greets", "result": False}]}, + "v-trajectory-mistakes": {"score": 0.5, "results": [{"id": "m1", "score": 0.5}]}} + + assert _failed(verifications, [_Step("check", "v")]) == [ + FailedCriterion("check", "greets", ""), FailedCriterion("v-trajectory-mistakes", "m1", "")] + + +def test_only_a_verification_scoring_below_1_has_failed_criteria(): + verifications = {"full": {"score": 1, "results": [{"id": "a", "result": False}]}, + "unscored": {"results": [{"id": "b", "result": False}]}, + "partial": {"score": 0.5, "results": [{"id": "c", "result": True}, {"id": "d", "result": False}]}} + + assert [criterion.criterion for criterion in _failed(verifications)] == ["d"] + + +def test_a_result_without_a_result_fails_by_its_score_and_a_skipped_one_never_fails(): + verifications = {"v": {"score": 0.4, "results": [ + {"id": "low", "score": 0.4}, {"id": "full", "score": 1.0}, {"id": "unsure"}, + {"id": "skipped", "result": False, "skipped": True}, "not a result"]}} + + assert [criterion.criterion for criterion in _failed(verifications)] == ["low"] + + +def test_the_reason_is_the_first_one_the_verifier_gave(): + verifications = {"v": {"score": 0, "results": [ + {"id": "a", "result": False, "justification": "why", "message": "what", "description": "about"}, + {"id": "b", "result": False, "message": "what", "description": "about"}, + {"id": "c", "result": False, "justification": " ", "description": "about"}, + {"type": "response_contains", "id": " ", "result": False}]}} + + assert [(criterion.criterion, criterion.reason) for criterion in _failed(verifications)] == [ + ("a", "why"), ("b", "what"), ("c", "about"), ("response_contains #3", "")]