Compare commits

...
Author SHA1 Message Date
Matt PocockandClaude Opus 4.8 80e9dcc685 tdd: drop the refactor stage — red → green, not red → green → refactor
Refactoring belongs to the review stage, not the implementation loop.
Remove the refactor rule and the now-orphaned refactoring.md (its home is
the review skill), and point the loop's rule at the review skill so it
isn't re-added.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 09:48:57 +01:00
Matt PocockandClaude Opus 4.8 e81f97660a tdd: reshape into reference-only with pre-agreed seams
The red → green → refactor loop is anchored by leading words the model
already holds, so the step-by-step Workflow mostly restated the loop and
duplicated the horizontal-slicing anti-pattern. Drop the Workflow and
per-cycle checklist; fold their durable idea (vertical slices / tracer
bullets) into Anti-patterns and a short Rules-of-the-loop list.

Introduce **seam** as the leading word for where tests go, collapsing the
old Philosophy "public interfaces" prose and the Planning "confirm
interface / behaviors" handshake into one rule: test only at pre-agreed
seams, confirmed with the user before any test is written.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-30 09:43:49 +01:00
Matt PocockandClaude Opus 4.8 43ea0884b0 tdd: add tautological-test anti-pattern
Tests whose assertion is recomputed the way the code computes it pass by
construction and give zero confidence. Add it as a peer of the existing
implementation-coupling anti-pattern: a Philosophy principle, a per-cycle
checklist gate, and a BAD/GOOD example pair in tests.md. Includes a patch
changeset.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-29 21:22:50 +01:00
Matt PocockandClaude Opus 4.8 a116824938 decision-mapping: key tickets by dash-case slugs, not numeric ids
Replace numeric ticket ids (#1, #2…) with short, token-efficient
dash-case slugs (e.g. relational-db) that read as mini-titles. Update
the Structure example, the next-ticket selection rule (lowest-numbered →
first in document order), the optional-arg wording, and the Handoff
copy-paste examples accordingly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
2026-06-29 16:50:46 +01:00
6 changed files with 60 additions and 108 deletions
+7
View File
@@ -0,0 +1,7 @@
---
"mattpocock-skills": patch
---
Reshape the `tdd` skill into reference-only. The red → green → refactor loop is anchored by leading words the model already holds, so the step-by-step Workflow was largely restating the loop and duplicating the horizontal-slicing anti-pattern. Dropped the Workflow and per-cycle checklist; folded their one durable idea — vertical slices / tracer bullets — into the Anti-patterns section and a short Rules-of-the-loop list. Introduced **seam** as the leading word for where tests go, collapsing the old Philosophy "public interfaces" prose and the Planning "confirm interface / behaviors" handshake into one rule: test only at pre-agreed seams, confirmed with the user before any test is written.
Also dropped the refactor stage — TDD is now red → green, not red → green → refactor. Refactoring belongs to the review stage, not the implementation loop, so the refactor rule and `refactoring.md` were removed (its home is the `review` skill).
+5
View File
@@ -0,0 +1,5 @@
---
"mattpocock-skills": patch
---
Add the **tautological test** anti-pattern to the `tdd` skill. Tests whose assertion is recomputed the way the code computes it pass by construction and give zero confidence — distinct from the implementation-coupling anti-pattern already covered. Added as a peer at the same three sites: a Philosophy principle (expected values must come from an independent source of truth), a per-cycle checklist gate, and a BAD/GOOD example pair in `tests.md`.
+16 -88
View File
@@ -5,104 +5,32 @@ description: Test-driven development. Use when the user wants to build features
# Test-Driven Development # Test-Driven Development
## Philosophy TDD is the red → green loop. This skill is the reference that makes that loop produce tests worth keeping: what a good test is, where tests go, the anti-patterns, and the rules of the loop. Every section applies on every cycle — consult them before and during the loop, not after.
**Core principle**: Tests should verify behavior through public interfaces, not implementation details. Code can change entirely; tests shouldn't. When exploring the codebase, read `CONTEXT.md` (if it exists) so test names and interface vocabulary match the project's domain language, and respect ADRs in the area you're touching.
**Good tests** are integration-style: they exercise real code paths through public APIs. They describe _what_ the system does, not _how_ it does it. A good test reads like a specification - "user can checkout with valid cart" tells you exactly what capability exists. These tests survive refactors because they don't care about internal structure. ## What a good test is
**Bad tests** are coupled to implementation. They mock internal collaborators, test private methods, or verify through external means (like querying a database directly instead of using the interface). The warning sign: your test breaks when you refactor, but behavior hasn't changed. If you rename an internal function and tests fail, those tests were testing implementation, not behavior. Tests verify behavior through public interfaces, not implementation details. Code can change entirely; tests shouldn't. A good test reads like a specification — "user can checkout with valid cart" tells you exactly what capability exists — and survives refactors because it doesn't care about internal structure.
See [tests.md](tests.md) for examples and [mocking.md](mocking.md) for mocking guidelines. See [tests.md](tests.md) for examples and [mocking.md](mocking.md) for mocking guidelines.
## Anti-Pattern: Horizontal Slices ## Seams — where tests go
**DO NOT write all tests first, then all implementation.** This is "horizontal slicing" - treating RED as "write all tests" and GREEN as "write all code." A **seam** is the public boundary you test at: the interface where you observe behavior without reaching inside. Tests live at seams, never against internals.
This produces **crap tests**: **Test only at pre-agreed seams.** Before writing any test, write down the seams under test and confirm them with the user. No test is written at an unconfirmed seam. You can't test everything — agreeing the seams up front is how testing effort lands on the critical paths and complex logic instead of every edge case.
- Tests written in bulk test _imagined_ behavior, not _actual_ behavior Ask: "What's the public interface, and which seams should we test?"
- You end up testing the _shape_ of things (data structures, function signatures) rather than user-facing behavior
- Tests become insensitive to real changes - they pass when behavior breaks, fail when behavior is fine
- You outrun your headlights, committing to test structure before understanding the implementation
**Correct approach**: Vertical slices via tracer bullets. One test → one implementation → repeat. Each test responds to what you learned from the previous cycle. Because you just wrote the code, you know exactly what behavior matters and how to verify it. ## Anti-patterns
``` - **Implementation-coupled** — mocks internal collaborators, tests private methods, or verifies through a side channel (querying the database instead of using the interface). The tell: the test breaks when you refactor but behavior hasn't changed.
WRONG (horizontal): - **Tautological** — the assertion recomputes the expected value the way the code does (`expect(add(a, b)).toBe(a + b)`, a snapshot derived by hand the same way, a constant asserted equal to itself), so it passes by construction and can never disagree with the code. Expected values must come from an independent source of truth — a known-good literal, a worked example, the spec.
RED: test1, test2, test3, test4, test5 - **Horizontal slicing** — writing all tests first, then all implementation. Bulk tests verify _imagined_ behavior: you test the _shape_ of things rather than user-facing behavior, the tests go insensitive to real changes, and you commit to test structure before understanding the implementation. Work in **vertical slices** instead — one test → one implementation → repeat, each test a **tracer bullet** that responds to what the last cycle taught you.
GREEN: impl1, impl2, impl3, impl4, impl5
RIGHT (vertical): ## Rules of the loop
RED→GREEN: test1→impl1
RED→GREEN: test2→impl2
RED→GREEN: test3→impl3
...
```
## Workflow - **Red before green.** Write the failing test first, then only enough code to pass it. Don't anticipate future tests or add speculative features.
- **One slice at a time.** One seam, one test, one minimal implementation per cycle.
### 1. Planning - **Refactoring is not part of the loop.** It belongs to the review stage (see the `review` skill), not the red → green implementation cycle.
When exploring the codebase, read `CONTEXT.md` (if it exists) so that test names and interface vocabulary match the project's domain language, and respect ADRs in the area you're touching.
Before writing any code:
- [ ] Confirm with user what interface changes are needed
- [ ] Confirm with user which behaviors to test (prioritize)
- [ ] Identify opportunities for deep modules (small interface, deep implementation) — run the `/codebase-design` skill for the vocabulary and the testability checks
- [ ] List the behaviors to test (not implementation steps)
- [ ] Get user approval on the plan
Ask: "What should the public interface look like? Which behaviors are most important to test?"
**You can't test everything.** Confirm with the user exactly which behaviors matter most. Focus testing effort on critical paths and complex logic, not every possible edge case.
### 2. Tracer Bullet
Write ONE test that confirms ONE thing about the system:
```
RED: Write test for first behavior → test fails
GREEN: Write minimal code to pass → test passes
```
This is your tracer bullet - proves the path works end-to-end.
### 3. Incremental Loop
For each remaining behavior:
```
RED: Write next test → fails
GREEN: Minimal code to pass → passes
```
Rules:
- One test at a time
- Only enough code to pass current test
- Don't anticipate future tests
- Keep tests focused on observable behavior
### 4. Refactor
After all tests pass, look for [refactor candidates](refactoring.md):
- [ ] Extract duplication
- [ ] Deepen modules (move complexity behind simple interfaces)
- [ ] Apply SOLID principles where natural
- [ ] Consider what new code reveals about existing code
- [ ] Run tests after each refactor step
**Never refactor while RED.** Get to GREEN first.
## Checklist Per Cycle
```
[ ] Test describes behavior, not implementation
[ ] Test uses public interface only
[ ] Test would survive internal refactor
[ ] Code is minimal for this test
[ ] No speculative features added
```
-10
View File
@@ -1,10 +0,0 @@
# Refactor Candidates
After TDD cycle, look for:
- **Duplication** → Extract function/class
- **Long methods** → Break into private helpers (keep tests on public interface)
- **Shallow modules** → Combine or deepen
- **Feature envy** → Move logic to where data lives
- **Primitive obsession** → Introduce value objects
- **Existing code** the new code reveals as problematic
+16
View File
@@ -59,3 +59,19 @@ test("createUser makes user retrievable", async () => {
expect(retrieved.name).toBe("Alice"); expect(retrieved.name).toBe("Alice");
}); });
``` ```
**Tautological tests**: Expected value restates the implementation, so the test passes by construction.
```typescript
// BAD: Expected value is recomputed the way the code computes it
test("calculateTotal sums line items", () => {
const items = [{ price: 10 }, { price: 5 }];
const expected = items.reduce((sum, i) => sum + i.price, 0);
expect(calculateTotal(items)).toBe(expected);
});
// GOOD: Expected value is an independent, known literal
test("calculateTotal sums line items", () => {
expect(calculateTotal([{ price: 10 }, { price: 5 }])).toBe(15);
});
```
+16 -10
View File
@@ -14,12 +14,14 @@ Assets created during tickets should be linked to from the map, not duplicated w
### Structure ### Structure
Numbered entries ("tickets"), each its own section keyed by its number: Entries ("tickets"), each its own section keyed by a short dash-case slug that
reads as a mini-title (e.g. `relational-db`, `auth-strategy`, `cache-layer`) —
terse enough to stay token-efficient, and unique within the map.
```markdown ```markdown
## #1: Relational Or Non-Relational Database? ## relational-db: Relational Or Non-Relational Database?
Blocked by: #<ticket-number>, #<ticket-number> Blocked by: <slug>, <slug>
Status: open | in-progress | resolved Status: open | in-progress | resolved
Type: Research | Prototype | Grilling Type: Research | Prototype | Grilling
@@ -32,7 +34,11 @@ Type: Research | Prototype | Grilling
<answer-here> <answer-here>
``` ```
A ticket is **unblocked** when every ticket in its `Blocked by` list is `resolved`. A session **claims** its ticket by setting `Status: in-progress` and saving the map before any work, so concurrent sessions skip it. The slug is the canonical id, used in every `Blocked by` edge and prose
reference; the title after the colon is optional. A ticket
is **unblocked** when every ticket in its `Blocked by` list is `resolved`. A
session **claims** its ticket by setting `Status: in-progress` and saving the map
before any work, so concurrent sessions skip it.
Each ticket must be sized to one 100K token agent session. Each ticket must be sized to one 100K token agent session.
@@ -62,10 +68,10 @@ User invokes with a loose idea.
### Work through the map ### Work through the map
User invokes with a path to an existing map. A ticket number is **optional** — without one, you pick the next decision, not the user. User invokes with a path to an existing map. A ticket slug is **optional** — without one, you pick the next decision, not the user.
1. Load the **whole map** as context. 1. Load the **whole map** as context.
2. Choose the ticket. If the user named one, use it. Otherwise pick the lowest-numbered `open` ticket that is [unblocked](#structure). [Claim it](#structure): set `Status: in-progress` and save before any work. 2. Choose the ticket. If the user named one, use it. Otherwise pick the first `open` ticket in document order that is [unblocked](#structure). [Claim it](#structure): set `Status: in-progress` and save before any work.
3. Resolve it, invoking skills as needed. If in doubt, use `/grilling` and `/domain-modeling`. 3. Resolve it, invoking skills as needed. If in doubt, use `/grilling` and `/domain-modeling`.
4. Record the answer in the ticket's body and set `Status: resolved`. 4. Record the answer in the ticket's body and set `Status: resolved`.
5. Add newly-discovered tickets with correct `Blocked by` edges. If the decisions made invalidate other parts of the map, update or delete those nodes. 5. Add newly-discovered tickets with correct `Blocked by` edges. If the decisions made invalidate other parts of the map, update or delete those nodes.
@@ -79,7 +85,7 @@ End every session by clearing the context and opening one or more fresh sessions
**Open tickets remain.** List the currently-unblocked tickets, then give two copy-paste options: a bare command for one session (you pick the next ticket), and one pinned command per unblocked ticket for running them in parallel. Paste one line per fresh window — opening one, some, or all of them. **Open tickets remain.** List the currently-unblocked tickets, then give two copy-paste options: a bare command for one session (you pick the next ticket), and one pinned command per unblocked ticket for running them in parallel. Paste one line per fresh window — opening one, some, or all of them.
> **Next steps** — 3 tickets unblocked: #4, #5, #6. > **Next steps** — 3 tickets unblocked: `auth-strategy`, `cache-layer`, `rate-limits`.
> Clear the context, then open fresh sessions. > Clear the context, then open fresh sessions.
> >
> **One session** — resolves the next unblocked ticket: > **One session** — resolves the next unblocked ticket:
@@ -89,9 +95,9 @@ End every session by clearing the context and opening one or more fresh sessions
> >
> **Parallel** — paste one line per window, up to all 3: > **Parallel** — paste one line per window, up to all 3:
> ``` > ```
> Invoke /decision-mapping with the map at <path>, ticket #4. > Invoke /decision-mapping with the map at <path>, ticket auth-strategy.
> Invoke /decision-mapping with the map at <path>, ticket #5. > Invoke /decision-mapping with the map at <path>, ticket cache-layer.
> Invoke /decision-mapping with the map at <path>, ticket #6. > Invoke /decision-mapping with the map at <path>, ticket rate-limits.
> ``` > ```
**No open tickets remain.** The fog is pushed back far enough that the path to the finish line is clear — the map is done. (The initial grilling may also surface no fog at all, in which case there was never a map to build.) Recommend implementing directly, or using `/to-prd` to schedule a multi-session implementation. **No open tickets remain.** The fog is pushed back far enough that the path to the finish line is clear — the map is done. (The initial grilling may also surface no fog at all, in which case there was never a map to build.) Recommend implementing directly, or using `/to-prd` to schedule a multi-session implementation.