Add per-skill eval scaffolding: contract tests, trigger evals, output cases

[lucasrodes]

Sets up evaluation for the four skills. The structure is the thing to review; I kept it uncommitted to any particular runner or grading service, so a layer can be dropped or a tool swapped without unpicking the rest.

Approach

Three layers, cheapest and most deterministic first. They answer different questions, and only the third needs a model grading output.

Layer	Question	Model?	Cost	Where
1. Contract tests	Do the endpoints and response shapes documented in SKILL.md still match reality?	no	seconds	evals/skills/<skill>/contract.sh
2. Trigger evals	Does the skill fire when it should, and not steal a sibling's job?	yes (routing only)	minutes	evals/skills/<skill>/triggers.json
3. Output evals	Is the output good, and better than no skill at all?	yes + a human	tens of minutes	evals/skills/<skill>/evals.json

Layer 1 comes first because it targets how these skills will actually break: they are thin documentation over live public HTTP, so they rot when the API changes, not when the prose gets worse. Layer 2 is next because all four skills describe overlapping subject matter — misrouting between them is a likelier defect than a bad answer.

The layer 3 file format matches the evals.json convention anthropics/skills' skill-creator expects, so its tooling can drive our cases later without a migration. Placement deliberately differs from getsentry/skills and openai/skills, which put evals inside the skill directory — see the distribution note below.

Layout

Makefile                            # entry points — what you actually run
evals/
├── README.md                       # the playbook: how to run each layer, how to iterate
├── lib/assert.sh                   # fetch / jq_true / has_keys / csv_header / skill_md_contains …
├── run-contract-tests.sh           # layer 1
├── run-trigger-eval.py             # layer 2 (uv script, stdlib only)
├── results/                        # gitignored
└── skills/<skill>/                 # mirrors skills/, one directory per skill
    ├── contract.sh
    ├── triggers.json               # 10 queries, 5 positive + 5 near-miss
    ├── evals.json                  # 3 cases: happy path / specific guidance / edge
    └── fixtures/                   # joining-data only, for now

skills/<skill>/SKILL.md             # the skill, and nothing else

Everything directly under evals/ is harness; everything under evals/skills/ is per-skill input, named exactly as the skill it tests. The runners glob evals/skills/*/, so they cannot mistake lib/ or results/ for a skill.

Two rules, both now enforced by make validate rather than trusted:

• Commit inputs, never outputs. Cases and fixtures are source; everything a run produces goes to the gitignored evals/results/.
• No SKILL.md references any eval file. Otherwise every user who triggers a skill pays context for test prose.

Evals are outside skills/ on purpose

Worth reviewing this bit specifically, because it reverses my first instinct.

A skill directory is the unit of distribution and is copied recursively into users' projects. The skills CLI the README recommends for Codex/Gemini/Cursor excludes only metadata.json, .git, __pycache__ and __pypackages__ — no ignore file, no opt-out. So anything under skills/<skill>/ ships to everyone who runs npx skills add owid/skills.

Skill activation was never the risk: progressive disclosure loads only name+description at startup, and nothing references the eval files. The risk is an agent's own filesystem exploration of a project that has these skills installed. Fixture CSVs are realistic by design, so once they land in .agents/skills/ they are indistinguishable from the user's own data. Simulated both layouts:

evals inside skills/          evals outside skills/
find . -name '*.csv'   3 hits, 2 ours     ->   1 hit, the user's own
grep -ril "per capita" 7 hits, 5 evals    ->   2 hits, both real SKILL.md prose
installed footprint    108K               ->   32K

Keeping evals in a sibling directory removes the possibility rather than mitigating it — the installer never sees them. Directory names still mirror the skills, so the locality cost is one hop.

Running it

make                        # list targets
make validate               # spec conformance, manifest, registration, no-eval-references
make lint                   # shellcheck for the shell, ruff for the Python
make test                   # contract tests (layer 1) — this is what CI runs
make test SKILL=<name>      # one skill
make triggers SKILL=<name>  # trigger evals (layer 2) — costs tokens

Five targets. Layer 3 gets none: it has no runner yet, and a catch-all "run all evals" would quietly spend tokens on layer 2.

make validate runs four checks, none of which subsumes another:

Check	Tool	Why separately
Spec conformance, per skill	skills-ref (the validator the spec recommends)	Agent-agnostic — covers Codex/Gemini/Cursor users
Marketplace manifest	claude plugin validate	Claude-specific, outside the spec
Marketplace registration	ours	Neither validator knows marketplace.json exists
No SKILL.md references evals	ours	The only path by which evals could reach an agent's context

Verified against four breakage classes: frontmatter name mismatch, a deleted required field, an unregistered skill, and a SKILL.md that references an eval file. Note skills-ref's published executable is agentskills, not skills-ref as the spec page shows; it is pinned >=0.1.1,<0.2 since it is pre-1.0.

CI has two jobs. validate is fast and should always be green; contract-tests runs on PRs and nightly at 06:00 UTC. That separates "someone broke the repo" from "the API moved".

Layer 2 loads all four skills together via --plugin-dir and records which skill fired, giving four outcomes: correct, miss, misroute, false_positive. triggers.json keeps the {query, should_trigger} shape skill-creator uses, plus an optional expected_skill for "a sibling should win this one":

{
  "query": "grab the csv behind https://ourworldindata.org/grapher/life-expectancy for the USA",
  "should_trigger": false,
  "expected_skill": "fetch-chart-data",
  "note": "url already known — discovery is not needed"
}

What the contract tests found

104 checks pass, 7 fail. All seven are real documentation drift, not broken tests. They are left failing on purpose so the first run reports the drift instead of freezing it in. #18 fixes all seven and takes the suite to 120/0.

Skill	Finding
search-charts	objectID documented as required on every hit — returned on 0 of 200 sampled.
search-charts	explorerType documented as required on SearchExplorerViewHit — never returned.
search-charts	chartConfigId documented as required on SearchMultiDimViewHit — never returned.
search-charts	The tab→URL table omits StackedDiscreteBar and Dumbbell, both of which the API returns. An agent can't build a ?tab= URL for either.
fetch-chart-data	descriptionKey typed string[] but is a single markdown bulleted string.
fetch-chart-data	Header documented as Entity,Code,Year, but the recommended useColumnShortNames=true returns lowercase entity,code,year. The recommendation contradicts the documented structure.
owid-catalog	search(..., sort_by="relevance") raises TypeError — no such parameter in owid-catalog 1.1.0. The documented example is broken code.

Two more are encoded as passing checks with a note, because they're traps rather than contradictions:

• A nonsense query returns five irrelevant hits, not zero. search-charts says "If you don't get any results, try slightly different terms" — but nbHits == 0 essentially never happens, so that guidance never fires and an agent can present unrelated charts as matches.
• csvType=filtered also applies the chart's own default entity selection. population.csv?csvType=filtered&time=2020 returns 7 rows — continents and World, no countries. joining-data reasonably assumes country rows are present.

And one in joining-data's prose a contract test can't assert: it tells agents to read $.columns.[0].timespan, but columns is an object keyed by column name, so that path is not valid jq. For population-with-un-projections it matters — the first column is estimates (1950–2023) and the projection to 2100 is a second column, so reading one column's timespan gives the wrong answer for exactly the recent/future-year case the skill recommends that chart for.

A harness bug worth knowing about

The failure list initially drifted between runs with no code changing, which turned out to be a bug in the tests rather than flakiness in the API.

q=energy+mix returned mostly explorerView hits one hour and only multiDimView hits the next. The explorerType assertion was [... | select(.type == "explorerView") | has("explorerType")] | all, and jq's all is true for an empty array — so with no explorerView hits in the sample it passed vacuously, silently retiring one of the findings this PR exists to demonstrate.

Same class of bug twice more: the tab-table check sits behind a [[ -s "$FILE" ]] guard that falls through with no output at all (renaming a variable made it stop running entirely, and nothing reported that); and contract_check.py gated four indicator checks on the semantic search succeeding, so a transient 502 from search.owid.io dropped all four from the summary.

Fixed by sampling a query that returns all three record types, asserting each type's shape only when that type is present, and skip-ing loudly with a reason otherwise. Fixing it surfaced two findings the fragile version was hiding: the chartConfigId drift, and Dumbbell missing from the tab table alongside StackedDiscreteBar.

The invariant is written down in evals/README.md: a check that did not run must never look like a check that passed. It applies to layer 2 as well — assert shapes, not populations.

Verification

• Layer 1: 104 passed / 7 failed against the live API, reproduced identically in CI.
• make lint is clean: shellcheck (with --external-sources) and ruff, both via uvx.
• A contract.sh still works run directly, without the runner's environment.
• Layer 2: detection validated against a real claude -p transcript — 2 hits in 80 lines, first was search-charts (the expected skill), no spurious matches from the available-skills listing. Fixed two bugs found doing this: claude -p blocking on stdin, and a fallback regex loose enough to match the skills listing itself.

Not done

• Layer 3 has no runner. Cases are definitions only. The with-skill/without-skill baseline and the grading/aggregation step are unimplemented.
• Trigger sets are 10 queries each, not the 20 Anthropic suggests. One query in evals/skills/joining-data/triggers.json is flagged in its note as genuinely ambiguous and wants a human call.
• No SKILL.md content was changed here — that's Fix the seven documentation drifts the contract tests found #18.
• Layer 2 costs real tokens (10 queries × 3 runs × 4 skills = 120 claude -p invocations), so it is deliberately not wired into CI.

Open questions

1. Are three layers too many for four skills, or is layer 1 alone enough for now?
2. Should a nightly contract-test failure notify anywhere, or is a red badge enough? A red run usually means OWID shipped an API change, not that someone broke this repo.
3. Bash is the weakest part of the design. A pytest + jsonschema rewrite would let the TypeScript types in each SKILL.md be checked structurally instead of via hand-transcribed jq assertions, and would express "expected to fail, tell me if it stops failing" as xfail(strict=True) — which is exactly what the seven intentional failures want, and would let CI be green. Worth doing now, or after the suite outgrows four skills?

🤖 Generated with Claude Code

[lucasrodes]

@codex review

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: c50563ddb5

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Treat failed Claude runs as evaluation failures

When a claude invocation exits nonzero, times out, or raises before emitting a Skill tool call, this handler records the run as fired: [] without preserving an error state. For a should_trigger: false case without expected_skill, classify() then marks that run correct, so authentication, configuration, or CLI failures can silently inflate negative-case accuracy instead of invalidating the evaluation.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Count failures from sibling-routing cases in the summary

Queries with should_trigger: false and expected_skill can be classified as miss or misroute, but these counters only examine positives, excluding every such sibling-routing case. As a result, the report can show reduced accuracy and rows labeled as failures while still claiming zero misses and zero misroutes, obscuring the main failure mode this runner is intended to diagnose.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Record skipped indicator checks as skips

With the documented SKIP_SLOW=1 make test invocation, this branch emits only a note, so the semantic-search check and its four dependent checks disappear from the totals and the suite can report zero skipped checks. This makes a deliberately reduced contract-test run look like a complete pass; emit explicit SKIP records for the omitted checks as the failure branch below already does.

Useful? React with 👍 / 👎.