Files
claude-skills/.wiki/concepts/tdd-criteria-design.md

178 lines
15 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
---
date: '2026-05-07'
source: .meeting-room/.archive/2026-05-07-tdd-criteria.md
status: promoted
type: design
title: tdd-criteria-design
amended: "2026-05-07: added test-immutability defence (Anti-loophole rule 4) after user noted symmetric vandalism risk on tests"
ingested_at: '2026-05-07T04:01:23.616Z'
ingested_by: OpeItcLoc03@DESKTOP-NSEF0UK
source_project: .meeting-room
---
# TDD-criteria — design rationale
> Design document for the `tdd-criteria` skill. Captures **why** the rule is shaped the way it is — the SKILL.md itself is the runtime artefact (procedure + triggers); this page is the argument.
## Problem statement
User is ready to adopt TDD as default across projects but wants **bright-line carve-outs** to prevent TDD from degenerating into a tax that every task pays. The risk is the «exploratory loophole»: if the rule is «without TDD is OK for exploratory work», every task ex post facto becomes «exploratory».
Bright-line means: at decision time, the agent (human or LLM) answers each criterion with **yes/no based on observable property** — no «мне кажется», no judgment calls. If a criterion needs intuition, it's not a criterion, it's a loophole.
## The argument behind TDD-default
The four classical arguments — bug fixes are easier with red-tests; pure logic is cheap to test; third-party contracts silently break; security/money has asymmetric blast radius — are about **correctness** of behaviour. They tell you when to defend against *wrong* behaviour.
The **strongest** argument, and the leading rationale in this design, is different: TDD-default is the only mechanism that protects behaviour **from being silently deleted**.
Mechanism:
- **Without a test**, the contract for a piece of code is «it exists in the repo». That's an artefact, not an invariant. An agent (LLM coder, new hire, future self) sees «messy code» → deletes it → commits «cleaner now» → reports success. That the code implemented real behaviour is **recorded nowhere except the code**, which is now gone. Recovery is `git log` archaeology, *after* somebody notices the regression — days or weeks later.
- **With a test**, the contract is «X(Y)=Z». That's an invariant. Delete X → test fails → pipeline red → success can't be reported. The test is **the advocate of the behaviour at the moment when the behaviour itself no longer exists**.
In a workflow that includes LLM coders (Claude, ChatGPT, future unknowns) and rotating contractors, this isn't theoretical. The user has personally observed agents deleting working code and reporting «I cleaned it up» — recovery required manual git archaeology. Tests would have made that impossible.
This reframes everything below:
### The contract is only as strong as the contract itself
But there's a **second-order vandalism mode** that the bare contract argument doesn't cover: the agent doesn't delete the code, it rewrites the **test**. Test fails → agent changes the expected value, adds `.skip`, or deletes the test → test now passes → success reported.
If the contract artefact (the test) is rewritable by the same agent that's failing to satisfy it, the invariant collapses back into an artefact. The defence requires **two layers**:
1. **Code is defended by tests.** Ironclad rules 1-4 below.
2. **Tests are defended by process discipline.** Anti-loophole rule 4 below — append-only by default, modifications require literal-`was/is` marker in commit subject, test changes are separate commits from impl changes.
Both layers are needed. Either alone leaves a path-of-least-resistance route to «success».
- **Ironclad** rules (TDD obligatory) — zones where recovery cost > defending cost. Signal mandatory.
- **Permissive** carve-outs — **zones of accepted risk for agentic vandalism**, not «zones where TDD doesn't apply». You explicitly accept that an agent can delete-and-claim-success here, because recovery is cheap (eyeball the next render; throwaway by contract; one-shot already ran; wrapper trivial to reconstruct).
## Decision algorithm
Walk through 8 questions top-to-bottom. First «yes» determines mode. All «no» → TDD by default.
```
1. Это исправление бага? → TDD (red-test первым)
2. Это код, потребляющий внешний контракт → TDD (contract-test)
(SDK, REST API, чужая schema)?
3. Это security / auth / money / identifiers? → TDD
4. Это pure logic — функция (input → output) → TDD
без I/O, без global state, bounded inputs?
5. Это visual / config — CSS, layout, design → SKIP, [skip-tdd: visual]
tokens, .env.example, prompt-тексты,
wiki, README?
6. Это явно объявленный spike (зафиксировано → SKIP, [skip-tdd: spike]
в commit/PR/task subject «POC, выкину»)? + spike-survivor task если выживет
7. Это one-shot скрипт — миграция, ETL backfill, → SKIP, [skip-tdd: oneshot]
ad-hoc cleanup, runs once?
8. Это транзитный wrapper ≤10 строк → SKIP, [skip-tdd: wrapper]
без branching (re-export, glue)?
(default) → TDD
```
## Ironclad — why TDD is cheaper than skipping
| # | Rule | Checkable property | Why TDD here |
|---|---|---|---|
| 1 | Bug fix | «есть issue / failure log / repro?» | Bug already reproduced — red-test is just codifying the repro. Marginal cost: 5 min. Marginal benefit: regression test forever. Without it, «fixed» = checked once, regresses on next refactor. |
| 2 | Pure logic, bounded inputs | «функция читает диск/сеть/БД/mutates global state?» — no | Cheapest TDD surface: no fixtures, no mocks, no setup. Test = input/output pair. Marginal cost ≈ 0. Skip is gratuitous. |
| 3 | Third-party contract | «вызывает чужой SDK / парсит чужую schema?» | SDK bumps change signatures silently. Contract-test pins «known input → known shape». Catches the break at first `npm install` instead of «endpoint висит несколько дней». Direct counter-case observed: `modules-db/fill-fields-ai-sdk-migration` post AI SDK v5→v6. |
| 4 | Security / auth / money / IDs | «касается токенов/паролей/валюты/идентификаторов/прав?» | Asymmetric blast radius: false positive (slow code, slow test) ≪ false negative (account takeover, data corruption, money loss). TDD = insurance. |
**Plus the meta-rationale**: in all four, recovery cost from silent deletion is high. The test is the only artefact that makes deletion visible.
## Permissive — accepted-risk zones
Not «TDD doesn't apply». **«You accept that an agent can vandalise this without immediate signal, because recovery is cheap.»** Entry into the zone is explicit, marked in the commit subject.
| # | Category | Trigger | Marker | Recovery cost (= reason TDD doesn't pay) |
|---|---|---|---|---|
| 5 | Visual / config | CSS, layout, design tokens, `.env.example`, prompts, wiki, README | `[skip-tdd: visual]` | Eyeball on next render. Visual regression infra exists (Playwright screenshots, Percy) but heavyweight for most projects. |
| 6 | Spike | Explicit POC «throwaway» in commit/PR/task subject | `[skip-tdd: spike]` | Throwaway by contract — deletion isn't a problem. **Survivor rule**: if spike code reaches master, the same merge-commit creates `[backfill-tests-<slug>]` task. Otherwise this category becomes the loophole. |
| 7 | One-shot | Migrations, ETL backfill, ad-hoc cleanup; runs once | `[skip-tdd: oneshot]` | Test never re-executes — cost not recovered. After run, deletion is irrelevant. |
| 8 | Wrapper | ≤10 lines, no branching (re-export, glue) | `[skip-tdd: wrapper]` | Test on `function foo(x) { return bar(x) }` re-states `bar`. Reconstruct cost ≈ delete cost. The defence is on `bar`, not `foo`. |
## Anti-loophole
1. **Skip без категории не существует.** One of four explicit categories — not «other reasons». No marker = violation, regardless of perceived justification.
2. **Spike survivor rule.** If spike code is merged to master → same merge-commit creates `[backfill-tests-<slug>]` task in `.tasks/STATUS.md`. Otherwise «spike» becomes «skipped tests forever».
3. **Friction is the point.** `[skip-tdd: visual]` 50 раз подряд при `web-design-system` итерации раздражает — это и есть замысел. Friction = fence, not bug. If it becomes unbearable, re-evaluate after ≥2 weeks of usage, not before.
4. **Tests are append-only by default** (added 2026-05-07). New tests: free. **Modifying** an existing assertion, **deleting** a test, or **disabling** it (`it.skip`, `xit`, `@pytest.mark.skip`, `@Disabled`, etc.) requires both:
**a)** A marker in commit subject:
```
[test-modify: <test-name>: was <X>; is <Y>; reason: <Z>]
```
Where `<X>` and `<Y>` are the **literal assertion expressions** before and after, not paraphrased. Example:
```
[test-modify: validates email format: was expect(isValid("a@b")).toBe(true); is expect(isValid("a@b.com")).toBe(true); reason: tightened spec to require TLD]
```
**b)** Test changes go in a **separate commit** from any impl changes. A single commit must not modify both `*.test.*` and `src/*` files (or their project-equivalents). This forces an audit-able split — `git log --grep '\[test-modify'` shows every test rewrite cleanly.
**Why literal `was/is`, not free-form reason:** an agent forced to write the literal assertion publishes exactly what they're rewriting. If `42` was the correct expectation and they changed it to `43` to make a buggy fix pass, the literal `was 42; is 43` line in `git log` identifies the culprit. A reason like «updated to match new behaviour» hides everything — agents will use it whenever it is allowed.
**Bright-line check** (for an optional pre-commit hook, see follow-up task `tdd-criteria-precommit-hook`):
- `git diff --cached` includes a removed `expect(...)` / `assert(...)` / `assertThat(...)` line, OR
- changes the arguments of an existing assertion call, OR
- adds `.skip`, `xit`, `@skip`, `@Disabled`, etc. annotation, OR
- deletes a test file or `it(...)` / `test(...)` / `def test_*` definition
AND the commit subject does not contain `[test-modify: ...]` matching the format above → block.
AND `git diff --cached --name-only` includes both test-pattern and impl-pattern files → block (require split).
5. **Don't apply rule 4 retroactively** to tests written before the rule was adopted. The rule applies to test changes made after the project's CLAUDE.md picks up `follow tdd-criteria`. Existing test bodies aren't grandfathered into requiring `was/is` for a one-time rewrite.
## What's excluded as not bright-line
These tempting formulations were rejected:
- ~~«Exploratory»~~ — every task becomes exploratory in retrospect.
- ~~«When time is short»~~ — time is never abundant.
- ~~«When the logic is obvious»~~ — it seems obvious until the first bug.
- ~~«When the code is temporary»~~ — there's no such thing; temporary code becomes permanent.
- ~~«Tests should not be modified casually»~~ — paraphrasable, agents will modify casually and call it «refactor». Replaced by Anti-loophole rule 4 with literal-evidence requirement.
All require judgment at decision time → automatically become loopholes. The four Permissive categories are bound to **observable properties** (file extension, marker in commit subject, directory, line count) — not mood.
## Composite tasks
A task that doesn't fit any single category cleanly is a **composite task** — break it down by artefact type.
Example: «add a user-profile-settings page»:
- HTML/CSS layout → `[skip-tdd: visual]`
- Validation form (email format, password strength) → Ironclad-4 (security) → TDD
- API call wrapper for save → Ironclad-3 (third-party contract if PUT to external endpoint) → TDD
- Update Pinia store reducer → Ironclad-2 (pure logic if bounded reducer) → TDD
- Hook `useProfileForm` composing the above → wrapper if ≤10 lines glue, else Ironclad-2
One «task» yields 4-5 commits with different modes. **The criterion applies per artefact, not per task.** This is the point — no «overall this is exploratory».
## Project-level overrides
Project `CLAUDE.md` files may **extend** Permissive (e.g. `karu` is a pure-CSS project — wider visual carve-out is appropriate) or **extend** Ironclad with project-specific categories (e.g. `books` could add «scheduler tasks touching Mongo state» as Ironclad-5). Project files **may not narrow Ironclad** — the global floor stands.
## Trade-offs (honest)
- **Supportive evidence is n=1.** The observable contrast is `books` (TDD applied selectively, features ship) vs `modules-db`/`pilonuxt` (TDD not applied, projects buksuyut). `books` could be more productive for reasons unrelated to TDD (codebase maturity, author experience, task type). The direction-of-effect agrees with first principles, so inversion is unlikely, but the magnitude is uncertain.
- **Friction in UI iterations.** `[skip-tdd: visual]` repeated 50× during a design-system rework is annoying. Intentional. Don't relax before ≥2 weeks of usage.
- **The literal-`was/is` requirement is verbose** for a renamed test or trivial typo fix. The verbosity is the point — an agent that genuinely fixed a typo writes the same assertion twice with one character changed; an agent that rewrote a failing test writes obviously different assertions. Reading `git log --grep '\[test-modify'` shows the difference at a glance.
- **«Don't codify at all» was rejected.** Precisely «not codified» is what produced the situation in `books` where, after the author left, his TDD criterion can't be reconstructed — it lived only in his head and his commits. A departed contributor is an argument *for* codification, not *against*.
- **The anti-vandalism argument is uncomfortable.** It says: «I don't trust agents not to delete my code». That's the position. If it changes, this rationale changes. Until then, this is what's load-bearing.
## Cross-agent applicability
Pure policy — no Claude-specific tool references (no `Read`/`Edit`/`Bash`/`Glob` calls in the SKILL.md body). Hermes mapping: `mode: auto`, `category: software-development`, no replace-rules. Future agents (Gemini, Copilot, in-house) inherit via their respective rollout adaptors.
## See also
- Skill artefact: `claude-skills/skills/tdd-criteria/SKILL.md` (runtime: triggers + procedure).
- Hermes mapping: `claude-skills/hermes/mapping.yaml` entry `tdd-criteria`.
- Original brainstorm: `.meeting-room/.archive/2026-05-07-tdd-criteria.md` (full discussion arc with 6 rounds + observable evidence from `mcp__projects-meta__tasks_get` on `books`/`modules-db`/`pilonuxt`).
- Precedent for «move a rule from `~/.claude/CLAUDE.md` into a skill»: `claude-skills/skills/recommend-dont-menu/SKILL.md` («Why this exists» section).