test-review

name	test_review
description	Review the tests of a commit set (or a named code area) — not the code — asking whether they actually pin the contract the code declares. Use when the user asks to "review the tests", "check test coverage" of a change, asks "would a test catch this", wants to know if a contract is pinned, or after landing a change whose tests were written alongside it.

Test review — do the tests pin the contract?

A procedure for reviewing the tests of a change, not the code. The unit of review is a commit set — one commit or a contiguous range — or an explicitly named code area: the promises the set's code changes introduce, and the tests the set added or changed to pin them, both read at the tip commit. When invoked standalone with no scope handed in, default to the latest commit (widened to the whole landing when the tip commits are one change split into pieces) and state the resolved SHAs in the report.

The question is never "is there a test near this code" or "is coverage high" — executing a line proves nothing about detecting a bug in it. The question is: for each promise the code makes, which test fails when that promise breaks? A promise with no failing-test is an unpinned contract, and unpinned contracts drift silently.

Ground rules for this repo: tests/ is the only live suite (uv run pytest tests/ -q); contracts live in docstrings and context/specs/; the two storage backends (src/vfs/storage/backends/memory.py and src/vfs/storage/backends/database/) must behave identically, with shared behavior pinned in tests/storage_conformance.py and only backend-specific facts in the per-backend test files.

Process

1. Trace contracts to tests

List every behavioral clause the changed code declares: docstring promises, error classifications returned, ordering guarantees, atomicity claims, boundary values (budgets, caps, chunk sizes), and spec clauses the change claims to implement. For each clause, find the specific test that would fail if the clause broke — name it, don't gesture at a file. Build a table:

| Clause (code file:line) | Pinning test (test file:line) or UNPINNED |

A clause is pinned only if some assertion distinguishes the promised behavior from a plausible wrong one. "A test calls this function" is not pinning.

2. Think in mutations — and execute the ones that matter

For each key changed line, ask: if I flipped this condition, off-by- one'd this bound, swapped < for <=, reordered these two calls, or deleted this call entirely — which test fails? Name the test. If you cannot name one, that surviving mutation is a candidate finding. Prioritize mutations on branches (error classification, capability gates, chunk boundaries) and on calls whose effect is only visible later (version stamps, cleanup, ordering).

Then execute the strongest candidates — a reasoned "no test would catch this" is a hypothesis; a mutant that runs green is proof. Do not run a mutation tool; apply each candidate by hand under the mutation protocol. The live repo stays read-only — other agents run against it concurrently, so even a briefly-mutated src/ poisons their reads and test runs. All mutation work happens in an isolated tree under the session scratchpad:

1. Once per review, make the isolated tree: git -C <repo> worktree add <scratchpad>/mutant-tree <tip-sha> (detached at the reviewed tip; if the review covers uncommitted state, copy those dirty files in on top). Then prime it once: uv sync inside the worktree (wheel and deps come from uv's cache; slow only on the first prime).
2. Apply the one-line mutation in the worktree; run the scoped tests from there (uv run --project <worktree> pytest <worktree>/tests/... — the test file or directory owning that surface, never the full suite).
3. Between mutants, restore the worktree file from its own committed state (git -C <worktree> checkout -- <file> — safe there and only there: the worktree holds nothing uncommitted by design) and confirm git -C <worktree> status --porcelain is clean.
4. When done, git -C <repo> worktree remove <scratchpad>/mutant-tree --force. Never point a mutation, a restore, or a test run at the live repo path.

An executed mutant that survives its scope is a finding with a repro (report the exact mutation and the green run); one that dies names its killer and goes in the coverage ledger as verified-pinned. When a landing later pins a proven survivor, the mutation belongs in context/standards/mutant-ledger.md as a new row.

3. Replay the mutant ledger

context/standards/mutant-ledger.md holds every mutation a past campaign proved and pinned. Replay the rows whose target files intersect the review scope — and when the scope is broad or time allows, the whole ledger (~2 minutes) — under the same mutation protocol (in the isolated worktree, never the live repo), following the ledger's own replay rules: rows record intent plus a best-known anchor, so a moved anchor means re-deriving the mutation from intent, not declaring failure; the assertion is ≥1 failure in the row's scope, never a named test (recorded killers are advisory diagnosis — mapping drift is normal and healthy). Report per row: killed (note it in coverage), survived (a pin regressed — a high-severity finding with the mutation as repro), or stale (the concept the row mutates no longer exists — propose retiring the row, with reasoning). Never skip a row silently.

4. Audit error paths

Untested error handling is where catastrophic failures live. Every classified error the changed code can return (each VFSErrorKind it can mint, each raise, each per-row batch classification) must be produced by at least one test — a test that constructs the offending input and asserts the exact classification, not just "an error happened". List each error the code can emit and the test that triggers it; an emitter with no trigger is a finding. Check batch paths specially: an error tested only for a single-item call may be classified differently (or lost) in a 10,000-row batch.

4. Check backend parity

Behavior promised by both backends but tested against one is a parity gap. For each behavioral clause from step 1, decide where its test lives:

• Shared semantics belong in storage_conformance.py, where both backends run it. A shared behavior tested only in test_backends_memory.py or test_backends_database.py is a finding — the other backend can break it unnoticed.
• Per-engine conditionals inside the conformance suite are out of contract — an if backend is ... in a shared test hides a real divergence instead of pinning it.
• Backend-specific files should hold only what is genuinely specific (identity, capabilities, provisioning, dialect policy).

The second implementation is the strongest oracle available — use it.

5. Scan for test smells

Weak tests pass review while pinning nothing. Flag:

• Assertions that cannot fail — asserting a value the test itself just set, tautologies, assert result is not None on a constructor.
• Over-broad assertions — checking success is True while leaving observations, error kinds, versions, or ordering unchecked; a test's power lives in its assertions, not its coverage.
• Conditional test logic — if/for guarding assertions, so a path through the test can pass without asserting anything.
• Assertion roulette — long unexplained assertion runs where a failure won't say which promise broke.
• Fragile coupling — assertions on incidental facts (dict/list order not promised, exact message text where only the kind is the contract, timestamps), which fail on lawful change and train people to ignore them.
• Missed properties — a pile of near-identical examples where one invariant (round-trip, idempotence, order-independence) would pin the whole family; suggest the property.

6. Evidence standard

Every finding names the untested clause or the surviving mutation, with file:line for the code and for the test that should have caught it (or the file where the missing test belongs). "Coverage seems thin" is not a finding. Verify before reporting: read the test you claim is missing a check — it may pin the clause indirectly (via the conformance suite, a fixture, or a law test). A finding that a test would not fail must survive you actually tracing the assertion.

7. Deliver findings

Report a severity-ordered list. Severity: high — unpinned docstring/spec clause, unproduced error classification, or parity gap on shared semantics; medium — surviving mutation on a key line, over-broad assertion on a changed path; low — smells and fragile couplings. For each finding, include the one-sentence test that should exist ("a test that writes at the chunk boundary +1 and asserts both chunks land with the same version"), and say which file it belongs in. Close with the clauses that are well pinned, so the strong parts of the suite are visible too.