From e8463e5613af9a22ff85c8f50859916dc393d38d Mon Sep 17 00:00:00 2001 From: Walker Hughes <74113220+walkerhughes@users.noreply.github.com> Date: Sun, 2 Aug 2026 17:58:18 -0700 Subject: [PATCH 1/3] feat(skills): add the prove-the-test-fails skill Encodes a rule from a real session: a test you just wrote is not verified until you have watched it fail for the right reason. The test that prompted it asserted that two retrieval strategies returned the same set of result types, which two empty lists satisfy. It was the only test guarding the project's central design invariant, it could not fail for the reason it existed, and it had been green in CI for weeks. Breaking the implementation on purpose was what found it. The skill covers when to reach for the loop, how to pick a mutation that is the smallest change at the seam the test claims to guard, what counts as confirmation including the blast radius across a parametrised suite, and reverting. It names the anti-patterns that make the exercise worthless, mutating too broadly or at the wrong layer or leaving the mutation behind, and the case where a surviving test is correct rather than decorative. Packaged as a top-level skills/ directory with its own marketplace entry using source "./" and a skills path, per the marketplace docs' pattern for several entries sharing one skills folder. The entry carries no version so Claude Code resolves it from the commit SHA and installed copies refresh without a manual bump. Two checks, both per-skill through paths filters. make check validates the frontmatter and the marketplace entry, then proves the eval fixture is still a trap by applying the mutation an agent is meant to find and asserting the exact shape of the run that follows. make evals gives a headless Claude Code run the fixture and grades on behaviour: the verdict is only reachable by breaking a strategy, the tracked files must match the starting commit, and the suite must be green again. A --control flag runs the same task without the skill. In the runs on record the control also passes, so this is a regression guard on the guidance rather than evidence of uplift, and the eval README says so. --- .claude-plugin/marketplace.json | 7 ++ .../workflows/skill-prove-the-test-fails.yml | 59 +++++++++++ README.md | 22 ++++- skills/prove-the-test-fails/Makefile | 10 ++ skills/prove-the-test-fails/SKILL.md | 82 ++++++++++++++++ skills/prove-the-test-fails/evals/README.md | 70 +++++++++++++ .../evals/check_fixture.sh | 80 +++++++++++++++ .../evals/check_wiring.py | 59 +++++++++++ .../evals/fixture/pyproject.toml | 3 + .../evals/fixture/src/retrieval.py | 57 +++++++++++ .../evals/fixture/tests/test_contract.py | 26 +++++ skills/prove-the-test-fails/evals/prompt.md | 14 +++ skills/prove-the-test-fails/evals/run_eval.sh | 98 +++++++++++++++++++ 13 files changed, 583 insertions(+), 4 deletions(-) create mode 100644 .github/workflows/skill-prove-the-test-fails.yml create mode 100644 skills/prove-the-test-fails/Makefile create mode 100644 skills/prove-the-test-fails/SKILL.md create mode 100644 skills/prove-the-test-fails/evals/README.md create mode 100755 skills/prove-the-test-fails/evals/check_fixture.sh create mode 100644 skills/prove-the-test-fails/evals/check_wiring.py create mode 100644 skills/prove-the-test-fails/evals/fixture/pyproject.toml create mode 100644 skills/prove-the-test-fails/evals/fixture/src/retrieval.py create mode 100644 skills/prove-the-test-fails/evals/fixture/tests/test_contract.py create mode 100644 skills/prove-the-test-fails/evals/prompt.md create mode 100755 skills/prove-the-test-fails/evals/run_eval.sh diff --git a/.claude-plugin/marketplace.json b/.claude-plugin/marketplace.json index 4c5f5d6..cd299ef 100644 --- a/.claude-plugin/marketplace.json +++ b/.claude-plugin/marketplace.json @@ -15,6 +15,13 @@ "name": "tastytrade", "source": "./plugins/tastytrade", "description": "Inspect TastyTrade brokerage accounts, positions, market data, option chains, and transactions from Claude Code. Order placement is gated off by default." + }, + { + "name": "prove-the-test-fails", + "source": "./", + "strict": false, + "skills": ["./skills/prove-the-test-fails"], + "description": "Break the code under test to confirm a test can actually fail, and fails for the right reason, before trusting it." } ] } diff --git a/.github/workflows/skill-prove-the-test-fails.yml b/.github/workflows/skill-prove-the-test-fails.yml new file mode 100644 index 0000000..0c9aaa9 --- /dev/null +++ b/.github/workflows/skill-prove-the-test-fails.yml @@ -0,0 +1,59 @@ +name: skill prove-the-test-fails + +# One workflow per skill, matching the per-component paths filters used for the +# MCP servers, so a change to one skill never runs another's checks. +# +# marketplace.json is in the filter because it is the shared file: an entry +# edited by another skill's pull request can silently stop this one loading. +on: + pull_request: + paths: + - "skills/prove-the-test-fails/**" + - ".claude-plugin/marketplace.json" + - ".github/workflows/skill-prove-the-test-fails.yml" + push: + branches: [main] + paths: + - "skills/prove-the-test-fails/**" + - ".claude-plugin/marketplace.json" + - ".github/workflows/skill-prove-the-test-fails.yml" + +defaults: + run: + working-directory: skills/prove-the-test-fails + +jobs: + wiring: + # Packaging, marketplace entry, and the fixture's honesty. No credentials. + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - uses: astral-sh/setup-uv@v5 + with: + python-version: "3.12" + - run: make check + - run: npm install -g @anthropic-ai/claude-code + - name: Validate the marketplace manifest + working-directory: . + run: claude plugin validate . --strict + + evals: + # Behavioural: does the skill lead to breaking the code and reading the + # result. Costs Anthropic tokens, so it skips without a key (fork PRs get + # no secrets). + runs-on: ubuntu-latest + env: + ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} + steps: + - uses: actions/checkout@v4 + - uses: astral-sh/setup-uv@v5 + with: + python-version: "3.12" + - name: Run the behavioural eval + run: | + if [ -z "${ANTHROPIC_API_KEY:-}" ]; then + echo "ANTHROPIC_API_KEY absent (fork PR?); skipping the behavioural eval." + exit 0 + fi + npm install -g @anthropic-ai/claude-code + make evals diff --git a/README.md b/README.md index e03955b..621ef30 100644 --- a/README.md +++ b/README.md @@ -15,6 +15,18 @@ Formerly `walkerhughes/mcps`, back when it only held MCP servers. They follow Honeycomb's [MCP, easy as 1-2-3](https://www.honeycomb.io/blog/mcp-easy-as-1-2-3) guidance: a few curated tools built around real questions rather than raw API endpoints, responses shaped for a model instead of a UI, and typed schemas that steer the model toward valid calls. +## Skills + +Skills live in top-level `skills/`, one directory each, and ship through the marketplace as their own installable entries. + +| Skill | What it does | +|-------|--------------| +| [`prove-the-test-fails`](skills/prove-the-test-fails/) | Breaks the code under test to confirm a test can actually fail, and fails for the right reason, before trusting it. | + +A skill's marketplace entry sets `source: "./"` with a `skills` path pointing at its own directory, so several skills share the one top-level folder without loading each other, and `strict: false` because the repository root has no `plugin.json` to be the authority. Entries deliberately carry no `version`: Claude Code then resolves the version from the commit SHA, so every change reaches installed copies without a manual bump, and the stale-cache trap described below does not apply. + +Each skill directory carries its own checks, run from that directory: `make check` for packaging and wiring, which needs no credentials, and `make evals` for behaviour, which costs tokens. CI runs both per skill through `paths` filters, as it does for the servers. + ## Install a plugin Run these as two separate commands, not as one paste: the first opens a prompt that expects only the `owner/repo`. @@ -44,11 +56,13 @@ Plugins require [`uv`](https://docs.astral.sh/uv/) on your PATH. The first launc ``` claude/ ├── .claude-plugin/ # marketplace manifest -└── plugins/ - ├── harbor-hub/ - └── tastytrade/ +├── plugins/ +│ ├── harbor-hub/ +│ └── tastytrade/ +└── skills/ + └── prove-the-test-fails/ ``` -Plugins live under `plugins/`, one directory each, named for the platform they talk to rather than for being an MCP server. Skills and other components get their own top-level directories as they arrive. +Plugins live under `plugins/`, one directory each, named for the platform they talk to rather than for being an MCP server. Skills live under `skills/`, named for the practice they encode. Other components get their own top-level directories as they arrive. Each plugin directory is self-contained: its own `README.md` covers install, credentials, tests, and tools. CI runs per subdirectory via `paths` filters, so a change to one never runs another's suite. diff --git a/skills/prove-the-test-fails/Makefile b/skills/prove-the-test-fails/Makefile new file mode 100644 index 0000000..9f5b9a4 --- /dev/null +++ b/skills/prove-the-test-fails/Makefile @@ -0,0 +1,10 @@ +.PHONY: check evals + +# Packaging, wiring, and the fixture's honesty. No API key, no cost. +check: + python3 evals/check_wiring.py + ./evals/check_fixture.sh + +# Behavioural eval. Needs Claude Code on PATH and working credentials. +evals: + ./evals/run_eval.sh diff --git a/skills/prove-the-test-fails/SKILL.md b/skills/prove-the-test-fails/SKILL.md new file mode 100644 index 0000000..488b8f0 --- /dev/null +++ b/skills/prove-the-test-fails/SKILL.md @@ -0,0 +1,82 @@ +--- +name: prove-the-test-fails +description: Break the code under test to confirm a test can actually fail, and fails for the right reason, before trusting it. Use after writing or changing a test, before citing a green run as evidence that something works, when a test has never been seen red, when a suite is said to enforce a contract or invariant, or when an assertion could be satisfied by an empty or degenerate result. +--- + +# Prove the test fails + +A test you just wrote is not verified until you have watched it fail for the right reason. +A green run tells you the assertions did not raise. It does not tell you they could. + +## When to run this + +- A test you just wrote or just refactored, before moving on. +- A test that has never been observed failing, including one inherited green from CI. +- Any suite described as guarding a contract, an invariant, or two implementations agreeing. + These are the ones that go vacuous quietly, because the assertion is about shape rather + than content. +- Before reporting "the tests pass" as evidence that a change works. + +An assertion that compares derived collections is the highest-risk shape: sets of types, +sorted key lists, lengths, `is not None`. Two empty results satisfy most of them. + +## The loop + +1. **Pick the mutation.** The smallest change to the code under test that should trip this + test, applied at the seam the test claims to guard. If the test says two strategies + agree, break one strategy. If it says a parser rejects bad input, make the parser + accept it. +2. **Run the suite** and read the output, not the exit code. +3. **Confirm the failure is the right one.** The test you are verifying is among the + failures, and its message names the thing the test exists to protect. A test that fails + with an import error or a fixture error has not been verified. +4. **Confirm the blast radius.** In a parametrised or multi-implementation suite, breaking + implementation A must fail A's cases and leave B's passing. Wrong radius means the test + is measuring something other than what its name says. +5. **Revert the mutation** and confirm the suite is green again. Always. A mutation left + behind is a broken repository. + +If the suite is unchanged by the mutation, the test is decorative. Fix the test, then run +this loop again on the fixed test. + +## Choosing the mutation + +Smallest, at the seam, one at a time. + +- Stub a function to return an empty result, a constant, or `None`. +- Invert a single comparison or drop a single filter. +- Delete one branch of the behaviour the test names. + +Do not mutate configuration, imports, or a shared base class to break a specific test. +Those produce failures everywhere and prove nothing about this test. + +## When a surviving test is correct + +Not every test that survives a mutation is decorative. A test can legitimately pass under +a break that is outside what it asserts. Stubbing a search function to return nothing +leaves `test_empty_query_returns_nothing` passing, and that is correct: the test asserts +emptiness, and emptiness is what it got. It would still fail if the function returned +something for an empty query, which is the failure it exists to catch. + +The question is never "did this test survive" but "can this test fail for the reason it +exists". Verify a surviving test with a mutation aimed at its own claim. + +## Anti-patterns + +- **Mutating too broadly.** Breaking a shared import or renaming a fixture fails the whole + suite and tells you nothing about the test in front of you. +- **Mutating the wrong layer.** Breaking the database when the test guards the parser + produces a red run that is not evidence. +- **Reading the exit code only.** A non-zero exit with your test still passing means the + test is decorative and something else caught the break. +- **Forgetting to revert.** Revert before you do anything else, and confirm green. +- **Treating every survivor as a defect.** See above. + +## What to report + +Name the mutation, the cases that failed, and the cases that did not: + +> Stubbed `lexical.search` to return `[]`. Exactly 10 `[lexical]` cases failed, zero +> `[bm25]`. Reverted, suite green. + +That is evidence. "50 tests pass" is not. diff --git a/skills/prove-the-test-fails/evals/README.md b/skills/prove-the-test-fails/evals/README.md new file mode 100644 index 0000000..3d12361 --- /dev/null +++ b/skills/prove-the-test-fails/evals/README.md @@ -0,0 +1,70 @@ +# prove-the-test-fails evals + +Two checks over one fixture. The fixture is a small retrieval library with a suite that +is green and a contract test that cannot fail, reproducing a defect found in a real +codebase: an assertion comparing sets of result types, on a query that matches nothing, +so it reduces to `set() == set()` whatever the strategies do. + +``` +fixture/ +├── src/retrieval.py two lexical strategies over one corpus +└── tests/test_contract.py five passing tests, one of them decorative +``` + +The fixture also carries the two cases the skill warns about. Stubbing `lexical_search` +to return nothing fails only the `[lexical]` cases of `test_finds_a_phrase_from_the_corpus` +and leaves the `[bm25]` cases green, which is the blast radius a correct mutation +produces. And `test_empty_query_returns_nothing[lexical]` survives that same mutation +legitimately, because asserting emptiness is satisfied by an empty implementation. + +## The eval + +```bash +./run_eval.sh [--model sonnet] # costs LLM tokens +./run_eval.sh --control # same task, skill absent +``` + +Copies the fixture into a scratch git repository, drops `SKILL.md` in as a project skill, +and asks a headless Claude Code run whether the suite actually guards the rule it claims +to. Grading is on behaviour rather than prose: + +| Check | Why | +| --- | --- | +| First line of `VERDICT.txt` is `UNGUARDED` | Only knowable by breaking a strategy and watching the contract test stay green | +| `src`, `tests`, and `pyproject.toml` match the starting commit | Any mutation was reverted | +| The suite is green again | The repository was left working | + +The skill is never named in the prompt, so an automatic load is also a test of the +`description` field. + +**What this measures.** It is a regression guard on the guidance: an edit to `SKILL.md` +that drops the revert step, or that stops the skill loading on this kind of question, +turns it red. It is not evidence of uplift. In the runs on record the control passes too, +so a current model reaches the same answer on this fixture unprompted. Raising the +fixture's difficulty until the control fails is the way to turn this into an uplift +measurement, and until that happens the eval should not be described as one. + +## The fixture check + +```bash +./check_fixture.sh # no agent, no LLM, runs in CI +``` + +The eval only asks a real question while the fixture's contract test genuinely cannot +fail. This applies the mutation an agent is expected to find and asserts the exact shape +of the run that follows: the contract test survives, the `[lexical]` phrase case dies, +the `[bm25]` case lives, and the empty-query case survives. Repairing the fixture's +assertion turns this red with a message saying the eval no longer poses its question. It +works on a copy, so the checked-in fixture is never mutated. + +## Why not a Harbor task + +The MCP plugins in this repo gate on [Harbor](https://www.harborframework.com) tasks, +which is the right shape there: an MCP server has to be exercised as a server, in a +container, against a live hub. A skill is a markdown file that has to be present in the +agent's own skill directory, and the version under test is the one in the branch. The +Harbor tasks here install their subject from GitHub's default branch, so a task would +gate on the published skill rather than the change under review. Containerising also +brings a hub key and Modal for no gain, since nothing here talks to a hub. A local +headless run keeps the same three phases, fixture then agent then grade, without any of +that. diff --git a/skills/prove-the-test-fails/evals/check_fixture.sh b/skills/prove-the-test-fails/evals/check_fixture.sh new file mode 100755 index 0000000..9642c7d --- /dev/null +++ b/skills/prove-the-test-fails/evals/check_fixture.sh @@ -0,0 +1,80 @@ +#!/bin/bash +# Proves the eval fixture is still a trap. No agent, no LLM, no network beyond +# fetching pytest. +# +# The agentic eval is only meaningful while `test_strategies_satisfy_the_same_contract` +# genuinely cannot fail. This applies the mutation the eval expects an agent to find, +# stubbing `lexical_search` to return nothing, and asserts the exact shape of the +# resulting run: the contract test survives, the lexical cases of the phrase test die, +# the bm25 cases live, and the empty-query test survives for its own good reason. If +# someone repairs the fixture's assertion, this goes red and says so. +# +# Runs on a copy: the checked-in fixture is never mutated. +set -euo pipefail + +EVALS_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +PYTEST=(uv run --no-project --with pytest pytest -v --tb=no -p no:cacheprovider) + +work="$(mktemp -d)" +trap 'rm -rf "$work"' EXIT +cp -R "$EVALS_DIR/fixture/." "$work/" + +report="$work/report.txt" + +echo "==> baseline: the fixture suite must be green" +(cd "$work" && "${PYTEST[@]}") > "$report" 2>&1 || { + cat "$report" + echo "FAIL: the untouched fixture suite is not green." >&2 + exit 1 +} + +echo "==> mutation: stub lexical_search to return nothing" +(cd "$work" && python3 - <<'PY' +import pathlib + +source = pathlib.Path("src/retrieval.py") +text = source.read_text() +anchor = ' """Full-text ranking: every match scores the same, ties break on identifier."""\n' +if anchor not in text: + raise SystemExit("FAIL: lexical_search no longer looks the way this check expects.") +source.write_text(text.replace(anchor, anchor + " return []\n", 1)) +PY +) + +echo "==> the mutated suite must fail in one specific shape" +(cd "$work" && "${PYTEST[@]}") > "$report" 2>&1 || true + +status=0 +expect() { + local node=$1 want=$2 why=$3 + if grep -qF "::${node} ${want}" "$report"; then + echo " ok ${node} ${want}" + else + echo " FAIL ${node} expected ${want}: ${why}" >&2 + status=1 + fi +} + +# The trap itself: the one test that claims to guard the shared contract does not +# notice that a strategy stopped returning anything. +expect "test_strategies_satisfy_the_same_contract" PASSED \ + "the fixture's contract test can fail now, so the eval no longer poses the question it was written to pose" + +# Blast radius: the mutation is confined to the lexical strategy. +expect "test_finds_a_phrase_from_the_corpus[lexical]" FAILED \ + "the mutation stopped reaching the code the test covers" +expect "test_finds_a_phrase_from_the_corpus[bm25]" PASSED \ + "the mutation leaked into the other strategy, so the fixture no longer shows a blast radius" + +# The legitimate survivor: asserting emptiness is satisfied by an empty implementation. +expect "test_empty_query_returns_nothing[lexical]" PASSED \ + "the fixture no longer contains a test that correctly survives the mutation" + +if [ "$status" -ne 0 ]; then + echo + cat "$report" + echo "FAIL: the fixture is no longer the trap the eval needs." >&2 + exit 1 +fi + +echo "PASS: the fixture's contract test still cannot fail." diff --git a/skills/prove-the-test-fails/evals/check_wiring.py b/skills/prove-the-test-fails/evals/check_wiring.py new file mode 100644 index 0000000..86c6d2f --- /dev/null +++ b/skills/prove-the-test-fails/evals/check_wiring.py @@ -0,0 +1,59 @@ +#!/usr/bin/env python3 +"""Checks the skill's frontmatter and its marketplace entry. + +The marketplace manifest is shared, so another skill's pull request can edit this +skill out of it without touching a file in this directory. Nothing here needs +credentials or a network. +""" + +import json +import re +import sys +from pathlib import Path + +SKILL_DIR = Path(__file__).resolve().parent.parent +SKILL_NAME = SKILL_DIR.name +REPO_ROOT = SKILL_DIR.parent.parent +MARKETPLACE = REPO_ROOT / ".claude-plugin" / "marketplace.json" + +failures: list[str] = [] + + +def require(condition: object, message: str) -> None: + if condition: + print(f" ok {message}") + else: + print(f" FAIL {message}") + failures.append(message) + + +frontmatter = re.match(r"---\n(.*?)\n---\n", (SKILL_DIR / "SKILL.md").read_text(), re.S) +require(frontmatter, "SKILL.md opens with YAML frontmatter") + +if frontmatter: + fields = dict(re.findall(r"^([a-z-]+):[ ]*(.+)$", frontmatter.group(1), re.M)) + require(fields.get("name") == SKILL_NAME, f"frontmatter name is {SKILL_NAME}") + require(len(fields.get("description", "")) > 80, "description is long enough to trigger on") + +entries = [ + entry + for entry in json.loads(MARKETPLACE.read_text())["plugins"] + if f"./skills/{SKILL_NAME}" in entry.get("skills", []) +] +require(len(entries) == 1, "exactly one marketplace entry loads this skill") + +if len(entries) == 1: + entry = entries[0] + require(entry["source"] == "./", "the entry's source is the marketplace root") + require( + entry.get("strict") is False, + "the entry is strict: false, since the root has no plugin.json", + ) + require( + "version" not in entry, "the entry carries no version, so it resolves from the commit SHA" + ) + require(entry.get("description"), "the entry has a description for the plugin picker") + +if failures: + sys.exit(f"FAIL: {len(failures)} wiring problem(s).") +print("PASS: the skill is packaged and listed correctly.") diff --git a/skills/prove-the-test-fails/evals/fixture/pyproject.toml b/skills/prove-the-test-fails/evals/fixture/pyproject.toml new file mode 100644 index 0000000..875366f --- /dev/null +++ b/skills/prove-the-test-fails/evals/fixture/pyproject.toml @@ -0,0 +1,3 @@ +[tool.pytest.ini_options] +pythonpath = ["src"] +testpaths = ["tests"] diff --git a/skills/prove-the-test-fails/evals/fixture/src/retrieval.py b/skills/prove-the-test-fails/evals/fixture/src/retrieval.py new file mode 100644 index 0000000..1b29046 --- /dev/null +++ b/skills/prove-the-test-fails/evals/fixture/src/retrieval.py @@ -0,0 +1,57 @@ +"""Two lexical retrieval strategies over one small corpus. + +Both are expected to satisfy the same result contract: for a given query they +return `Evidence` objects drawn from the corpus, ordered best first. +""" + +from dataclasses import dataclass + +CORPUS: list[tuple[int, str]] = [ + (1, "chunks pack consecutive turns toward two hundred words"), + (2, "ranked results break ties on identifier"), + (3, "the search index is a projection of the chunk table"), + (4, "a quarter of real turns are under twenty words"), +] + + +@dataclass(frozen=True) +class Evidence: + """One retrieved passage and the score the strategy gave it.""" + + chunk_id: int + text: str + score: float + + +def _terms(query: str) -> list[str]: + return [term for term in query.lower().split() if term] + + +def _matches(term: str, text: str) -> bool: + return term in text.split() + + +def lexical_search(query: str) -> list[Evidence]: + """Full-text ranking: every match scores the same, ties break on identifier.""" + terms = _terms(query) + hits = [ + Evidence(chunk_id, text, 1.0) + for chunk_id, text in CORPUS + if any(_matches(term, text) for term in terms) + ] + return sorted(hits, key=lambda evidence: evidence.chunk_id) + + +def bm25_search(query: str) -> list[Evidence]: + """BM25-style ranking: a term matching fewer documents contributes more.""" + terms = _terms(query) + scored: list[Evidence] = [] + for chunk_id, text in CORPUS: + score = sum( + 1.0 / sum(1 for _, other in CORPUS if _matches(term, other)) + for term in terms + if _matches(term, text) + ) + if score: + scored.append(Evidence(chunk_id, text, score)) + return sorted(scored, key=lambda evidence: (-evidence.score, evidence.chunk_id)) diff --git a/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py b/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py new file mode 100644 index 0000000..fefdd0a --- /dev/null +++ b/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py @@ -0,0 +1,26 @@ +"""The suite guarding the two retrieval strategies.""" + +import pytest + +from retrieval import bm25_search, lexical_search + +STRATEGIES = {"bm25": bm25_search, "lexical": lexical_search} + + +@pytest.mark.parametrize("strategy", sorted(STRATEGIES)) +def test_finds_a_phrase_from_the_corpus(strategy: str) -> None: + results = STRATEGIES[strategy]("identifier") + assert [evidence.chunk_id for evidence in results] == [2] + + +@pytest.mark.parametrize("strategy", sorted(STRATEGIES)) +def test_empty_query_returns_nothing(strategy: str) -> None: + assert STRATEGIES[strategy]("") == [] + + +def test_strategies_satisfy_the_same_contract() -> None: + """Both strategies return the same kind of result for the same query.""" + query = "embeddings" + assert {type(evidence) for evidence in lexical_search(query)} == { + type(evidence) for evidence in bm25_search(query) + } diff --git a/skills/prove-the-test-fails/evals/prompt.md b/skills/prove-the-test-fails/evals/prompt.md new file mode 100644 index 0000000..5f7504f --- /dev/null +++ b/skills/prove-the-test-fails/evals/prompt.md @@ -0,0 +1,14 @@ +This repository has two lexical retrieval strategies, `lexical_search` and `bm25_search`, +and a suite in `tests/test_contract.py`. All five tests pass. + +Before code review leans on that suite, establish whether it actually guards the rule the +project cares about: that both strategies satisfy one shared result contract. + +You may change anything you like while you work, as long as you leave the repository +exactly as you found it. Then write your conclusion to `VERDICT.txt` in the repository +root. The first line must be exactly one of: + +- `GUARDED` if the suite would go red when a strategy stops satisfying the contract +- `UNGUARDED` if it would stay green + +Use the rest of the file for what you did and what you observed. diff --git a/skills/prove-the-test-fails/evals/run_eval.sh b/skills/prove-the-test-fails/evals/run_eval.sh new file mode 100755 index 0000000..4de47f1 --- /dev/null +++ b/skills/prove-the-test-fails/evals/run_eval.sh @@ -0,0 +1,98 @@ +#!/bin/bash +# The agentic eval for the prove-the-test-fails skill. Costs LLM tokens. +# +# Copies the fixture into a scratch git repository, makes the skill available to a +# headless Claude Code run as a project skill, and asks the question the skill exists to +# answer: can this test fail? Grading is on behaviour, not prose: +# +# 1. the verdict is UNGUARDED, which is only knowable by breaking a strategy +# 2. the tracked files are byte-identical to the starting commit, so any mutation the +# agent applied was reverted +# 3. the suite is green again at the end +# +# --control runs the same task with the skill absent, which is how the eval shows the +# guidance changed anything. +# +# Usage: run_eval.sh [--control] [--model ] +set -euo pipefail + +EVALS_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)" +SKILL_DIR="$(dirname "$EVALS_DIR")" +SKILL_NAME="$(basename "$SKILL_DIR")" +CLAUDE_BIN="${CLAUDE_BIN:-claude}" +PYTEST=(uv run --no-project --with pytest pytest -q --tb=no -p no:cacheprovider) + +with_skill=1 +model="" +while [ $# -gt 0 ]; do + case "$1" in + --control) with_skill=0 ;; + --model) model="$2"; shift ;; + *) echo "unknown argument: $1" >&2; exit 2 ;; + esac + shift +done + +# The transcript lives beside the repository rather than in it, so the agent never sees +# its own output as a file in the tree it is auditing. +run_dir="$(mktemp -d)" +work="$run_dir/repo" +log="$run_dir/agent.log" +mkdir -p "$work" +cp -R "$EVALS_DIR/fixture/." "$work/" + +git -C "$work" init -q +git -C "$work" add -A +git -C "$work" -c user.name=eval -c user.email=eval@localhost \ + commit -qm "the fixture, green" +base="$(git -C "$work" rev-parse HEAD)" + +# The skill is on disk but outside git, so a dirty tree can only mean the agent left +# something behind. +if [ "$with_skill" -eq 1 ]; then + mkdir -p "$work/.claude/skills/$SKILL_NAME" + cp "$SKILL_DIR/SKILL.md" "$work/.claude/skills/$SKILL_NAME/SKILL.md" + echo ".claude/" >> "$work/.git/info/exclude" + echo "==> running with the skill available" +else + echo "==> control run: the skill is absent" +fi + +claude_args=(-p "$(cat "$EVALS_DIR/prompt.md")" + --permission-mode acceptEdits + --allowedTools "Bash Read Edit Write Grep Glob") +if [ -n "$model" ]; then + claude_args+=(--model "$model") +fi + +(cd "$work" && "$CLAUDE_BIN" "${claude_args[@]}") 2>&1 | tee "$log" || true + +echo +echo "==> grading" +status=0 +check() { + local what=$1 + shift + if "$@" > /dev/null 2>&1; then + echo " ok $what" + else + echo " FAIL $what" + status=1 + fi +} + +verdict="$(head -n 1 "$work/VERDICT.txt" 2>/dev/null | tr -d '[:space:]' || true)" +check "verdict is UNGUARDED (got: '${verdict:-}')" \ + test "$verdict" = UNGUARDED +check "tracked files are unchanged, so any mutation was reverted" \ + git -C "$work" diff --quiet "$base" -- src tests pyproject.toml +suite_green() { (cd "$work" && "${PYTEST[@]}"); } +check "the suite is green again" suite_green + +if [ "$status" -ne 0 ]; then + echo "FAIL: transcript and repository left at $run_dir" >&2 + exit 1 +fi + +rm -rf "$run_dir" +echo "PASS" From f663918d3f103ba435be8646032202b9f9c69827 Mon Sep 17 00:00:00 2001 From: Walker Hughes Date: Sun, 2 Aug 2026 18:44:22 -0700 Subject: [PATCH 2/3] refactor(skills): generalise prove-the-test-fails beyond one codebase Everything committed here is meant to be reusable on any project, and this skill's practice already was. Its illustrations were not: every one came from the Python retrieval codebase that prompted it, so a reader working in Go, TypeScript, Swift or Rust met a skill whose examples were all one search engine. State up front that no step depends on a language or a runner, and spread the examples. "Two strategies agree" becomes two implementations of one interface. The blast-radius step now names the shape it needs in three ecosystems, since table-driven subtests in Go, one describe per implementation in Jest and a parametrised pytest case report their cases the same way. The reporting example moves to a cache contract under Vitest and keeps what made it good: a named mutation, exact counts on both sides, and a confirmed revert. The empty-result survivor stays, restated without a search function. Recast the eval fixture as two route matchers, a compiled-regex one and a segment walk, rather than the origin project's two search strategies. The defect is unchanged: a contract test comparing sets of result types on an input that matches nothing, which reduces to set() == set() whichever implementation is broken. One fixture in one language stays the right trade, so this is a recast rather than a fixture per ecosystem. check_fixture.sh follows the rename and still asserts the exact run shape. It was verified against three mutations it must catch: a contract test made non-vacuous, a blast radius leaked into the second implementation, and drift in the line it anchors on. Each turned it red with the message that explains why, and reverting turned it green. The eval and its control both pass on the recast fixture, so the README's admission is unchanged and still accurate: with the skill absent the model reaches the same answer here, which makes this a regression guard on the guidance rather than evidence of uplift. --- skills/prove-the-test-fails/SKILL.md | 47 ++++++++------ skills/prove-the-test-fails/evals/README.md | 27 ++++---- .../evals/check_fixture.sh | 30 ++++----- .../evals/fixture/src/retrieval.py | 57 ----------------- .../evals/fixture/src/routing.py | 64 +++++++++++++++++++ .../evals/fixture/tests/test_contract.py | 31 +++++---- skills/prove-the-test-fails/evals/prompt.md | 8 +-- skills/prove-the-test-fails/evals/run_eval.sh | 2 +- 8 files changed, 141 insertions(+), 125 deletions(-) delete mode 100644 skills/prove-the-test-fails/evals/fixture/src/retrieval.py create mode 100644 skills/prove-the-test-fails/evals/fixture/src/routing.py diff --git a/skills/prove-the-test-fails/SKILL.md b/skills/prove-the-test-fails/SKILL.md index 488b8f0..c5474cf 100644 --- a/skills/prove-the-test-fails/SKILL.md +++ b/skills/prove-the-test-fails/SKILL.md @@ -8,31 +8,37 @@ description: Break the code under test to confirm a test can actually fail, and A test you just wrote is not verified until you have watched it fail for the right reason. A green run tells you the assertions did not raise. It does not tell you they could. +Nothing below depends on a language or a runner. `go test`, pytest, Jest, Vitest, XCTest, +RSpec and `cargo test` all report the same two things: which cases ran, and which failed. + ## When to run this - A test you just wrote or just refactored, before moving on. - A test that has never been observed failing, including one inherited green from CI. -- Any suite described as guarding a contract, an invariant, or two implementations agreeing. - These are the ones that go vacuous quietly, because the assertion is about shape rather - than content. +- Any suite described as guarding a contract, an invariant, or two implementations of one + interface agreeing. These are the ones that go vacuous quietly, because the assertion is + about shape rather than content. - Before reporting "the tests pass" as evidence that a change works. An assertion that compares derived collections is the highest-risk shape: sets of types, -sorted key lists, lengths, `is not None`. Two empty results satisfy most of them. +sorted key lists, lengths, non-null checks. Two empty results satisfy most of them. ## The loop 1. **Pick the mutation.** The smallest change to the code under test that should trip this - test, applied at the seam the test claims to guard. If the test says two strategies - agree, break one strategy. If it says a parser rejects bad input, make the parser - accept it. + test, applied at the seam the test claims to guard. If the test says two implementations + of one interface agree, break one of them. If it says a parser rejects bad input, make + the parser accept it. 2. **Run the suite** and read the output, not the exit code. 3. **Confirm the failure is the right one.** The test you are verifying is among the failures, and its message names the thing the test exists to protect. A test that fails - with an import error or a fixture error has not been verified. -4. **Confirm the blast radius.** In a parametrised or multi-implementation suite, breaking - implementation A must fail A's cases and leave B's passing. Wrong radius means the test - is measuring something other than what its name says. + to compile, or dies in an import or a fixture, has not been verified. +4. **Confirm the blast radius.** Where one test body runs over several inputs or several + implementations, breaking A must fail A's cases and leave B's passing. Table-driven + subtests in Go, one `describe` per implementation in Jest, a parametrised case in + pytest: three names for the shape this step needs, and each reports its cases + separately. Wrong radius means the test is measuring something other than what its name + says. 5. **Revert the mutation** and confirm the suite is green again. Always. A mutation left behind is a broken repository. @@ -43,20 +49,20 @@ this loop again on the fixed test. Smallest, at the seam, one at a time. -- Stub a function to return an empty result, a constant, or `None`. +- Return an empty collection, a constant, or a null from one function. - Invert a single comparison or drop a single filter. - Delete one branch of the behaviour the test names. -Do not mutate configuration, imports, or a shared base class to break a specific test. -Those produce failures everywhere and prove nothing about this test. +Do not mutate configuration, build settings, imports, or a shared base class to break a +specific test. Those produce failures everywhere and prove nothing about this test. ## When a surviving test is correct Not every test that survives a mutation is decorative. A test can legitimately pass under -a break that is outside what it asserts. Stubbing a search function to return nothing -leaves `test_empty_query_returns_nothing` passing, and that is correct: the test asserts -emptiness, and emptiness is what it got. It would still fail if the function returned -something for an empty query, which is the failure it exists to catch. +a break that is outside what it asserts. Stub a function to return nothing and a test +asserting that it returns nothing for a degenerate input keeps passing, and that is +correct: the test asserts emptiness, and emptiness is what it got. It would still fail if +the function returned something for that input, which is the failure it exists to catch. The question is never "did this test survive" but "can this test fail for the reason it exists". Verify a surviving test with a mutation aimed at its own claim. @@ -76,7 +82,8 @@ exists". Verify a surviving test with a mutation aimed at its own claim. Name the mutation, the cases that failed, and the cases that did not: -> Stubbed `lexical.search` to return `[]`. Exactly 10 `[lexical]` cases failed, zero -> `[bm25]`. Reverted, suite green. +> Made `RedisCache.get` return `undefined` unconditionally. All 8 cases in the `redis` +> describe of `cache contract` failed; the 8 `memory` cases passed and nothing else moved. +> Reverted, `vitest run` green. That is evidence. "50 tests pass" is not. diff --git a/skills/prove-the-test-fails/evals/README.md b/skills/prove-the-test-fails/evals/README.md index 3d12361..9c3b584 100644 --- a/skills/prove-the-test-fails/evals/README.md +++ b/skills/prove-the-test-fails/evals/README.md @@ -1,20 +1,23 @@ # prove-the-test-fails evals -Two checks over one fixture. The fixture is a small retrieval library with a suite that -is green and a contract test that cannot fail, reproducing a defect found in a real -codebase: an assertion comparing sets of result types, on a query that matches nothing, -so it reduces to `set() == set()` whatever the strategies do. +Two checks over one fixture. The fixture is a small route matcher with a suite that is +green and a contract test that cannot fail, reproducing a defect shape found in a real +codebase: an assertion comparing sets of result types, on an input that matches nothing, +so it reduces to `set() == set()` whatever the implementations do. ``` fixture/ -├── src/retrieval.py two lexical strategies over one corpus +├── src/routing.py two implementations of one matcher over one route table └── tests/test_contract.py five passing tests, one of them decorative ``` -The fixture also carries the two cases the skill warns about. Stubbing `lexical_search` -to return nothing fails only the `[lexical]` cases of `test_finds_a_phrase_from_the_corpus` -and leaves the `[bm25]` cases green, which is the blast radius a correct mutation -produces. And `test_empty_query_returns_nothing[lexical]` survives that same mutation +Python because the checks need some real codebase to run, not because the skill is about +Python. A fixture per ecosystem would cost a lot and measure the same method. + +The fixture also carries the two cases the skill warns about. Stubbing `regex_router` to +return nothing fails only the `[regex]` case of `test_matches_a_route_with_a_parameter` +and leaves the `[segment]` case green, which is the blast radius a correct mutation +produces. And `test_empty_path_matches_nothing[regex]` survives that same mutation legitimately, because asserting emptiness is satisfied by an empty implementation. ## The eval @@ -30,7 +33,7 @@ to. Grading is on behaviour rather than prose: | Check | Why | | --- | --- | -| First line of `VERDICT.txt` is `UNGUARDED` | Only knowable by breaking a strategy and watching the contract test stay green | +| First line of `VERDICT.txt` is `UNGUARDED` | Only knowable by breaking an implementation and watching the contract test stay green | | `src`, `tests`, and `pyproject.toml` match the starting commit | Any mutation was reverted | | The suite is green again | The repository was left working | @@ -52,8 +55,8 @@ measurement, and until that happens the eval should not be described as one. The eval only asks a real question while the fixture's contract test genuinely cannot fail. This applies the mutation an agent is expected to find and asserts the exact shape -of the run that follows: the contract test survives, the `[lexical]` phrase case dies, -the `[bm25]` case lives, and the empty-query case survives. Repairing the fixture's +of the run that follows: the contract test survives, the `[regex]` parameter case dies, +the `[segment]` case lives, and the empty-path case survives. Repairing the fixture's assertion turns this red with a message saying the eval no longer poses its question. It works on a copy, so the checked-in fixture is never mutated. diff --git a/skills/prove-the-test-fails/evals/check_fixture.sh b/skills/prove-the-test-fails/evals/check_fixture.sh index 9642c7d..77bc23a 100755 --- a/skills/prove-the-test-fails/evals/check_fixture.sh +++ b/skills/prove-the-test-fails/evals/check_fixture.sh @@ -2,11 +2,11 @@ # Proves the eval fixture is still a trap. No agent, no LLM, no network beyond # fetching pytest. # -# The agentic eval is only meaningful while `test_strategies_satisfy_the_same_contract` +# The agentic eval is only meaningful while `test_routers_satisfy_the_same_contract` # genuinely cannot fail. This applies the mutation the eval expects an agent to find, -# stubbing `lexical_search` to return nothing, and asserts the exact shape of the -# resulting run: the contract test survives, the lexical cases of the phrase test die, -# the bm25 cases live, and the empty-query test survives for its own good reason. If +# stubbing `regex_router` to return nothing, and asserts the exact shape of the +# resulting run: the contract test survives, the regex case of the parameter test dies, +# the segment case lives, and the empty-path test survives for its own good reason. If # someone repairs the fixture's assertion, this goes red and says so. # # Runs on a copy: the checked-in fixture is never mutated. @@ -28,15 +28,15 @@ echo "==> baseline: the fixture suite must be green" exit 1 } -echo "==> mutation: stub lexical_search to return nothing" +echo "==> mutation: stub regex_router to return nothing" (cd "$work" && python3 - <<'PY' import pathlib -source = pathlib.Path("src/retrieval.py") +source = pathlib.Path("src/routing.py") text = source.read_text() -anchor = ' """Full-text ranking: every match scores the same, ties break on identifier."""\n' +anchor = ' """Compiled patterns: each route becomes a regex the whole path must match."""\n' if anchor not in text: - raise SystemExit("FAIL: lexical_search no longer looks the way this check expects.") + raise SystemExit("FAIL: regex_router no longer looks the way this check expects.") source.write_text(text.replace(anchor, anchor + " return []\n", 1)) PY ) @@ -56,18 +56,18 @@ expect() { } # The trap itself: the one test that claims to guard the shared contract does not -# notice that a strategy stopped returning anything. -expect "test_strategies_satisfy_the_same_contract" PASSED \ +# notice that an implementation stopped returning anything. +expect "test_routers_satisfy_the_same_contract" PASSED \ "the fixture's contract test can fail now, so the eval no longer poses the question it was written to pose" -# Blast radius: the mutation is confined to the lexical strategy. -expect "test_finds_a_phrase_from_the_corpus[lexical]" FAILED \ +# Blast radius: the mutation is confined to one of the two implementations. +expect "test_matches_a_route_with_a_parameter[regex]" FAILED \ "the mutation stopped reaching the code the test covers" -expect "test_finds_a_phrase_from_the_corpus[bm25]" PASSED \ - "the mutation leaked into the other strategy, so the fixture no longer shows a blast radius" +expect "test_matches_a_route_with_a_parameter[segment]" PASSED \ + "the mutation leaked into the other implementation, so the fixture no longer shows a blast radius" # The legitimate survivor: asserting emptiness is satisfied by an empty implementation. -expect "test_empty_query_returns_nothing[lexical]" PASSED \ +expect "test_empty_path_matches_nothing[regex]" PASSED \ "the fixture no longer contains a test that correctly survives the mutation" if [ "$status" -ne 0 ]; then diff --git a/skills/prove-the-test-fails/evals/fixture/src/retrieval.py b/skills/prove-the-test-fails/evals/fixture/src/retrieval.py deleted file mode 100644 index 1b29046..0000000 --- a/skills/prove-the-test-fails/evals/fixture/src/retrieval.py +++ /dev/null @@ -1,57 +0,0 @@ -"""Two lexical retrieval strategies over one small corpus. - -Both are expected to satisfy the same result contract: for a given query they -return `Evidence` objects drawn from the corpus, ordered best first. -""" - -from dataclasses import dataclass - -CORPUS: list[tuple[int, str]] = [ - (1, "chunks pack consecutive turns toward two hundred words"), - (2, "ranked results break ties on identifier"), - (3, "the search index is a projection of the chunk table"), - (4, "a quarter of real turns are under twenty words"), -] - - -@dataclass(frozen=True) -class Evidence: - """One retrieved passage and the score the strategy gave it.""" - - chunk_id: int - text: str - score: float - - -def _terms(query: str) -> list[str]: - return [term for term in query.lower().split() if term] - - -def _matches(term: str, text: str) -> bool: - return term in text.split() - - -def lexical_search(query: str) -> list[Evidence]: - """Full-text ranking: every match scores the same, ties break on identifier.""" - terms = _terms(query) - hits = [ - Evidence(chunk_id, text, 1.0) - for chunk_id, text in CORPUS - if any(_matches(term, text) for term in terms) - ] - return sorted(hits, key=lambda evidence: evidence.chunk_id) - - -def bm25_search(query: str) -> list[Evidence]: - """BM25-style ranking: a term matching fewer documents contributes more.""" - terms = _terms(query) - scored: list[Evidence] = [] - for chunk_id, text in CORPUS: - score = sum( - 1.0 / sum(1 for _, other in CORPUS if _matches(term, other)) - for term in terms - if _matches(term, text) - ) - if score: - scored.append(Evidence(chunk_id, text, score)) - return sorted(scored, key=lambda evidence: (-evidence.score, evidence.chunk_id)) diff --git a/skills/prove-the-test-fails/evals/fixture/src/routing.py b/skills/prove-the-test-fails/evals/fixture/src/routing.py new file mode 100644 index 0000000..d5c7f5c --- /dev/null +++ b/skills/prove-the-test-fails/evals/fixture/src/routing.py @@ -0,0 +1,64 @@ +"""Two implementations of one route matcher over one route table. + +Both are expected to satisfy the same result contract: for a given path they +return `Match` objects for the routes that accept it, most specific first. +""" + +import re +from dataclasses import dataclass + +ROUTES: list[tuple[int, str]] = [ + (1, "/users"), + (2, "/users/{id}"), + (3, "/users/{id}/settings"), + (4, "/health"), +] + + +@dataclass(frozen=True) +class Match: + """One route that accepts the path, and how specific that route is.""" + + route_id: int + pattern: str + specificity: int + + +def _is_parameter(segment: str) -> bool: + return segment.startswith("{") and segment.endswith("}") + + +def _specificity(pattern: str) -> int: + return sum(1 for segment in pattern.split("/") if segment and not _is_parameter(segment)) + + +def _ranked(matches: list[Match]) -> list[Match]: + return sorted(matches, key=lambda match: (-match.specificity, match.route_id)) + + +def regex_router(path: str) -> list[Match]: + """Compiled patterns: each route becomes a regex the whole path must match.""" + matches = [] + for route_id, pattern in ROUTES: + expression = "/".join( + "[^/]+" if _is_parameter(segment) else re.escape(segment) + for segment in pattern.split("/") + ) + if re.fullmatch(expression, path): + matches.append(Match(route_id, pattern, _specificity(pattern))) + return _ranked(matches) + + +def segment_router(path: str) -> list[Match]: + """Segment walk: the path is split on / and compared one segment at a time.""" + segments = path.split("/") + matches = [] + for route_id, pattern in ROUTES: + expected = pattern.split("/") + if len(expected) != len(segments): + continue + if all( + (_is_parameter(want) and got) or want == got for want, got in zip(expected, segments) + ): + matches.append(Match(route_id, pattern, _specificity(pattern))) + return _ranked(matches) diff --git a/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py b/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py index fefdd0a..563db16 100644 --- a/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py +++ b/skills/prove-the-test-fails/evals/fixture/tests/test_contract.py @@ -1,26 +1,25 @@ -"""The suite guarding the two retrieval strategies.""" +"""The suite guarding the two route matchers.""" import pytest +from routing import regex_router, segment_router -from retrieval import bm25_search, lexical_search +ROUTERS = {"regex": regex_router, "segment": segment_router} -STRATEGIES = {"bm25": bm25_search, "lexical": lexical_search} +@pytest.mark.parametrize("router", sorted(ROUTERS)) +def test_matches_a_route_with_a_parameter(router: str) -> None: + matches = ROUTERS[router]("/users/42") + assert [match.route_id for match in matches] == [2] -@pytest.mark.parametrize("strategy", sorted(STRATEGIES)) -def test_finds_a_phrase_from_the_corpus(strategy: str) -> None: - results = STRATEGIES[strategy]("identifier") - assert [evidence.chunk_id for evidence in results] == [2] +@pytest.mark.parametrize("router", sorted(ROUTERS)) +def test_empty_path_matches_nothing(router: str) -> None: + assert ROUTERS[router]("") == [] -@pytest.mark.parametrize("strategy", sorted(STRATEGIES)) -def test_empty_query_returns_nothing(strategy: str) -> None: - assert STRATEGIES[strategy]("") == [] - -def test_strategies_satisfy_the_same_contract() -> None: - """Both strategies return the same kind of result for the same query.""" - query = "embeddings" - assert {type(evidence) for evidence in lexical_search(query)} == { - type(evidence) for evidence in bm25_search(query) +def test_routers_satisfy_the_same_contract() -> None: + """Both routers return the same kind of result for the same path.""" + path = "/orders/7" + assert {type(match) for match in regex_router(path)} == { + type(match) for match in segment_router(path) } diff --git a/skills/prove-the-test-fails/evals/prompt.md b/skills/prove-the-test-fails/evals/prompt.md index 5f7504f..f9348a6 100644 --- a/skills/prove-the-test-fails/evals/prompt.md +++ b/skills/prove-the-test-fails/evals/prompt.md @@ -1,14 +1,14 @@ -This repository has two lexical retrieval strategies, `lexical_search` and `bm25_search`, -and a suite in `tests/test_contract.py`. All five tests pass. +This repository has two implementations of one route matcher, `regex_router` and +`segment_router`, and a suite in `tests/test_contract.py`. All five tests pass. Before code review leans on that suite, establish whether it actually guards the rule the -project cares about: that both strategies satisfy one shared result contract. +project cares about: that both implementations satisfy one shared result contract. You may change anything you like while you work, as long as you leave the repository exactly as you found it. Then write your conclusion to `VERDICT.txt` in the repository root. The first line must be exactly one of: -- `GUARDED` if the suite would go red when a strategy stops satisfying the contract +- `GUARDED` if the suite would go red when an implementation stops satisfying the contract - `UNGUARDED` if it would stay green Use the rest of the file for what you did and what you observed. diff --git a/skills/prove-the-test-fails/evals/run_eval.sh b/skills/prove-the-test-fails/evals/run_eval.sh index 4de47f1..8959078 100755 --- a/skills/prove-the-test-fails/evals/run_eval.sh +++ b/skills/prove-the-test-fails/evals/run_eval.sh @@ -5,7 +5,7 @@ # headless Claude Code run as a project skill, and asks the question the skill exists to # answer: can this test fail? Grading is on behaviour, not prose: # -# 1. the verdict is UNGUARDED, which is only knowable by breaking a strategy +# 1. the verdict is UNGUARDED, which is only knowable by breaking an implementation # 2. the tracked files are byte-identical to the starting commit, so any mutation the # agent applied was reverted # 3. the suite is green again at the end From 62300350631946c0d4584ba0672deb533a0769f9 Mon Sep 17 00:00:00 2001 From: Walker Hughes <74113220+walkerhughes@users.noreply.github.com> Date: Sun, 2 Aug 2026 23:11:53 -0700 Subject: [PATCH 3/3] refactor(skills): state prove-the-test-fails as principle and workflow The skill read as a procedure manual: a mutation catalogue, a five-item anti-pattern list, a section on legitimate survivors, and a runner inventory in the opening. Each piece was defensible and the whole was minutiae, which is not what a skill is for. Lead with the principle that a test is not evidence until observed failing for the reason it exists, keep the loop as the workflow it is, and cut the elaboration. What survives is what changes behaviour the principle alone would not settle: mutate at the seam the test claims to guard, check that the right cases failed and the wrong ones did not, and always revert. One anti-pattern and one gold standard remain, compressed. Body drops from 84 lines to 47. --- skills/prove-the-test-fails/SKILL.md | 117 +++++++++------------------ 1 file changed, 40 insertions(+), 77 deletions(-) diff --git a/skills/prove-the-test-fails/SKILL.md b/skills/prove-the-test-fails/SKILL.md index c5474cf..1b6349b 100644 --- a/skills/prove-the-test-fails/SKILL.md +++ b/skills/prove-the-test-fails/SKILL.md @@ -5,85 +5,48 @@ description: Break the code under test to confirm a test can actually fail, and # Prove the test fails -A test you just wrote is not verified until you have watched it fail for the right reason. -A green run tells you the assertions did not raise. It does not tell you they could. +A test is not evidence until it has been observed failing for the reason it exists. A green +run says the assertions did not raise; it does not say they could. So break the thing the +test guards, and watch what happens. -Nothing below depends on a language or a runner. `go test`, pytest, Jest, Vitest, XCTest, -RSpec and `cargo test` all report the same two things: which cases ran, and which failed. - -## When to run this - -- A test you just wrote or just refactored, before moving on. -- A test that has never been observed failing, including one inherited green from CI. -- Any suite described as guarding a contract, an invariant, or two implementations of one - interface agreeing. These are the ones that go vacuous quietly, because the assertion is - about shape rather than content. -- Before reporting "the tests pass" as evidence that a change works. - -An assertion that compares derived collections is the highest-risk shape: sets of types, -sorted key lists, lengths, non-null checks. Two empty results satisfy most of them. +Two things follow. Break at the seam the test claims to guard rather than wherever a break +is easy, because a red run the test did not cause proves nothing about that test. And +surviving a mutation does not make a test decorative, since it may assert something the +mutation left true. The question is never whether a test survived but whether it can fail +for the reason it exists, so re-aim at its own claim before calling it decorative. ## The loop 1. **Pick the mutation.** The smallest change to the code under test that should trip this - test, applied at the seam the test claims to guard. If the test says two implementations - of one interface agree, break one of them. If it says a parser rejects bad input, make - the parser accept it. -2. **Run the suite** and read the output, not the exit code. -3. **Confirm the failure is the right one.** The test you are verifying is among the - failures, and its message names the thing the test exists to protect. A test that fails - to compile, or dies in an import or a fixture, has not been verified. -4. **Confirm the blast radius.** Where one test body runs over several inputs or several - implementations, breaking A must fail A's cases and leave B's passing. Table-driven - subtests in Go, one `describe` per implementation in Jest, a parametrised case in - pytest: three names for the shape this step needs, and each reports its cases - separately. Wrong radius means the test is measuring something other than what its name - says. -5. **Revert the mutation** and confirm the suite is green again. Always. A mutation left - behind is a broken repository. - -If the suite is unchanged by the mutation, the test is decorative. Fix the test, then run -this loop again on the fixed test. - -## Choosing the mutation - -Smallest, at the seam, one at a time. - -- Return an empty collection, a constant, or a null from one function. -- Invert a single comparison or drop a single filter. -- Delete one branch of the behaviour the test names. - -Do not mutate configuration, build settings, imports, or a shared base class to break a -specific test. Those produce failures everywhere and prove nothing about this test. - -## When a surviving test is correct - -Not every test that survives a mutation is decorative. A test can legitimately pass under -a break that is outside what it asserts. Stub a function to return nothing and a test -asserting that it returns nothing for a degenerate input keeps passing, and that is -correct: the test asserts emptiness, and emptiness is what it got. It would still fail if -the function returned something for that input, which is the failure it exists to catch. - -The question is never "did this test survive" but "can this test fail for the reason it -exists". Verify a surviving test with a mutation aimed at its own claim. - -## Anti-patterns - -- **Mutating too broadly.** Breaking a shared import or renaming a fixture fails the whole - suite and tells you nothing about the test in front of you. -- **Mutating the wrong layer.** Breaking the database when the test guards the parser - produces a red run that is not evidence. -- **Reading the exit code only.** A non-zero exit with your test still passing means the - test is decorative and something else caught the break. -- **Forgetting to revert.** Revert before you do anything else, and confirm green. -- **Treating every survivor as a defect.** See above. - -## What to report - -Name the mutation, the cases that failed, and the cases that did not: - -> Made `RedisCache.get` return `undefined` unconditionally. All 8 cases in the `redis` -> describe of `cache contract` failed; the 8 `memory` cases passed and nothing else moved. -> Reverted, `vitest run` green. - -That is evidence. "50 tests pass" is not. + test, applied where the test says it is looking. If it claims two implementations agree, + break one of them. Not a shared import, a fixture, or a build setting: those fail + everything and say nothing about the test in front of you. +2. **Run the suite and read the output**, not the exit code. +3. **Confirm the failure is the right one.** The test under scrutiny is among the failures, + and its message names what that test protects. A run that dies in an import or a fixture + has verified nothing. +4. **Confirm the blast radius.** Where one test body runs over several inputs or + implementations, breaking one must fail its own cases and leave the rest green. The + wrong radius means the test measures something other than its name. +5. **Revert and confirm the suite is green.** Always, and before anything else. A mutation + left behind is a broken repository. + +One mutation at a time, so the failures have one cause. Independent seams are therefore +independent runs, and the loop fans out: give each seam its own working copy, run the loop +there, and collect which cases each break turned red. + +## An assertion that cannot fail + +Assertions about shape rather than content go vacuous quietly. A contract test comparing +the result types of two implementations, on an input that matches nothing, reduces to +comparing two empty sets: it passes, it names a contract, and it guards nothing. Sorted key +lists, lengths, and non-null checks fail the same way. Suspect those first, along with any +green never yet contradicted. + +## Evidence, not a count + +> Made the cache's read return nothing unconditionally. All 8 cases in the `redis` group +> failed, the 8 `memory` cases passed, nothing else moved. Reverted, suite green. + +The mutation, the cases that failed, the cases that did not, and the confirmed revert are +evidence. "50 tests pass" is not.