diff --git a/CHANGELOG.md b/CHANGELOG.md index 391d5546..b2cfd0f7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ tag releases both in lockstep, so entries below are keyed by the engine version. ## [Unreleased] +## [1.12.3] - 2026-09-15 + ### Fixed - **`explore profile` no longer offers composite keys that are artifacts of a @@ -185,6 +187,33 @@ tag releases both in lockstep, so entries below are keyed by the engine version. Also available as `DexEngine.test_mutations(model)`. +- **A cross-skill, externally authored triggering corpus for the Tier-2 eval + harness** ([#216]). Each skill's own `evals.json` `positive`/`negative` list + is written by whoever wrote the description it tests, at the same time, + and checked with every other skill disabled; neither weakness is visible + from inside that suite. `evals/corpus/ade_bench_triggering.json` sources 30 + real analytics-engineering requests from + [dbt-labs/ade-bench](https://github.com/dbt-labs/ade-bench) (Apache-2.0), + hand-labeled with the skill each should fire (or `none`), and run with + every skill available at once, one live call per prompt rather than one + per prompt-per-skill. + + `python -m evals --corpus evals/corpus/ade_bench_triggering.json` reports + per-skill precision and recall plus which cases missed, and always exits + 0: it is a measurement against externally authored prompts, not a release + gate, since the initial pass rate is expected to be low and that is the + signal the corpus exists to produce. + + `Classifier.classify` reports every skill marker a call finds, not a single + winner: a prompt where two skills both fire is real evidence of cross-skill + contamination, and picking one silently would hide exactly the failure + mode this corpus exists to catch. A per-call failure is caught by the + classifier itself and recorded per case, excluded from precision/recall; + a setup failure (the `claude` binary missing) is left to propagate + immediately instead, so it aborts the run once with the existing + prerequisite message and exit code 2, rather than being recorded as 30 + separate case errors while the command still reports a clean exit. + - **`maintain verify` reports a built relation whose grain is not unique** ([#229]). For every selected model, the intended grain is determined from a declared `unique` test, a declared composite `unique_combination_of_columns` @@ -1812,23 +1841,6 @@ tag releases both in lockstep, so entries below are keyed by the engine version. contributions rather than only the final score, which is new design work the capping fix does not need. -- **A cross-skill, externally authored triggering corpus for the Tier-2 eval - harness** ([#216]). Each skill's own `evals.json` `positive`/`negative` list - is written by whoever wrote the description it tests, at the same time, - and checked with every other skill disabled; neither weakness is visible - from inside that suite. `evals/corpus/ade_bench_triggering.json` sources 30 - real analytics-engineering requests from - [dbt-labs/ade-bench](https://github.com/dbt-labs/ade-bench) (Apache-2.0), - hand-labeled with the skill each should fire (or `none`), and run with - every skill available at once, one live call per prompt rather than one - per prompt-per-skill. - - `python -m evals --corpus evals/corpus/ade_bench_triggering.json` reports - per-skill precision and recall plus which cases missed, and always exits - 0: it is a measurement against externally authored prompts, not a release - gate, since the initial pass rate is expected to be low and that is the - signal the corpus exists to produce. - - **`maintain schema` detects a model added, removed, or content-changed since the baseline** ([#164]). The transform layer's fingerprint (model names, per-file content hashes) was captured on every snapshot and diff --git a/evals/README.md b/evals/README.md index a65bdb13..5989881b 100644 --- a/evals/README.md +++ b/evals/README.md @@ -46,13 +46,43 @@ a workspace with the dex plugin installed. The command exits non-zero unless the suite passes (clean triggering and no regression versus baseline), so the same invocation works locally and as a release gate. +## The cross-skill triggering corpus + +The per-skill `positive`/`negative` lists above have a structural blind spot: they +are written by the same person who wrote the description they test, at the same +time, so a positive is often just a paraphrase of the description, and each skill +is only ever checked with the *other* skills disabled. Neither failure mode is +visible from inside one skill's own suite. + +`evals/corpus/` holds externally authored fixtures for exactly this: real prompts +from someone with no knowledge of these descriptions, run with every skill +available at once, each hand-labeled with the skill (or `none`) it should fire. +`evals/corpus/ade_bench_triggering.json` is the first one, sourced from +[dbt-labs/ade-bench](https://github.com/dbt-labs/ade-bench) (Apache-2.0); see the +file's own `source` field for the exact provenance and what was and was not +carried over. + +``` +python -m evals --corpus evals/corpus/ade_bench_triggering.json +python -m evals --corpus evals/corpus/ade_bench_triggering.json --json +``` + +One live call per prompt (not per prompt-per-skill, since every skill is available +in the same call), and the report is per-skill precision/recall plus which cases +missed. This mode always exits 0: it is a measurement, not a release gate. Expect +the first run's numbers to be low and record them as the baseline (there is no +committed baseline yet; run it and note what you get in the PR, and treat later +runs as measuring drift from that number rather than pass/fail). + ## Layout -- `suite.py` loads and validates a skill's `evals.json` (stdlib dataclasses). -- `runner.py` is the deterministic scoring core: triggering, output quality, and - uplift. It takes an agent and a judge by dependency injection. -- `claude_agent.py` is the live backend: the `AgentRunner` and `Judge` driven by - the `claude` CLI. A non-Claude agent is a second backend behind the same two - protocols, with no change to the core. +- `suite.py` loads and validates a skill's `evals.json`, and a cross-skill + `Corpus` from `evals/corpus/*.json` (stdlib dataclasses both ways). +- `runner.py` is the deterministic scoring core: triggering, output quality, + uplift, and the corpus's per-skill precision/recall. It takes an agent, a judge, + or a classifier by dependency injection. +- `claude_agent.py` is the live backend: the `AgentRunner`/`Judge`/`Classifier` + protocols driven by the `claude` CLI. A non-Claude agent is a second backend + behind the same protocols, with no change to the core. - `__main__.py` is the CLI. - `tests/` covers the scoring core with fake backends (no model, free, in CI). diff --git a/evals/__main__.py b/evals/__main__.py index f4398954..1935d547 100644 --- a/evals/__main__.py +++ b/evals/__main__.py @@ -1,13 +1,18 @@ -"""CLI: run a skill's Tier-2 eval suite. +"""CLI: run a skill's Tier-2 eval suite, or the cross-skill triggering corpus. python -m evals skills/explore # full suite (live Claude) python -m evals skills/explore --triggering # triggering only (cheaper) python -m evals skills/explore --json # machine-readable report + python -m evals --corpus evals/corpus/ade_bench_triggering.json + # cross-skill corpus (#216) The default backend drives Claude Code headless; it needs the ``claude`` CLI and a workspace with the dex plugin installed. Exit code is non-zero if the suite does not pass (clean triggering and no regression versus baseline), so the same command -serves both local runs and the release gate. +serves both local runs and the release gate. ``--corpus`` always exits 0: it is a +measurement (per-skill precision/recall against externally authored prompts), not +a pass/fail gate, per the issue that added it (#216) -- the initial numbers are +expected to be low, and that is the signal, not a bug to chase away. """ from __future__ import annotations @@ -16,14 +21,21 @@ import json import sys -from .claude_agent import ClaudeCliAgent, ClaudeCliJudge, ClaudeNotAvailableError -from .runner import run_suite, run_triggering -from .suite import load_suite +from .claude_agent import ( + ClaudeCliAgent, + ClaudeCliClassifier, + ClaudeCliJudge, + ClaudeNotAvailableError, +) +from .runner import CorpusReport, run_corpus, run_suite, run_triggering +from .suite import load_corpus, load_suite def main(argv: list[str] | None = None) -> int: parser = argparse.ArgumentParser(prog="evals", description="Tier-2 skill evals") - parser.add_argument("skill", help="skill dir or path to evals.json") + parser.add_argument( + "skill", nargs="?", default=None, help="skill dir or path to evals.json" + ) parser.add_argument("--model", default=None, help="model override for the agent") parser.add_argument( "--triggering", action="store_true", help="run only the triggering check" @@ -32,14 +44,35 @@ def main(argv: list[str] | None = None) -> int: parser.add_argument( "--timeout", type=int, default=180, help="per-agent-call timeout (seconds)" ) - args = parser.parse_args(argv) - - suite = load_suite(args.skill) - agent = ClaudeCliAgent( - skill_name=suite.skill_name, model=args.model, timeout=args.timeout + parser.add_argument( + "--corpus", + default=None, + help=( + "path to a cross-skill triggering corpus (see evals/corpus/); runs " + "independently of the skill argument, one live call per prompt" + ), ) + args = parser.parse_args(argv) try: + if args.corpus: + corpus = load_corpus(args.corpus) + classifier = ClaudeCliClassifier(model=args.model, timeout=args.timeout) + report = run_corpus(corpus, classifier) + if args.json: + print(json.dumps(report.to_dict(), indent=2)) + else: + _print_corpus(report) + return 0 # a measurement, never a gate; see the module docstring + + if args.skill is None: + parser.error("skill is required unless --corpus is given") + + suite = load_suite(args.skill) + agent = ClaudeCliAgent( + skill_name=suite.skill_name, model=args.model, timeout=args.timeout + ) + if args.triggering: trig = run_triggering(suite, agent) if args.json: @@ -60,6 +93,31 @@ def main(argv: list[str] | None = None) -> int: return 2 +def _print_corpus(report: CorpusReport) -> None: + print( + f"[{report.corpus_name}] {len(report.results)} cases, " + f"accuracy {report.accuracy:.0%}" + ) + for skill in sorted(report.per_skill): + pr = report.per_skill[skill] + print( + f" {skill:10s} precision {pr.precision:.0%} recall {pr.recall:.0%} " + f"f1 {pr.f1:.0%} (tp={pr.true_positives} fp={pr.false_positives} " + f"fn={pr.false_negatives})" + ) + errors = [r for r in report.results if r.error is not None] + misses = [r for r in report.results if not r.correct and r.error is None] + if misses: + print(f" {len(misses)} miss(es):") + for r in misses: + fired = sorted(r.fired_skills) or ["none"] + print(f" {r.task_id}: expected {r.expected_skill!r}, got {fired}") + if errors: + print(f" {len(errors)} call(s) failed (excluded from precision/recall):") + for r in errors: + print(f" {r.task_id}: {r.error}") + + def _print_triggering(skill: str, report) -> None: fired = f"{report.positives_fired}/{report.positives_total} positives fired" print(f"[{skill}] triggering") diff --git a/evals/claude_agent.py b/evals/claude_agent.py index 4addcfe5..9cfb3ab1 100644 --- a/evals/claude_agent.py +++ b/evals/claude_agent.py @@ -20,7 +20,7 @@ import subprocess from dataclasses import dataclass, field -from .runner import AgentResult +from .runner import AgentResult, ClassifyResult from .suite import EvalCase @@ -93,6 +93,45 @@ def _plugin(self) -> str: return "dex" +@dataclass +class ClaudeCliClassifier: + """Runs one prompt with every dex skill available and reports every + marker that showed up, for the cross-skill corpus (``run_corpus``). + + Unlike :class:`ClaudeCliAgent`, this never suppresses a skill: ``args`` + is whatever invokes Claude Code with the plugin installed normally, the + same condition a real request arrives in. Reports *every* marker found, + not just one: picking a single "winner" when two fire would hide the + unwanted one entirely, which is exactly the cross-skill contamination + this corpus exists to catch, not a detail to summarize away. + """ + + skill_names: tuple[str, ...] = ("explore", "transform", "maintain") + binary: str = "claude" + model: str | None = None + timeout: int = 180 + args: list[str] = field(default_factory=list) + plugin: str = "dex" + + def classify(self, prompt: str) -> ClassifyResult: + # _require_claude runs outside the try, same as ClaudeCliAgent.run: + # a missing binary is a setup problem every subsequent call would hit + # identically, so it raises here and propagates out of run_corpus + # rather than being caught into one case's error. + binary = _require_claude(self.binary) + args = list(self.args) + if self.model: + args += ["--model", self.model] + try: + output = _invoke(binary, args, prompt, self.timeout) + except Exception as exc: + return ClassifyResult(error=str(exc)) + fired = frozenset( + name for name in self.skill_names if f"/{self.plugin}:{name}" in output + ) + return ClassifyResult(fired_skills=fired) + + @dataclass class ClaudeCliJudge: """Grades one assertion against an agent result with an LLM yes/no judge.""" diff --git a/evals/corpus/ade_bench_triggering.json b/evals/corpus/ade_bench_triggering.json new file mode 100644 index 00000000..ab1ad803 --- /dev/null +++ b/evals/corpus/ade_bench_triggering.json @@ -0,0 +1,162 @@ +{ + "name": "ade_bench_triggering", + "source": { + "repo": "https://github.com/dbt-labs/ade-bench", + "license": "Apache-2.0", + "license_url": "https://github.com/dbt-labs/ade-bench/blob/main/LICENSE", + "authored_by": "dbt Labs, with no knowledge of dex's skill descriptions", + "note": "Task prompts only, the 'base' prompt from each task's task.yaml, reproduced verbatim. No task setup scripts, seed data, solutions, or test code is included. This is a representative sample across ADE-bench's domains (airbnb, analytics_engineering, asana, f1, helixops_saas, intercom, quickbooks, shopify-analytics, simple, workday), not the full ~75-task set. expected_skill is hand-labeled by a dex maintainer reading the prompt text alone, the same information a real triggering decision would have. See evals/README.md for how this differs from the per-skill positive/negative lists and for the known corpus skew (ADE-bench is a fix/build-the-dbt-project benchmark, so it produced far more transform-shaped prompts than explore- or maintain-shaped ones; only two maintain-labeled entries survived honest labeling rather than being forced)." + }, + "cases": [ + { + "task_id": "airbnb001", + "expected_skill": "transform", + "prompt": "I got this error, please fix it, and fix it anywhere else it needs to be fixed.\n\nCompilation Error in model monthly_agg_reviews (models/agg/monthly_agg_reviews.sql)\n\nWarning: `dbt_utils.surrogate_key` has been replaced by `dbt_utils.generate_surrogate_key`. The new macro treats null values differently to empty strings. To restore the behaviour of the original macro, add a global variable in dbt_project.yml called `surrogate_key_treat_nulls_as_empty_strings` to your dbt_project.yml file with a value of True. The dbt_airbnb.monthly_agg_reviews model triggered this warning.\n\n> in macro default__surrogate_key (macros/sql/surrogate_key.sql)\n> called by macro surrogate_key (macros/sql/surrogate_key.sql)\n> called by model monthly_agg_reviews (models/agg/monthly_agg_reviews.sql)" + }, + { + "task_id": "airbnb003", + "expected_skill": "transform", + "prompt": "Update the all the source models to be views." + }, + { + "task_id": "airbnb006", + "expected_skill": "transform", + "prompt": "Update the CTEs in the models to be named correctly." + }, + { + "task_id": "airbnb009", + "expected_skill": "explore", + "prompt": "In mom_agg_reviews, there should be a row for every day. Right now, some days are missing. Can you identify why these days are missing, and fix that model?" + }, + { + "task_id": "airbnb012", + "expected_skill": "transform", + "prompt": "Add tests to verify the logic in the `listing_agg_nps_reviews` and `daily_agg_nps_reviews`\nmodels is correct." + }, + { + "task_id": "analytics_engineering001", + "expected_skill": "none", + "prompt": "This is a dbt project. Do not make any changes." + }, + { + "task_id": "analytics_engineering003", + "expected_skill": "transform", + "prompt": "Create a model called \"dim_customer\" that renames id to customer_id, and makes that row a unique primary key." + }, + { + "task_id": "analytics_engineering005", + "expected_skill": "maintain", + "prompt": "The fact_inventory model should be unique on inventory_id and it's not. Update the model so that it has only one row per inventory_id." + }, + { + "task_id": "analytics_engineering007", + "expected_skill": "maintain", + "prompt": "The id field in the \"products\" table is now a string. This has broken several models. Update them so that they work with product ids that are strings." + }, + { + "task_id": "asana001", + "expected_skill": "transform", + "prompt": "We updated the Fivetran Asana package, and now the project is erroring. Fix the issue without updating the underlying data." + }, + { + "task_id": "asana003", + "expected_skill": "transform", + "prompt": "Fivetran is updating their Asana package, so I want to change that package directly. Remove all of the models in the tmp folder and have the stg_asana__[ name ].sql models reference the source tables directly." + }, + { + "task_id": "asana005", + "expected_skill": "transform", + "prompt": "I want to refactor the asana__project model into a new intermediate model. I want the calculations done in the agg_project_users and count_project_users CTEs to be done in a new model called int_asana__project_user_agg.\nThat model should have the following columns:\n- project_id\n- users\n- number_of_users_involved\n\nCreate that intermediate model and then update the asana__project model to use it instead of the logic it uses currently.\n\nPrior to fixing this, also fix whatever is causing the project to fail." + }, + { + "task_id": "f1001", + "expected_skill": "transform", + "prompt": "Fix this project by adding src files and updating the staging models to point to them. The src models should be views, should be called src_.sql, and should all point directly to existing tables in the database." + }, + { + "task_id": "f1004", + "expected_skill": "explore", + "prompt": "Several columns in the \"finishes_by_driver\" are showing 0 results for every driver. Can you figure out what's happening, fix it, and update the table?" + }, + { + "task_id": "f1007", + "expected_skill": "explore", + "prompt": "Something is wrong with the staging results table, where it's missing a lot of rows. Can you figure out why, fix it, and update the table?" + }, + { + "task_id": "f1010", + "expected_skill": "transform", + "prompt": "I want to figure out if drivers have gotten faster or slower over time. To help me figure this out, create a new model called analysis__lap_times that has three columns:\n\n- circuit_name: The name of the track.\n- race_year: The year of the race.\n- avg_lap_time_in_ms: (INTEGER) The average lap time in milliseconds of all laps on that track in that year." + }, + { + "task_id": "helixops_saas001", + "expected_skill": "transform", + "prompt": "Add billing_country to dim_accounts." + }, + { + "task_id": "helixops_saas004", + "expected_skill": "transform", + "prompt": "I need to be able to work out what departments users belong to across their workspace memberships. Please add a department column to int_workspace_roster, you can infer it from job title if needed." + }, + { + "task_id": "helixops_saas008", + "expected_skill": "transform", + "prompt": "stg_accounts has account_status instead of customer_status, please rename and propagate." + }, + { + "task_id": "helixops_saas012", + "expected_skill": "transform", + "prompt": "Please move the monthly revenue prep model into being a CTE for the main revenue model." + }, + { + "task_id": "helixops_saas016", + "expected_skill": "transform", + "prompt": "We have new SLA targets for enterprise accounts only, effective 2025-06-16 at 08:00 UTC. Please update the SLA model so enterprise accounts get: urgent=20min, high=45min, medium=120min, standard=900min. Other segments keep existing SLAs." + }, + { + "task_id": "intercom001", + "expected_skill": "transform", + "prompt": "Create a model called intercom__threads that aggregates the conversation_parts by conversation_id. Include the following columns:\n - conversation_id\n - conversation_created_at\n - total_conversation_parts\n - first_contact_reply_at\n - first_assignment_at\n - first_admin_response_at\n - first_reopen_at\n - last_assignment_at\n - last_contact_reply_at\n - last_admin_response_at\n - last_reopen_at\n - total_assignments\n - total_reopens" + }, + { + "task_id": "intercom002", + "expected_skill": "transform", + "prompt": "Create two models - one called intercom__threads and one called intercom__conversation_metrics. The threads model should aggregate the conversation_parts by conversation_id, and the metrics model should add additional metrics.\n\nintercom__threads should include the following columns:\n - conversation_id\n - conversation_created_at\n - total_conversation_parts\n - first_contact_reply_at\n - first_assignment_at\n - first_admin_response_at\n - first_reopen_at\n - last_assignment_at\n - last_contact_reply_at\n - last_admin_response_at" + }, + { + "task_id": "intercom003", + "expected_skill": "transform", + "prompt": "Create a model called intercom__conversation_metrics that aggregates the conversation_parts by conversation_id. Include the following columns and metrics:\n\n- conversation_id\n- conversation_created_at\n- total_reopens\n- total_conversation_parts\n- total_assignments\n- first_contact_reply_at\n- first_assignment_at\n- first_admin_response_at\n- first_reopen_at\n- last_assignment_at\n- last_contact_reply_at\n- last_admin_response_at" + }, + { + "task_id": "quickbooks001", + "expected_skill": "transform", + "prompt": "This project is erroring out and needs to be fixed. Fix the underlying issue and update the tables." + }, + { + "task_id": "quickbooks003", + "expected_skill": "transform", + "prompt": "We are no longer using departments in our billing, so we should remove the using_department variable and all references to it. Do not remove it in the Fivetran source package." + }, + { + "task_id": "shopify-analytics", + "expected_skill": "transform", + "prompt": "Create two tables: one that pulls together product data like total sales, refunds, discounts, and taxes, and another that tracks daily shop performance, including orders, abandoned checkouts, and fulfillment statuses." + }, + { + "task_id": "simple001", + "expected_skill": "transform", + "prompt": "Change the dim_customer model to be called dimension_customer." + }, + { + "task_id": "simple002", + "expected_skill": "transform", + "prompt": "Change the dim_customer model to be called dimension_customer." + }, + { + "task_id": "workday001", + "expected_skill": "none", + "prompt": "Do nothing." + } + ] +} diff --git a/evals/runner.py b/evals/runner.py index e9d34e71..6722f195 100644 --- a/evals/runner.py +++ b/evals/runner.py @@ -19,7 +19,7 @@ from dataclasses import dataclass, field from typing import Any, Protocol, runtime_checkable -from .suite import EvalCase, EvalSuite +from .suite import Corpus, EvalCase, EvalSuite @dataclass @@ -52,6 +52,49 @@ class Judge(Protocol): def grade(self, case: EvalCase, assertion: str, result: AgentResult) -> bool: ... +@dataclass +class ClassifyResult: + """One classification call: every skill marker found, or the error if the + call itself failed. + + Mirrors :class:`AgentResult`'s ``error`` field, for the same reason: a + failed invocation (a crash, a timeout) and one that genuinely found no + skill both have nowhere else to be told apart, and collapsing them would + let infrastructure noise masquerade as a real classification. A + :class:`Classifier` implementation is expected to catch its own + per-*call* failures into ``error`` here (see :class:`ClaudeCliAgent.run` + for the pattern) and reserve a raised exception for a setup problem that + makes every subsequent call pointless too (the ``claude`` binary missing, + say) -- :func:`run_corpus` does not catch anything, so that kind of + failure aborts the whole run immediately rather than being recorded once + per case. + + ``fired_skills`` may hold more than one name: a prompt two skills both + claim is real evidence of cross-skill contamination, exactly what this + corpus exists to catch, and picking a single "winner" would silently + discard the other one's finding. + """ + + fired_skills: frozenset[str] = field(default_factory=frozenset) + error: str | None = None + + +@runtime_checkable +class Classifier(Protocol): + """Runs one prompt with every skill available and reports which fired. + + Deliberately not :class:`AgentRunner`: that protocol toggles one named + skill on or off for the uplift measurement, which presumes the question + is "does *this* skill help." A cross-skill corpus asks a different + question, "which skills, if any, claim this prompt out of all of them at + once," so it needs the names of whichever fired rather than a bool scoped + to one. One live call per prompt instead of one per (prompt, skill) pair, + too. + """ + + def classify(self, prompt: str) -> ClassifyResult: ... + + @dataclass class TriggeringReport: positives_total: int @@ -244,3 +287,167 @@ def _all_pass( def _rate(flags: list[bool]) -> float: return round(sum(flags) / len(flags), 4) if flags else 0.0 + + +@dataclass +class CorpusCaseResult: + """One corpus prompt classified: what it should fire, what it did. + + ``fired_skills`` is every marker the classifier found, not a single + winner: a case where two skills both fired is a real cross-skill + contamination finding, and this keeps it visible instead of quietly + picking one and discarding the other's evidence. + + ``error`` is set instead of trusting ``fired_skills`` when the call + itself failed (CLI crash, timeout, malformed output): a failed call and a + call that genuinely found nothing both surface as an empty set from a + :class:`~evals.runner.Classifier`, and collapsing them would let + infrastructure noise masquerade as "correctly classified none" or as an + ordinary miss. A case with ``error`` set is never ``correct`` and is kept + out of every skill's precision/recall (see :func:`run_corpus`). + """ + + task_id: str + prompt: str + expected_skill: str | None + fired_skills: frozenset[str] = field(default_factory=frozenset) + error: str | None = None + + @property + def actual_skill(self) -> str | None: + """The one fired skill, when exactly one did; ``None`` when none or + more than one did (``fired_skills`` carries the full picture either + way -- this is a display convenience for the common, unambiguous + case, not the source of truth ``correct`` and the per-skill scoring + below read from).""" + return next(iter(self.fired_skills)) if len(self.fired_skills) == 1 else None + + @property + def correct(self) -> bool: + if self.error is not None: + return False + expected = {self.expected_skill} if self.expected_skill is not None else set() + return set(self.fired_skills) == expected + + def to_dict(self) -> dict[str, Any]: + return { + "task_id": self.task_id, + "expected_skill": self.expected_skill, + "fired_skills": sorted(self.fired_skills), + "actual_skill": self.actual_skill, + "correct": self.correct, + "error": self.error, + } + + +@dataclass +class SkillPrecisionRecall: + """Precision/recall for one skill, computed as a one-vs-rest split of the + corpus: a true positive is this skill correctly firing, a false positive + is this skill firing when something else (or nothing) should have, a + false negative is this skill staying quiet when it should have fired.""" + + skill: str + true_positives: int = 0 + false_positives: int = 0 + false_negatives: int = 0 + + @property + def precision(self) -> float: + fired = self.true_positives + self.false_positives + return round(self.true_positives / fired, 4) if fired else 1.0 + + @property + def recall(self) -> float: + expected = self.true_positives + self.false_negatives + return round(self.true_positives / expected, 4) if expected else 1.0 + + @property + def f1(self) -> float: + p, r = self.precision, self.recall + return round(2 * p * r / (p + r), 4) if (p + r) else 0.0 + + def to_dict(self) -> dict[str, Any]: + return { + "skill": self.skill, + "true_positives": self.true_positives, + "false_positives": self.false_positives, + "false_negatives": self.false_negatives, + "precision": self.precision, + "recall": self.recall, + "f1": self.f1, + } + + +@dataclass +class CorpusReport: + corpus_name: str + results: list[CorpusCaseResult] = field(default_factory=list) + per_skill: dict[str, SkillPrecisionRecall] = field(default_factory=dict) + + @property + def accuracy(self) -> float: + return _rate([r.correct for r in self.results]) + + def to_dict(self) -> dict[str, Any]: + return { + "corpus_name": self.corpus_name, + "case_count": len(self.results), + "accuracy": self.accuracy, + "per_skill": {name: pr.to_dict() for name, pr in self.per_skill.items()}, + "results": [r.to_dict() for r in self.results], + } + + +def run_corpus(corpus: Corpus, classifier: Classifier) -> CorpusReport: + """Classify every corpus prompt and score per-skill precision/recall. + + One live call per prompt, with every skill available at once, is the + point: it is the shape a real request actually arrives in, unlike the + per-skill positive/negative lists in ``skills//evals/evals.json``, + which each only ever see one skill enabled at a time and so cannot catch + a case where a *sibling's* description fires instead of the right one. + + Nothing here is wrapped in a ``try``: a :class:`Classifier` is expected + to catch its own per-call failures into :class:`ClassifyResult.error` + (see that class), the same contract :class:`AgentRunner` already has with + :func:`run_triggering`/:func:`run_quality`. A call that raises anyway is + a setup problem, not a one-off flake (the ``claude`` binary missing is + the concrete case this matters for), and every case after it would fail + identically, so it is left to propagate to the caller immediately rather + than being recorded 30 times over. + """ + + results = [] + for case in corpus.cases: + outcome = classifier.classify(case.prompt) + results.append( + CorpusCaseResult( + task_id=case.task_id, + prompt=case.prompt, + expected_skill=case.expected_skill, + fired_skills=outcome.fired_skills, + error=outcome.error, + ) + ) + + scored = [r for r in results if r.error is None] + skills = sorted( + {r.expected_skill for r in scored if r.expected_skill is not None} + | {skill for r in scored for skill in r.fired_skills} + ) + per_skill: dict[str, SkillPrecisionRecall] = {} + for skill in skills: + pr = SkillPrecisionRecall(skill=skill) + for r in scored: + expected = r.expected_skill == skill + actual = skill in r.fired_skills + if expected and actual: + pr.true_positives += 1 + elif actual and not expected: + pr.false_positives += 1 + elif expected and not actual: + pr.false_negatives += 1 + per_skill[skill] = pr + + return CorpusReport(corpus_name=corpus.name, results=results, per_skill=per_skill) diff --git a/evals/suite.py b/evals/suite.py index e0665d6a..ae47f334 100644 --- a/evals/suite.py +++ b/evals/suite.py @@ -48,6 +48,36 @@ class EvalSuite: evals: list[EvalCase] = field(default_factory=list) +@dataclass +class CorpusCase: + """One externally authored prompt, labeled with the skill it should fire + (or ``None`` where no skill should fire).""" + + task_id: str + prompt: str + expected_skill: str | None + + +@dataclass +class Corpus: + """An externally authored triggering fixture: real requests written by + someone with no knowledge of the skill descriptions, each hand-labeled. + + Unlike :class:`EvalSuite`, a corpus is not scoped to one skill: it is run + against every skill at once (see ``evals.runner.run_corpus``), which is + what makes it able to catch a case a per-skill positive/negative list + cannot: two skills' descriptions both plausibly firing on the same prompt, + or the right skill losing to a sibling's wording. + + ``source`` carries provenance: where the prompts came from and under what + license, since these are someone else's words, reproduced verbatim. + """ + + name: str + source: dict[str, Any] + cases: list[CorpusCase] = field(default_factory=list) + + def load_suite(skill_or_file: Path | str) -> EvalSuite: """Load a suite from a skill directory or a direct path to ``evals.json``.""" @@ -89,3 +119,47 @@ def _suite_from_dict(raw: dict[str, Any], path: Path) -> EvalSuite: ) ) return EvalSuite(skill_name=skill_name, triggering=triggering, evals=cases) + + +def load_corpus(path: Path | str) -> Corpus: + """Load an externally authored triggering corpus (see :class:`Corpus`).""" + + path = Path(path) + if not path.is_file(): + raise FileNotFoundError(f"no corpus file at {path}") + + raw = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(raw, dict): + raise InvalidSuiteError(f"{path}: top level must be an object") + + name = raw.get("name") + if not isinstance(name, str) or not name: + raise InvalidSuiteError(f"{path}: 'name' is required") + source = raw.get("source") + if not isinstance(source, dict) or not source: + raise InvalidSuiteError( + f"{path}: 'source' is required (provenance and license for the " + "externally authored prompts)" + ) + + cases: list[CorpusCase] = [] + for i, entry in enumerate(raw.get("cases", [])): + if "prompt" not in entry: + raise InvalidSuiteError(f"{path}: cases[{i}] has no 'prompt'") + if "expected_skill" not in entry: + raise InvalidSuiteError(f"{path}: cases[{i}] has no 'expected_skill'") + # "none" is the on-disk spelling (JSON has nothing better to write for a + # case where no skill should fire); a Classifier reports the same + # absence as Python None, so the two are reconciled here rather than + # everywhere a case gets compared against a classifier's answer. + label = entry["expected_skill"] + cases.append( + CorpusCase( + task_id=entry.get("task_id", str(i)), + prompt=entry["prompt"], + expected_skill=None if label == "none" else label, + ) + ) + if not cases: + raise InvalidSuiteError(f"{path}: 'cases' is empty") + return Corpus(name=name, source=source, cases=cases) diff --git a/evals/tests/test_runner.py b/evals/tests/test_runner.py index 5b7a0c72..0608f1a0 100644 --- a/evals/tests/test_runner.py +++ b/evals/tests/test_runner.py @@ -9,8 +9,25 @@ from pathlib import Path -from evals.runner import AgentResult, run_suite, run_triggering -from evals.suite import EvalCase, EvalSuite, TriggeringCases, load_suite +import pytest + +from evals.runner import ( + AgentResult, + ClassifyResult, + run_corpus, + run_suite, + run_triggering, +) +from evals.suite import ( + Corpus, + CorpusCase, + EvalCase, + EvalSuite, + InvalidSuiteError, + TriggeringCases, + load_corpus, + load_suite, +) _REPO = Path(__file__).resolve().parents[2] @@ -103,3 +120,211 @@ def run(self, prompt: str, *, skill_enabled: bool) -> AgentResult: report = run_suite(_suite(), NeverHelps(), FakeJudge()) assert report.quality_pass_rate == 0.0 assert all(not c.passed for c in report.quality) + + +# --- the cross-skill corpus (#216) ---------------------------------------------- + + +class FakeClassifier: + """Reports every skill whose trigger word appears in the prompt.""" + + def __init__(self, triggers: dict[str, str]): + self.triggers = triggers # word -> skill + + def classify(self, prompt: str) -> ClassifyResult: + lowered = prompt.lower() + fired = frozenset( + skill for word, skill in self.triggers.items() if word in lowered + ) + return ClassifyResult(fired_skills=fired) + + +def _corpus() -> Corpus: + return Corpus( + name="fake", + source={"repo": "https://example.invalid/x", "license": "Apache-2.0"}, + cases=[ + CorpusCase( + task_id="a", prompt="explore the warehouse", expected_skill="explore" + ), + CorpusCase(task_id="b", prompt="build a model", expected_skill="transform"), + CorpusCase( + task_id="c", prompt="did anything drift", expected_skill="maintain" + ), + CorpusCase(task_id="d", prompt="say hello", expected_skill=None), + ], + ) + + +def test_run_corpus_is_perfect_when_every_case_classifies_correctly(): + classifier = FakeClassifier( + {"explore": "explore", "build": "transform", "drift": "maintain"} + ) + report = run_corpus(_corpus(), classifier) + assert report.accuracy == 1.0 + assert report.per_skill["explore"].precision == 1.0 + assert report.per_skill["explore"].recall == 1.0 + assert report.per_skill["transform"].f1 == 1.0 + assert report.per_skill["maintain"].true_positives == 1 + # "none" never appears in fired_skills by construction (an empty set is + # how a classifier reports nothing fired), so it earns no entry in + # per_skill: there is no skill named "none" to score. + assert "none" not in report.per_skill + + +def test_run_corpus_counts_a_false_trigger_against_precision_not_recall(): + # The classifier fires "transform" on the maintain case too (a false + # positive for transform) and stays silent on the real transform case (a + # false negative for transform), so both metrics move, in the direction + # each is supposed to. + classifier = FakeClassifier({"explore": "explore", "drift": "transform"}) + report = run_corpus(_corpus(), classifier) + transform = report.per_skill["transform"] + assert transform.false_positives == 1 + assert transform.false_negatives == 1 + assert transform.precision == 0.0 + assert transform.recall == 0.0 + maintain = report.per_skill["maintain"] + assert maintain.false_negatives == 1 + assert maintain.recall == 0.0 + + +def test_run_corpus_result_rows_say_which_cases_missed(): + classifier = FakeClassifier({"explore": "explore"}) + report = run_corpus(_corpus(), classifier) + wrong = [r for r in report.results if not r.correct] + assert {r.task_id for r in wrong} == {"b", "c"} + assert all(r.actual_skill is None for r in wrong) + + +def test_run_corpus_never_hides_a_second_marker_behind_the_first(): + # "did" spuriously fires explore on the maintain case, on top of "drift" + # correctly firing maintain there. A classifier that reported only the + # first name in some fixed order would pick one and hide the other, and + # this genuine cross-skill contamination would never show up -- the exact + # failure mode #216 exists to catch. + classifier = FakeClassifier( + {"explore": "explore", "did": "explore", "drift": "maintain"} + ) + report = run_corpus(_corpus(), classifier) + contaminated = next(r for r in report.results if r.task_id == "c") + assert contaminated.fired_skills == frozenset({"explore", "maintain"}) + assert contaminated.actual_skill is None # ambiguous: more than one fired + assert contaminated.correct is False + # explore fired where it should not have (a real false positive for it, + # even though maintain -- the expected skill -- also fired on this case). + assert report.per_skill["explore"].false_positives >= 1 + assert report.per_skill["maintain"].true_positives == 1 + + +class _FlakyClassifier: + """Reports a per-call failure the way a well-behaved Classifier should: + via ClassifyResult.error, not by raising (a raise means the whole run + should abort, see test_run_corpus_lets_a_setup_failure_abort_the_run).""" + + def __init__(self, fails_on: str, otherwise): + self.fails_on = fails_on + self.otherwise = otherwise + + def classify(self, prompt: str) -> ClassifyResult: + if prompt == self.fails_on: + return ClassifyResult(error="claude exited 1") + return self.otherwise.classify(prompt) + + +def test_run_corpus_keeps_a_failed_call_out_of_precision_and_recall(): + # The maintain case's call fails outright. A failed call is neither "fired" + # nor "did not fire": folding it into maintain's false-negative count would + # blame the skill description for an infrastructure failure it had nothing + # to do with, and folding it into "correctly classified none" would hide + # the failure entirely. + classifier = _FlakyClassifier( + fails_on="did anything drift", + otherwise=FakeClassifier( + {"explore": "explore", "build": "transform", "drift": "maintain"} + ), + ) + report = run_corpus(_corpus(), classifier) + failed = next(r for r in report.results if r.task_id == "c") + assert failed.error is not None + assert failed.actual_skill is None + assert failed.correct is False + assert "maintain" not in report.per_skill or ( + report.per_skill["maintain"].true_positives == 0 + and report.per_skill["maintain"].false_negatives == 0 + ) + # The other three cases are unaffected. + assert report.per_skill["explore"].recall == 1.0 + assert report.per_skill["transform"].recall == 1.0 + + +class _UnavailableClassifier: + """Raises on the very first call and never gets a chance to run again, + modeling ClaudeNotAvailableError: the CLI binary is missing, so every + call would fail identically, and there is nothing case-specific about + it.""" + + def classify(self, prompt: str) -> ClassifyResult: + raise RuntimeError("'claude' not found on PATH") + + +def test_run_corpus_lets_a_setup_failure_abort_the_run(): + # Unlike a per-call failure (reported via ClassifyResult.error and kept + # in the results list), a classifier that raises is a setup problem the + # whole run cannot proceed past. run_corpus does not catch it: it is left + # to propagate to the caller (main() catches ClaudeNotAvailableError + # specifically and exits 2), rather than being recorded once per corpus + # case with the CLI still reporting a clean, misleading exit. + with pytest.raises(RuntimeError, match="not found on PATH"): + run_corpus(_corpus(), _UnavailableClassifier()) + + +def test_the_committed_ade_bench_corpus_loads_and_validates(): + corpus = load_corpus(_REPO / "evals" / "corpus" / "ade_bench_triggering.json") + assert corpus.name == "ade_bench_triggering" + assert corpus.source["license"] == "Apache-2.0" + assert corpus.source["repo"] + assert len(corpus.cases) >= 20 + valid = {"explore", "transform", "maintain", None} + for case in corpus.cases: + assert case.task_id + assert case.prompt + assert case.expected_skill in valid + + +def test_corpus_requires_provenance(tmp_path): + import json + + path = tmp_path / "no_source.json" + path.write_text( + json.dumps({"name": "x", "cases": [{"prompt": "p", "expected_skill": "none"}]}), + encoding="utf-8", + ) + with pytest.raises(InvalidSuiteError, match="source"): + load_corpus(path) + + +def test_corpus_case_requires_expected_skill(tmp_path): + import json + + path = tmp_path / "no_label.json" + path.write_text( + json.dumps( + { + "name": "x", + "source": {"repo": "https://example.invalid", "license": "MIT"}, + "cases": [{"prompt": "p"}], + } + ), + encoding="utf-8", + ) + with pytest.raises(InvalidSuiteError, match="expected_skill"): + load_corpus(path) + + +def test_cli_requires_a_skill_unless_corpus_is_given(): + from evals.__main__ import main + + with pytest.raises(SystemExit) as exc: + main([]) + assert exc.value.code == 2 diff --git a/references/evaluation.md b/references/evaluation.md index a5c75e2b..e6d47edc 100644 --- a/references/evaluation.md +++ b/references/evaluation.md @@ -54,6 +54,16 @@ quality (the hard constraints as executable assertions), and uplift versus baseline. Three skills share a description budget, so negative cases are first-class. +Every one of those triggering cases is written by whoever wrote the +description it tests, at the same time, and checked with the other skills +disabled: a structural blind spot a same-author suite cannot see past. +`evals/corpus/` supplements it with externally authored prompts (ADE-bench +task text, hand-labeled with the skill each should fire or `none`), run with +every skill available at once and scored as per-skill precision/recall +(`python -m evals --corpus evals/corpus/ade_bench_triggering.json`). It never +gates: a low pass rate is the measurement this corpus exists to produce, not +a regression to chase away. + ## Tier 3: external benchmarks (published) Scheduled and cost-capped, not per-commit. Two are published, each with its raw