aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorCraig Jennings <c@cjennings.net>2026-07-27 14:13:32 -0500
committerCraig Jennings <c@cjennings.net>2026-07-27 14:13:32 -0500
commit2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5 (patch)
treec89e06112df046d72905531123bfbfc1bce339a1
parent931f364864441676eb250cecd074c4876012a1cd (diff)
downloadrulesets-2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5.tar.gz
rulesets-2f45b6e0010ce375e2c52c91a6537c6d6e8bc0a5.zip
refactor(rules): split testing.md, fix the approval gate, require first person
Three changes to the same layer. I split testing.md the way I split commits.md, by what has to be resident rather than by size. What stays is the standing directive: TDD is the default, write the failing test first, and every unit needs Normal, Boundary, and Error cases. That has to fire before any code is written, which is exactly when no skill has been summoned, so it can't ride a trigger. Everything else moved to the testing-standards skill: characterization recipes, the per-category detail, property-based and mutation testing, the pyramid, integration rules, naming, the test-quality and mocking rules, coverage targets, the spike exception, and the anti-patterns. 2,824 words down to 347. I fixed the approval gate in the publish flow. It decided whether to ask for approval by checking whether .ai/ is tracked, using that as a proxy for "team repo." The proxy was wrong in the direction that matters: rulesets, home, and work all track .ai/ while all three are private single-user repos, so the rule skipped the gate on the three projects I use most. It now checks whether any remote is on a host other than cjennings.net, which is the thing that actually decides whether someone else reads the log. Every current project resolves to gate-applies, which matches how the flow has actually been run. I also added a first-person directive to the always-loaded core. One already existed for commit bodies and PR prose, but it moved into the publish skill with everything else, and it never covered code comments at all. Now everything I author in or about the repo is first person, with one carve-out: a comment describing what the code does stays third person, because there the code is the actor and not me. Separately I split the publish skill internally. PR descriptions and the three review shapes moved to references/pull-requests.md, since a plain commit never needs them. Always-loaded rules are now about 28,900 tokens, down from 57,800 this morning. One risk on the record. testing.md's margin is thinner than commits.md's was. If testing-standards fails to trigger while I'm writing tests I lose the mocking-boundary rules, which is a quality regression rather than a permanent one, but it is a real bet where commits.md's was not. I also moved the TDD rationalization table rather than cutting it. The posts argue that kind of over-argument is counterproductive now, but removing your defense against me skipping TDD is your call, not mine.
-rw-r--r--.ai/workflows/code-quality.org2
-rw-r--r--claude-rules/commits.md23
-rw-r--r--claude-rules/testing.md381
-rw-r--r--claude-rules/todo-format.md3
-rw-r--r--claude-templates/.ai/workflows/code-quality.org2
-rw-r--r--publish/SKILL.md131
-rw-r--r--publish/references/pull-requests.md105
-rw-r--r--review-code/SKILL.md2
-rw-r--r--testing-standards/SKILL.md391
9 files changed, 573 insertions, 467 deletions
diff --git a/.ai/workflows/code-quality.org b/.ai/workflows/code-quality.org
index 0481166..3c4ed8f 100644
--- a/.ai/workflows/code-quality.org
+++ b/.ai/workflows/code-quality.org
@@ -12,7 +12,7 @@ workflow only sequences them and collects the residue.
*Behavior-preserving rests on a test net.* The passes below claim to preserve
behavior, but a refactor on untested code is a guess, not a preservation. Where
the scope has no tests, bring it under a characterization net first
-(Normal/Boundary/Error per unit, per =testing.md='s "Adding Tests to Existing
+(Normal/Boundary/Error per unit, per the =testing-standards= skill's "Adding Tests to Existing
Untested Code") — that net is what turns "behavior-preserving" from an assertion
into something the green suite actually verifies across each pass.
diff --git a/claude-rules/commits.md b/claude-rules/commits.md
index e3f75a8..51c452a 100644
--- a/claude-rules/commits.md
+++ b/claude-rules/commits.md
@@ -91,6 +91,29 @@ Edge case: when one of these files *is* the change (a commit in the rulesets rep
**Tooling-path enumeration is the same leak.** Citing a rule as authority isn't the only way the tooling layer leaks into history. A commit whose *content* must name these paths — a `.gitignore` adding `.claude/`, `CLAUDE.md`, `.ai/` — has unavoidable, correct file content, but its *message prose* must not enumerate them ("chore: ignore .claude tooling, CLAUDE.md, and session files"). On a public or shared-remote repo that enumeration exposes the tooling layer's structure in the log just as a citation would. Name the category instead: "chore: extend gitignore for local tooling and build artifacts". The same holds for any incidental mention, not only `.gitignore` commits. Two exemptions: a commit whose change *is* one of these files (the edge case above), and private single-user repos with no shared remote, where the history is the project and there's no third party to leak to.
+## Write in the first person
+
+Everything I author in or about this repo is first person: code comments,
+commit messages, PR descriptions, PR review comments, and any note that lands
+in the repo or its history. When I made a choice, say so as a choice — "I swept
+the copies rather than repairing them, because a drifted copy outranked the
+global rule" beats "the copies are swept rather than repaired."
+
+Third-person constructions like "This change introduces X" or "This PR restores
+Y" read as press-release self-narration. The commit is the change, so it does
+not need announcing.
+
+**The one carve-out: code is the actor when describing behavior.** A comment
+saying *what the code does* stays third person, because the subject genuinely
+is the code and not me — "the sweep only fires when the global rule exists",
+"the guard rejects a malformed payload". First person is for the decision
+behind it, third person for the behavior itself. Both often belong in the same
+comment: what it does, then why I chose it.
+
+This is the rule the publish flow already applied to commit bodies. It lives
+here because code comments get written constantly and the publish skill is not
+loaded then.
+
## The publish flow lives in the `publish` skill
Everything about *how* a commit, PR, or review comment gets written, reviewed,
diff --git a/claude-rules/testing.md b/claude-rules/testing.md
index 81bd391..dd15282 100644
--- a/claude-rules/testing.md
+++ b/claude-rules/testing.md
@@ -16,377 +16,30 @@ TDD is the default workflow for all code, including demos and prototypes. **Writ
Do not skip TDD for demo code. Demos build muscle memory — the habit carries into production.
-### Understand Before You Test
-Before writing tests, invest time in understanding the code:
+## Test Categories — required for all code
-1. **Explore the codebase** — Read the module under test, its callers, and its dependencies. Understand the data flow end to end.
-2. **Identify the root cause** — If fixing a bug, trace the problem to its origin. Don't test (or fix) surface symptoms when the real issue is deeper in the call chain.
-3. **Reason through edge cases** — Consider boundary conditions, error states, concurrent access, and interactions with adjacent modules. Your tests should cover what could actually go wrong, not just the obvious happy path.
+Every unit under test needs all three, not just the happy path:
-### Adding Tests to Existing Untested Code
+1. **Normal** — standard inputs, common workflows, typical volumes.
+2. **Boundary** — zero, one, max, empty vs null, single-element collections, unicode, very long input, timezone and date edges.
+3. **Error** — invalid input, type mismatches, network failure, missing parameters, permission denied, resource exhaustion, malformed data.
-When working in a codebase without tests:
+The negative and boundary cases are the ones that find bugs. A unit with only
+Normal coverage is not tested, it is demonstrated.
-1. Write a **characterization test** that captures current behavior before making changes
-2. Use the characterization test as a safety net while refactoring
-3. Then follow normal TDD for the new change
+## The rest of the standard lives in the `testing-standards` skill
-A characterization test asserts what the code *actually does* right now, not
-what it *should* do. Write it by running the code against a fixed input,
-reading the exact value or effect it currently produces, and asserting that
-value — Feathers' recipe is to assert something you know is wrong, run it, and
-paste the real value out of the failure. You don't need to know the correct
-answer to write one; you record the observed one. That's what makes it
-mechanical enough to bring a large untested surface under test without
-re-deriving each unit's spec.
+Characterization tests for untested code, the per-category detail, combinatorial
+and property-based and mutation testing, organization and the pyramid,
+integration-test rules, naming, the test-quality rules (independence,
+determinism, mocking boundaries, signs of overmocking), the
+refactor-when-tests-are-hard principle, coverage targets, the spike exception,
+and the anti-pattern list are all in the `testing-standards` skill. Load it when
+writing tests.
-**Characterize with the same Normal/Boundary/Error set as any unit** (the three
-categories below), not one happy-path capture per function. On a characterization
-test the negative and boundary cases are the ones that find bugs: untested legacy
-code is weakest exactly at the empty input, the malformed value, the missing
-upstream, and pinning what it *currently* does there writes the wrong behavior
-down in black and white, where it becomes a bug you can see and decide on. When a
-pinned case turns out to be a bug rather than behavior worth preserving, that one
-test graduates from "record current" to "assert correct" and you fix the code.
-The happy-path case is the regression net; the negative and boundary cases are
-the audit.
-
-Bugs that live *inside* a unit are caught by this three-category set; bugs in how
-units compose — ordering, shared state handed between them — are invisible to any
-per-unit test and need a functional/integration test over the composed path (see
-Integration Tests below and the pyramid).
-
-## Test Categories (Required for All Code)
-
-Every unit under test requires coverage across three categories:
-
-### 1. Normal Cases (Happy Path)
-- Standard inputs and expected use cases
-- Common workflows and default configurations
-- Typical data volumes
-
-### 2. Boundary Cases
-- Minimum/maximum values (0, 1, -1, MAX_INT)
-- Empty vs null vs undefined (language-appropriate)
-- Single-element collections
-- Unicode and internationalization (emoji, RTL text, combining characters)
-- Very long strings, deeply nested structures
-- Timezone boundaries (midnight, DST transitions)
-- Date edge cases (leap years, month boundaries)
-
-### 3. Error Cases
-- Invalid inputs and type mismatches
-- Network failures and timeouts
-- Missing required parameters
-- Permission denied scenarios
-- Resource exhaustion
-- Malformed data
-
-## Combinatorial Coverage
-
-For functions with 3+ parameters that each take multiple values (feature-flag
-combinations, config matrices, permission/role interactions, multi-field
-form validation, API parameter spaces), the exhaustive test count explodes
-(M^N) while 3-5 ad-hoc cases miss pair interactions. Use **pairwise /
-combinatorial testing** — generate a minimal matrix that hits every 2-way
-combination of parameter values. Empirically catches 60-90% of combinatorial
-bugs with 80-99% fewer tests.
-
-Invoke `/pairwise-tests` on the offending function; continue using `/add-tests`
-and the Normal/Boundary/Error discipline for the rest. The two approaches
-complement: pairwise covers parameter *interactions*; category discipline
-covers each parameter's individual edge space.
-
-Skip pairwise when: the function has 1-2 parameters (just write the cases),
-the context requires *provably* exhaustive coverage (regulated systems — document
-in an ADR), or the testing target is non-parametric (single happy path,
-performance regression, a specific error).
-
-## Escalation Beyond Category and Pairwise
-
-The Normal/Boundary/Error categories and the pairwise matrix are the default
-discipline. Two further techniques escalate beyond them — reach for them when
-the default leaves a gap, not on every unit.
-
-### Property-Based Testing
-
-When an invariant holds across a broad input domain — round-trips
-(`decode(encode(x)) == x`), idempotence (`f(f(x)) == f(x)`), ordering
-invariants (output is always sorted), or any "output always satisfies X" —
-generate inputs and assert the property instead of enumerating cases. The
-generator explores corners you wouldn't think to write by hand, and a
-failing case shrinks to a minimal reproducer. Use the standard tool for the
-language (Hypothesis for Python, fast-check for JS, proptest for Rust).
-State the property as the test name and let the framework supply the inputs.
-
-Reach for this when the behavior is a law over a domain rather than a fixed
-set of examples. Keep category-discipline cases for the specific edges that
-must always hold; the property test covers the space between them.
-
-### Mutation Testing
-
-When line coverage is high but you suspect the assertions are thin — tests
-that execute the code without checking its output, or that pass with a
-function body replaced by a stub — use mutation testing to measure whether
-the suite actually kills injected faults. The tool flips conditionals, swaps
-operators, and deletes statements, then reruns the suite; a surviving mutant
-is a fault the tests didn't catch. Use mutmut or cosmic-ray for Python,
-Stryker for JS. High line coverage with a low mutation score means weak
-assertions, not a tested codebase.
-
-Reach for this on critical logic where coverage looks reassuring but you
-want evidence the tests would fail on a regression. It's a diagnostic, not a
-gate on every change — mutation runs are slow.
-
-## Test Organization
-
-Typical layout:
-
-```
-tests/
- unit/ # One test file per source file
- integration/ # Multi-component workflows
- e2e/ # Full system tests
-```
-
-Per-language files may adjust this (e.g. Elisp collates ERT tests into
-`tests/test-<module>*.el` without subdirectories).
-
-### Testing Pyramid
-
-Rough proportions for most projects:
-- Unit tests: 70-80% (fast, isolated, granular)
-- Integration tests: 15-25% (component interactions, real dependencies)
-- E2E tests: 5-10% (full system, slowest)
-
-Don't duplicate coverage: if unit tests fully exercise a function's logic,
-integration tests should focus on *how* components interact — not repeat the
-function's case coverage.
-
-## Integration Tests
-
-Integration tests exercise multiple components together. Two rules:
-
-**The docstring names every component integrated** and marks which are real vs
-mocked. Integration failures are harder to pinpoint than unit failures;
-enumerating the participants up front tells you where to start looking.
-
-Example:
-
-```
-def test_integration_refund_during_sync_updates_ledger_atomically():
- """Refund processed mid-sync updates order and ledger in one transaction.
-
- Components integrated:
- - OrderService.refund (entry point)
- - PaymentGateway.reverse (MOCKED — returns success)
- - Ledger.credit (real)
- - db.transaction (real)
-
- Validates:
- - Refund rolls back if ledger write fails
- - Both tables updated or neither
- """
-```
-
-**Write an integration test when** multiple components must work together,
-state crosses function boundaries, or edge cases combine. **Don't** when
-single-function behavior suffices, or when mocking would erase the interaction
-you meant to test.
-
-## Naming Convention
-
-- Unit: `test_<module>_<function>_<scenario>_<expected>`
-- Integration: `test_integration_<workflow>_<scenario>_<outcome>`
-
-Examples:
-- `test_cart_apply_discount_expired_coupon_raises_error`
-- `test_integration_order_sync_network_timeout_retries_three_times`
-
-Languages that prefer camelCase, kebab-case, or other conventions keep the
-structure but use their idiom. Consistency within a project matters more than
-the specific case choice.
-
-## Test Quality
-
-### Independence
-- No shared mutable state between tests
-- Each test runs successfully in isolation
-- Explicit setup and teardown
-
-### Determinism
-- Never hardcode dates or times — generate them relative to `now()`
-- No reliance on test execution order
-- No flaky network calls in unit tests
-- Time/clock-mocking helpers must avoid two recurring failure modes:
- - *Infinite recursion.* The helper must not call the primitive it's
- replacing. If the mock for `now()` calls `now()`, the test stack
- overflows. Compute the mock value from a fixed source (a captured
- instant, an injected fake clock).
- - *Scope-shadowing without reach.* A mock that only exists inside
- the test function won't affect production code that reads the
- symbol through its canonical path. Replace the symbol at its
- definition site (monkey-patch the module attribute in Python,
- redefine the global in Lisp, swap the package-level binding in
- Go, replace the named export in JavaScript) — or inject a fake
- via dependency-inversion. Don't lean on scope-shadowing
- primitives (Lisp `let`, Python local rebind, JS shadowed `let`)
- that fence the mock to the test's lexical scope; production code
- won't see them and the test passes against the real clock.
-
-### Performance
-- Unit tests: <100ms each
-- Integration tests: <1s each
-- E2E tests: <10s each
-- Mark slow tests with appropriate decorators/tags
-
-### Mocking Boundaries
-Mock external dependencies at the system boundary:
-- Network calls (HTTP, gRPC, WebSocket)
-- File I/O and cloud storage
-- Time and dates
-- Third-party service clients
-
-Never mock:
-- The code under test
-- Internal domain logic
-- Framework behavior (ORM queries, middleware, hooks, buffer primitives)
-
-### Signs of Overmocking
-
-Ask yourself:
-
-- Would this test still pass if I replaced the function body with `raise NotImplementedError` (or equivalent)? If yes, the mocks are doing the work — you're testing mocks, not code.
-- Is the mock more complex than the function being tested? Smell.
-- Am I mocking internal string / parsing / decoding helpers? Those aren't boundaries — they're the work.
-- Does the test break when I refactor without changing behavior? Good tests survive refactors; overmocked ones couple to implementation.
-
-When tests demand heavy internal mocking, the fix isn't better mocks — it's
-restructuring the code (see *If Tests Are Hard to Write* below).
-
-### Testing Code That Uses Frameworks
-
-When a function mostly delegates to framework or library code, test *your*
-integration logic:
-- ✓ "I call the library with the right arguments in the right context"
-- ✓ "I handle its return value correctly"
-- ✗ "The library works in 50 scenarios" — trust it; it has its own tests
-
-For polyglot behavior (e.g., comment handling across C/Java/Go/JS), test 2-3
-representative modes thoroughly plus a minimal smoke test in the others.
-Exhaustive permutations are diminishing returns.
-
-### Test Real Code, Not Copies
-
-Never inline or copy production code into test files. Always `require`/`import`
-the module under test. Copied code passes even when production breaks — the
-bug hides behind the duplicate.
-
-Mock dependencies at their boundary; exercise the real function body.
-
-### Error Behavior, Not Error Text
-
-Test that errors occur with the right type; don't assert exact wording:
-- ✓ Right exception type (`pytest.raises(ValueError)`, `(should-error ... :type 'user-error)`)
-- ✓ Regex on values the message *must* contain (e.g., the offending filename)
-- ✗ `assert str(e) == "File 'foo' not found"` — breaks when prose changes even though behavior is unchanged
-
-Production code should emit clear, contextual errors. Tests verify the
-behavior (raised, caught, returned nil) and values that must appear — not the
-prose.
-
-## If Tests Are Hard to Write, Refactor the Code
-
-If a test needs extensive mocking of internal helpers, elaborate fixture
-scaffolding, or mocks that recreate the function's own logic, the production
-code needs restructuring — not the test.
-
-Signals:
-- Deep nesting (callbacks inside callbacks)
-- Long functions doing multiple things ("fetch AND parse AND decode AND save")
-- Tests that mock internal string / parsing / I/O helpers
-- Tests that break on refactors with no behavior change
-
-Fix: extract focused helpers (one responsibility each), test each in isolation
-with real inputs, compose them in a thin outer function. Several small unit
-tests plus one composition test beats one monster test behind a wall of mocks.
-
-When the untestable function is legacy code you're hardening, this extraction
-**is** the hardening — not a detour around it. A function whose boundary or
-error case can't be exercised without mocking the world (a shell function that
-calls `tmux`/`git` directly, a handler that reaches straight into I/O) can't be
-characterized, so you can't refactor it safely and you can't pin its edge
-behavior. Extracting the pure decision logic into a helper that takes plain
-inputs and returns a plain result makes that logic characterizable with the full
-Normal/Boundary/Error set; the I/O calls become a thin wrapper you cover once
-with a single composition test. "It needs too much mocking to test" is therefore
-never a reason to skip the boundary and error cases — it's the signal to reshape
-the function so those cases are writable.
-
-## Coverage Targets
-
-- Business logic and domain services: **90%+**
-- API endpoints and views: **80%+**
-- UI components: **70%+**
-- Utilities and helpers: **90%+**
-- Overall project minimum: **80%+**
-
-New code must not decrease coverage. PRs that lower coverage require justification.
-
-## TDD Discipline
-
-TDD is non-negotiable. These are the rationalizations agents use to skip it — don't fall for them:
-
-| Excuse | Why It's Wrong |
-|--------|----------------|
-| "This is too simple to need a test" | Simple code breaks too. The test takes 30 seconds. Write it. |
-| "I'll add tests after the implementation" | You won't, and even if you do, they'll test what you wrote rather than what was needed. Test-after validates implementation, not behavior. |
-| "Let me just get it working first" | That's not TDD. If you can't write a failing test, you don't understand the requirement yet. |
-| "This is just a refactor" | Refactors without tests are guesses. Write a characterization test first, then refactor while it stays green. |
-| "I'm only changing one line" | One-line changes cause production outages. Write a test that covers the line you're changing. |
-| "The existing code has no tests" | Start with a characterization test. Don't make the problem worse. |
-| "This is demo/prototype code" | Demos build habits. Untested demo code becomes untested production code. |
-| "I need to spike first" | Spikes are fine — under the protocol below. Throw the spike away, then write the first failing test before productionizing. |
-
-If you catch yourself thinking any of these, stop and write the test.
-
-### The Spike Exception (Disciplined)
-
-TDD stays the default. The one sanctioned way to write code before a test is
-a spike — exploratory code that answers "is this approach even viable?" when
-you can't yet write a meaningful failing test because the shape of the
-solution is unknown. A spike is disciplined only when all three hold:
-
-1. **Timebox it.** Set a limit before starting (an hour, an afternoon) and
- stop when it's up. An open-ended spike is just untested implementation
- wearing a different name.
-2. **Do not commit spike code.** The spike is a learning artifact, not a
- deliverable. It never enters the branch history. Keep it in a scratch
- file or a throwaway worktree.
-3. **Throw the spike away, then start with a failing test.** Once the spike
- has answered the viability question, delete it. Write the first failing
- test against the now-understood behavior, then productionize under normal
- Red/Green/Refactor. The production code is written test-first even though
- the exploration wasn't — you don't promote the spike into production by
- bolting tests on after.
-
-The spike buys understanding, not code. If you find yourself keeping the
-spike because rewriting it feels wasteful, the timebox was too long or the
-problem was tractable enough to TDD from the start.
-
-## Anti-Patterns (Do Not Do)
-
-- Hardcoded dates or timestamps (they rot)
-- Testing implementation details instead of behavior
-- Mocking the thing you're testing
-- Mocking internal helpers (string ops, parsing, decoding) — those are the work
-- Inlining production code into test files — always `require` / `import` the real module
-- Asserting exact error-message text instead of type + key values
-- Shared mutable state between tests
-- Non-deterministic tests (random without seed, network in unit tests)
-- Testing framework behavior instead of your code
-- Ignoring or skipping failing tests without a tracking issue
+What stays here is what has to be true before any code is written, which is when
+no skill has been summoned yet: test first, and cover all three categories.
## Content scope
diff --git a/claude-rules/todo-format.md b/claude-rules/todo-format.md
index 58570b1..8038b98 100644
--- a/claude-rules/todo-format.md
+++ b/claude-rules/todo-format.md
@@ -139,7 +139,8 @@ measurable one:
against the list, so "covered" is checkable where "found everything"
isn't.
2. **Net the behavior.** Bring the surface under characterization tests
- (Normal/Boundary/Error per unit — see `testing.md`) before changing
+ (Normal/Boundary/Error per unit — see `testing.md`, and the
+ `testing-standards` skill for the characterization recipe) before changing
anything. This is the objective floor: writing a characterization test is
mechanical (record what the code does, not what it should), so it scales
across the surface, and it doubles as the safety net that makes any
diff --git a/claude-templates/.ai/workflows/code-quality.org b/claude-templates/.ai/workflows/code-quality.org
index 0481166..3c4ed8f 100644
--- a/claude-templates/.ai/workflows/code-quality.org
+++ b/claude-templates/.ai/workflows/code-quality.org
@@ -12,7 +12,7 @@ workflow only sequences them and collects the residue.
*Behavior-preserving rests on a test net.* The passes below claim to preserve
behavior, but a refactor on untested code is a guess, not a preservation. Where
the scope has no tests, bring it under a characterization net first
-(Normal/Boundary/Error per unit, per =testing.md='s "Adding Tests to Existing
+(Normal/Boundary/Error per unit, per the =testing-standards= skill's "Adding Tests to Existing
Untested Code") — that net is what turns "behavior-preserving" from an assertion
into something the green suite actually verifies across each pass.
diff --git a/publish/SKILL.md b/publish/SKILL.md
index 48924b7..a93f3dd 100644
--- a/publish/SKILL.md
+++ b/publish/SKILL.md
@@ -251,16 +251,37 @@ enough to skip review" exemption on top of it.
*Voice patterns are always personal for publish artifacts.* Commit messages, PR titles + bodies, and PR review comments all go out under the user's name, so they always run through `/voice personal` (the full pattern walk — general + Craig's-voice + the artifact-mechanics patterns: first-person rewrite, public-artifact scope flag, praise/correction asymmetry, finding stems), regardless of whether `.ai/` is tracked. These three are personal-voice artifacts by definition — the skill's personal mode exists for exactly them. Pattern #39 (public-artifact scope flag) matters *most* on team-visible artifacts, so it must never be skipped on a PR comment or PR body. There is no "general-voice mode" for publish artifacts.
-*The approval gate is the only thing `.ai/`-tracking decides.* Before drafting, run this command:
+*The approval gate turns on whether anyone else reads the history.* The gate
+exists so Craig sees the exact words that go out under his name. Skipping it
+trades that for velocity, and that trade is only worth making where the repo is
+genuinely shared with other people.
+
+The old signal for this was whether `.ai/` is tracked, used as a proxy for
+"team repo." It was the wrong proxy and it failed in the direction that
+matters: rulesets, home, and work all track `.ai/` — rulesets as a committed
+mirror, the others because the project history *is* the project — while all
+three are Craig's private single-user repos. The rule as written skipped the
+gate on his three most-used projects.
+
+Check the remote host instead, which is what actually distinguishes them:
```
-git ls-files :/.ai/ 2>/dev/null | head -1
+git remote -v 2>/dev/null | grep -v 'cjennings\.net' | head -1
```
-The `:/` pathspec anchors the search to the repo root, so the command works from any subdirectory. Without it, running from a subdir returns no matches even when `.ai/` is tracked at the repo root, which silently misclassifies the project.
+- **No output** — every remote is on `cjennings.net`, so the repo is Craig's
+ own and nobody else reads the log. **Gate applies**: write to `/tmp`, run
+ `/voice personal`, print inline, ask approve / request changes / open in
+ editor, and publish only on explicit approval.
+- **Any output** — a remote on a host someone else can read (GitHub, a GHE
+ instance, a team server). **Gate skipped for velocity**: write to `/tmp`, run
+ `/voice personal`, print inline, publish immediately.
+- **No remote at all** — a local-only repo. Gate applies; there is no
+ velocity argument without a reader.
-- **No output** — `.ai/` is gitignored, missing, or empty (the user's personal repos). **Gate applies**: write to `/tmp`, run `/voice personal`, print inline, ask approve / request changes / open in editor, then publish only on explicit approval.
-- **Any output** — one or more files under `.ai/` are tracked (a shared / team repo). **Gate skipped for velocity**: write to `/tmp`, run `/voice personal`, print inline, publish immediately.
+As of 2026-07-27 every project resolves to gate-applies, because every remote
+is `cjennings.net`. That is the correct answer, not a bug: it matches how the
+flow has actually been run.
Either way the draft runs through `/voice personal` first. The subflows below describe the full gated path. For the gate-skipped path, run the same `/voice personal` pass, then collapse the "Ask: approve, request changes, or open in editor" step — the draft prints inline and the publish step runs immediately afterward.
@@ -274,100 +295,12 @@ Either way the draft runs through `/voice personal` first. The subflows below de
- **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again.
- **Open in editor** → only if the user asks. `emacsclient -n /tmp/commit-<short-slug>.md`. After the editor closes, re-read the file, re-print the contents inline, and ask again.
-**For PR descriptions:**
-
-1. Write the title as line 1 and the body below it to `/tmp/pr-<slug>.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay).
-2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title.
-3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings.
-4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default.
- - **Approve** → continue to step 5.
- - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again.
- - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<ticket-or-slug>.md`. After the editor closes, re-read the file, re-print inline, ask again.
-5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 <file>)"` (or a heredoc) so formatting is preserved. Add `--reviewer <user[,user...]>` in the same call when you already know who should review.
-6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit <N> --add-reviewer <user>`. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review.
-7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-<team>.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow.
-
-**For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):**
-
-Pick the shape first. Most reviews are Shape 1.
-
-- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`.
-- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification.
-- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification.
-
-**Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline.
-
-**Shape 1: Single review (bundled summary + inline)**
-
-1. Identify findings, split into **inline-eligible** (each names a specific `path:line`) and **summary-only** (cross-cutting). Decide the verdict.
-
-2. Write one concatenated draft to `/tmp/pr-<N>-review.md` with explicit separators:
-
- ```
- === SUMMARY ===
- <verdict summary body>
-
- === INLINE path=frontend/src/foo.tsx line=440 ===
- <inline body 1>
-
- === INLINE path=frontend/src/bar.tsx line=137 ===
- <inline body 2>
- ```
-
- The separator format is exactly `=== SUMMARY ===` and `=== INLINE path=<path> line=<n> ===`. The summary block is mandatory even for verdict-only reviews. Inline blocks are zero-or-more.
-
-3. Run `/voice personal` on the file once. The skill walks its full pattern list across every block at the same time. The separators stay intact because they aren't prose.
-
-4. Print the final draft inline in the terminal. Every block — the summary body AND the full prose of every inline comment — exactly as it'll be posted, with its separator header. Print the inline in full; never describe it in place of printing it ("I'd pair it with one inline on…"). Craig approves the exact words that post under his name, so the exact words must be on screen. State that the skill ran (e.g. "/voice personal — full pattern walk across summary + 3 inline"). Surface any pattern #39 warnings.
-
-5. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default.
- - **Approve** → continue to step 6.
- - **Request changes** → make them, re-run `/voice personal` on the whole file, re-print inline, ask again.
- - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<N>-review.md`. After the editor closes, re-read, re-print inline, ask again.
-
-6. Split the file on the separator lines and post in **a single** `gh api` call:
-
- ```
- gh api repos/<owner>/<repo>/pulls/<N>/reviews \
- --hostname <ghe-host-or-omit> \
- -F event=REQUEST_CHANGES \
- -F body="<summary block>" \
- -F "comments[][path]=<path1>" \
- -F "comments[][line]=<line1>" \
- -F "comments[][body]=<inline 1>" \
- -F "comments[][path]=<path2>" \
- -F "comments[][line]=<line2>" \
- -F "comments[][body]=<inline 2>"
- ```
-
- `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance).
-
-7. Verify the review landed. `gh api repos/<owner>/<repo>/pulls/<N>/reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted.
-
-8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not.
-
-**Shape 2: Issue-thread comment (no verdict)**
-
-Use when the post is informal discussion that shouldn't appear as a review verdict (e.g. "I'd like to discuss the X approach before you continue").
-
-1. Write the proposed comment to `/tmp/pr-<N>-comment.md`.
-2. Run `/voice personal`.
-3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5.
-4. Post: `gh pr comment <N> --body-file /tmp/pr-<N>-comment.md`.
-5. Verify: `gh api repos/<owner>/<repo>/issues/<N>/comments`.
-6. No review notification.
-
-**Shape 3: Reply on an existing inline thread**
-
-Use when responding to a specific prior reviewer comment.
-
-1. Find the parent comment ID: `gh api repos/<owner>/<repo>/pulls/<N>/comments`.
-2. Write the reply to `/tmp/pr-<N>-reply-<comment-id>.md`.
-3. Run `/voice personal`.
-4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5.
-5. Post: `gh api repos/<owner>/<repo>/pulls/<N>/comments -F in_reply_to=<comment-id> -F body="$(cat /tmp/pr-<N>-reply-<comment-id>.md)"`.
-6. Verify in the same `comments` list.
-7. No review notification.
+**For PR descriptions, and for PR review comments and replies:** read
+`references/pull-requests.md` in this skill directory. It carries the PR
+description shape, the three review shapes (bundled review with inline pins,
+issue-thread comment, threaded reply), the `gh api` calls, and the
+publishing-overlay hooks. A plain commit needs none of it, so it is kept out of
+this file — load it when the artifact is a PR.
**Approve does not authorize a merge.** Reviewing a PR never authorizes merging it. Anything in `## Merge Strategy` below applies only to merges *you* are about to perform on your own branches — and even then, the merge needs its own explicit user confirmation per the rules there. A project's publishing overlay may add a team merge practice (e.g. approve-then-author-merges, where the review notification hands the merge decision to the PR author); that's an overlay concern, not a global one.
diff --git a/publish/references/pull-requests.md b/publish/references/pull-requests.md
new file mode 100644
index 0000000..9ac4ded
--- /dev/null
+++ b/publish/references/pull-requests.md
@@ -0,0 +1,105 @@
+# Pull requests and PR review comments
+
+Loaded from the `publish` skill when the artifact is a PR description or a PR
+review comment. A plain commit never needs any of this, which is why it lives
+here rather than in SKILL.md.
+
+Steps 0 and 1 of the publish flow (pre-flight reconcile, local code review) and
+the `/voice personal` pass still apply — see SKILL.md. This file carries only
+what is specific to PRs.
+
+**For PR descriptions:**
+
+1. Write the title as line 1 and the body below it to `/tmp/pr-<slug>.md`. **Title format:** the conventional-commit subject (`refactor: remove dead if-count-is-not-None check in admin`). If the project defines a publishing overlay with a ticket system, follow it for the ticket suffix in the title and the cross-link line in the body (see the overlay).
+2. Run `/voice personal` on the file. The PR title stays imperative per Conventional Commits — `/voice personal` rewrites the body, not the title.
+3. Print the final draft inline in the terminal. Title on line 1, blank line, then body — exactly as it'll be posted. State that the skill ran. Surface any pattern #39 (public-artifact scope) warnings.
+4. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default.
+ - **Approve** → continue to step 5.
+ - **Request changes** → make them, re-run `/voice personal`, re-print inline, ask again.
+ - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<ticket-or-slug>.md`. After the editor closes, re-read the file, re-print inline, ask again.
+5. Split the file on the first blank line and pass the title and body to `gh pr create --title "..." --body "$(tail -n +3 <file>)"` (or a heredoc) so formatting is preserved. Add `--reviewer <user[,user...]>` in the same call when you already know who should review.
+6. Request reviewers on the new PR if you didn't pass `--reviewer` at create time. Use `gh pr edit <N> --add-reviewer <user>`. If the repo has a `CODEOWNERS` file, GitHub auto-suggests based on touched paths. Still issue the explicit request so the reviewer gets notified. Pick reviewers per the team's convention for the area touched (often documented in the per-repo `CLAUDE.md`). For follow-up PRs, consider tagging the parent PR's author if their context would help. PRs without a human reviewer request stall — "checks passed" is not a substitute for review.
+7. **Project publishing overlay (if present).** If the project defines a publishing overlay — a `publishing-<team>.md` rule loaded from its `.claude/rules/` — run its post-create steps now: ticket cross-linking, ticket-state moves, and any other tracker integration it specifies. A project with no overlay skips this; the PR is already open and reviewers are requested, which is the complete universal flow.
+
+**For PR review comments and replies (review verdicts, threaded discussion, follow-up notes on someone else's PR or your own):**
+
+Pick the shape first. Most reviews are Shape 1.
+
+- **Shape 1 — Single review** (verdict + summary body + 0+ inline pins). The default for any post that carries a verdict (`APPROVE`, `REQUEST_CHANGES`, `COMMENT`), even when the verdict has no line-specific findings. One `gh api` call posts the summary, every inline pin, and the verdict together. review notification fires once for `APPROVE` or `REQUEST_CHANGES`.
+- **Shape 2 — Issue-thread comment** (no verdict). General PR discussion, not a review. No inline pins. No review notification.
+- **Shape 3 — Reply on an existing inline thread**. Responding to a specific prior reviewer comment. Threads under that comment. No review notification.
+
+**Inline threshold for Shape 1.** Any finding that names a `path:line` belongs as an inline comment pinned to that line. Cross-cutting observations (verdict rationale, "third PR with the same pattern", overall test-coverage gaps that don't pin to one place) stay in the summary body. There's no "fold one inline into the summary" exception — a single line-specific finding still goes inline.
+
+**Shape 1: Single review (bundled summary + inline)**
+
+1. Identify findings, split into **inline-eligible** (each names a specific `path:line`) and **summary-only** (cross-cutting). Decide the verdict.
+
+2. Write one concatenated draft to `/tmp/pr-<N>-review.md` with explicit separators:
+
+ ```
+ === SUMMARY ===
+ <verdict summary body>
+
+ === INLINE path=frontend/src/foo.tsx line=440 ===
+ <inline body 1>
+
+ === INLINE path=frontend/src/bar.tsx line=137 ===
+ <inline body 2>
+ ```
+
+ The separator format is exactly `=== SUMMARY ===` and `=== INLINE path=<path> line=<n> ===`. The summary block is mandatory even for verdict-only reviews. Inline blocks are zero-or-more.
+
+3. Run `/voice personal` on the file once. The skill walks its full pattern list across every block at the same time. The separators stay intact because they aren't prose.
+
+4. Print the final draft inline in the terminal. Every block — the summary body AND the full prose of every inline comment — exactly as it'll be posted, with its separator header. Print the inline in full; never describe it in place of printing it ("I'd pair it with one inline on…"). Craig approves the exact words that post under his name, so the exact words must be on screen. State that the skill ran (e.g. "/voice personal — full pattern walk across summary + 3 inline"). Surface any pattern #39 warnings.
+
+5. Ask: approve, request changes, or open in editor. Wait for an explicit answer. Do not open the file in `emacsclient` (or any editor) by default.
+ - **Approve** → continue to step 6.
+ - **Request changes** → make them, re-run `/voice personal` on the whole file, re-print inline, ask again.
+ - **Open in editor** → only if the user asks. `emacsclient -n /tmp/pr-<N>-review.md`. After the editor closes, re-read, re-print inline, ask again.
+
+6. Split the file on the separator lines and post in **a single** `gh api` call:
+
+ ```
+ gh api repos/<owner>/<repo>/pulls/<N>/reviews \
+ --hostname <ghe-host-or-omit> \
+ -F event=REQUEST_CHANGES \
+ -F body="<summary block>" \
+ -F "comments[][path]=<path1>" \
+ -F "comments[][line]=<line1>" \
+ -F "comments[][body]=<inline 1>" \
+ -F "comments[][path]=<path2>" \
+ -F "comments[][line]=<line2>" \
+ -F "comments[][body]=<inline 2>"
+ ```
+
+ `event` is one of `APPROVE`, `REQUEST_CHANGES`, `COMMENT`. The `comments[]` array can be empty for verdicts with zero line-specific findings — the call still uses the same endpoint. Pass `--hostname` for non-`github.com` hosts (a project's publishing overlay names its host when it's a GitHub Enterprise instance).
+
+7. Verify the review landed. `gh api repos/<owner>/<repo>/pulls/<N>/reviews --hostname ...` returns the latest review with bundled inlines. Confirm `state` matches the verdict and the inline count matches what was posted.
+
+8. **Project review-notification overlay (if present).** If the project defines a publishing overlay with a review-notification step (e.g. a Slack ping to the PR author), run it now — but only for `APPROVE` and `REQUEST_CHANGES` verdicts. The overlay owns the channel, the message format, the author-mention lookup, and the threading. A project with no overlay skips notification entirely. `COMMENT` verdicts and Shapes 2-3 below never notify, overlay or not.
+
+**Shape 2: Issue-thread comment (no verdict)**
+
+Use when the post is informal discussion that shouldn't appear as a review verdict (e.g. "I'd like to discuss the X approach before you continue").
+
+1. Write the proposed comment to `/tmp/pr-<N>-comment.md`.
+2. Run `/voice personal`.
+3. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5.
+4. Post: `gh pr comment <N> --body-file /tmp/pr-<N>-comment.md`.
+5. Verify: `gh api repos/<owner>/<repo>/issues/<N>/comments`.
+6. No review notification.
+
+**Shape 3: Reply on an existing inline thread**
+
+Use when responding to a specific prior reviewer comment.
+
+1. Find the parent comment ID: `gh api repos/<owner>/<repo>/pulls/<N>/comments`.
+2. Write the reply to `/tmp/pr-<N>-reply-<comment-id>.md`.
+3. Run `/voice personal`.
+4. Print inline, ask approve/changes/edit, gate as in Shape 1 step 5.
+5. Post: `gh api repos/<owner>/<repo>/pulls/<N>/comments -F in_reply_to=<comment-id> -F body="$(cat /tmp/pr-<N>-reply-<comment-id>.md)"`.
+6. Verify in the same `comments` list.
+7. No review notification.
+
diff --git a/review-code/SKILL.md b/review-code/SKILL.md
index a2b3c2b..05bc6af 100644
--- a/review-code/SKILL.md
+++ b/review-code/SKILL.md
@@ -261,7 +261,7 @@ Do **not** flag any of these as issues:
- **Issues explicitly silenced in code** (e.g., `# type: ignore[...]` with a reason, lint ignore comments) unless the silencing is unjustified
- **Intentional changes** in functionality clearly related to the PR's stated goal
- **Changes in unmodified lines** (real issues in files the PR touches but on lines it doesn't change)
-- **Framework behavior being tested** — see `testing.md` anti-patterns
+- **Framework behavior being tested** — see the `testing-standards` skill, anti-patterns
### Severity Categorization
diff --git a/testing-standards/SKILL.md b/testing-standards/SKILL.md
new file mode 100644
index 0000000..1f16528
--- /dev/null
+++ b/testing-standards/SKILL.md
@@ -0,0 +1,391 @@
+---
+name: testing-standards
+description: |
+ The full testing standard: characterization tests for untested legacy code, the Normal/Boundary/Error case detail, combinatorial and property-based and mutation testing, test organization and the pyramid, integration-test rules, naming conventions, test-quality rules (independence, determinism, performance, mocking boundaries, signs of overmocking, testing framework-heavy code, never inlining production code, asserting error behavior not error text), the refactor-when-tests-are-hard principle, coverage targets, the TDD discipline table, the spike exception, and the anti-pattern list.
+
+ Use when writing or reviewing tests, deciding how to test something, hardening untested code, or judging whether a test suite is adequate.
+
+ Do NOT use for the standing directive itself — that TDD is the default and that every unit needs Normal, Boundary, and Error cases lives in claude-rules/testing.md and is always loaded. Also see the add-tests skill for the guided coverage workflow and pairwise-tests for the combinatorial matrix generator.
+---
+
+# Testing Standards — the detail
+
+Applies to test code and to decisions about how to test.
+
+The standing directive is NOT here. That TDD is the default, and that every
+unit needs Normal, Boundary, and Error cases, lives in `claude-rules/testing.md`
+and is always loaded, because it has to fire before any code gets written and
+nothing else would summon it. This file is everything needed once you are
+actually writing the tests.
+
+### Understand Before You Test
+
+Before writing tests, invest time in understanding the code:
+
+1. **Explore the codebase** — Read the module under test, its callers, and its dependencies. Understand the data flow end to end.
+2. **Identify the root cause** — If fixing a bug, trace the problem to its origin. Don't test (or fix) surface symptoms when the real issue is deeper in the call chain.
+3. **Reason through edge cases** — Consider boundary conditions, error states, concurrent access, and interactions with adjacent modules. Your tests should cover what could actually go wrong, not just the obvious happy path.
+
+### Adding Tests to Existing Untested Code
+
+When working in a codebase without tests:
+
+1. Write a **characterization test** that captures current behavior before making changes
+2. Use the characterization test as a safety net while refactoring
+3. Then follow normal TDD for the new change
+
+A characterization test asserts what the code *actually does* right now, not
+what it *should* do. Write it by running the code against a fixed input,
+reading the exact value or effect it currently produces, and asserting that
+value — Feathers' recipe is to assert something you know is wrong, run it, and
+paste the real value out of the failure. You don't need to know the correct
+answer to write one; you record the observed one. That's what makes it
+mechanical enough to bring a large untested surface under test without
+re-deriving each unit's spec.
+
+**Characterize with the same Normal/Boundary/Error set as any unit** (the three
+categories below), not one happy-path capture per function. On a characterization
+test the negative and boundary cases are the ones that find bugs: untested legacy
+code is weakest exactly at the empty input, the malformed value, the missing
+upstream, and pinning what it *currently* does there writes the wrong behavior
+down in black and white, where it becomes a bug you can see and decide on. When a
+pinned case turns out to be a bug rather than behavior worth preserving, that one
+test graduates from "record current" to "assert correct" and you fix the code.
+The happy-path case is the regression net; the negative and boundary cases are
+the audit.
+
+Bugs that live *inside* a unit are caught by this three-category set; bugs in how
+units compose — ordering, shared state handed between them — are invisible to any
+per-unit test and need a functional/integration test over the composed path (see
+Integration Tests below and the pyramid).
+
+### 1. Normal Cases (Happy Path)
+- Standard inputs and expected use cases
+- Common workflows and default configurations
+- Typical data volumes
+
+### 2. Boundary Cases
+- Minimum/maximum values (0, 1, -1, MAX_INT)
+- Empty vs null vs undefined (language-appropriate)
+- Single-element collections
+- Unicode and internationalization (emoji, RTL text, combining characters)
+- Very long strings, deeply nested structures
+- Timezone boundaries (midnight, DST transitions)
+- Date edge cases (leap years, month boundaries)
+
+### 3. Error Cases
+- Invalid inputs and type mismatches
+- Network failures and timeouts
+- Missing required parameters
+- Permission denied scenarios
+- Resource exhaustion
+- Malformed data
+
+## Combinatorial Coverage
+
+For functions with 3+ parameters that each take multiple values (feature-flag
+combinations, config matrices, permission/role interactions, multi-field
+form validation, API parameter spaces), the exhaustive test count explodes
+(M^N) while 3-5 ad-hoc cases miss pair interactions. Use **pairwise /
+combinatorial testing** — generate a minimal matrix that hits every 2-way
+combination of parameter values. Empirically catches 60-90% of combinatorial
+bugs with 80-99% fewer tests.
+
+Invoke `/pairwise-tests` on the offending function; continue using `/add-tests`
+and the Normal/Boundary/Error discipline for the rest. The two approaches
+complement: pairwise covers parameter *interactions*; category discipline
+covers each parameter's individual edge space.
+
+Skip pairwise when: the function has 1-2 parameters (just write the cases),
+the context requires *provably* exhaustive coverage (regulated systems — document
+in an ADR), or the testing target is non-parametric (single happy path,
+performance regression, a specific error).
+
+## Escalation Beyond Category and Pairwise
+
+The Normal/Boundary/Error categories and the pairwise matrix are the default
+discipline. Two further techniques escalate beyond them — reach for them when
+the default leaves a gap, not on every unit.
+
+### Property-Based Testing
+
+When an invariant holds across a broad input domain — round-trips
+(`decode(encode(x)) == x`), idempotence (`f(f(x)) == f(x)`), ordering
+invariants (output is always sorted), or any "output always satisfies X" —
+generate inputs and assert the property instead of enumerating cases. The
+generator explores corners you wouldn't think to write by hand, and a
+failing case shrinks to a minimal reproducer. Use the standard tool for the
+language (Hypothesis for Python, fast-check for JS, proptest for Rust).
+State the property as the test name and let the framework supply the inputs.
+
+Reach for this when the behavior is a law over a domain rather than a fixed
+set of examples. Keep category-discipline cases for the specific edges that
+must always hold; the property test covers the space between them.
+
+### Mutation Testing
+
+When line coverage is high but you suspect the assertions are thin — tests
+that execute the code without checking its output, or that pass with a
+function body replaced by a stub — use mutation testing to measure whether
+the suite actually kills injected faults. The tool flips conditionals, swaps
+operators, and deletes statements, then reruns the suite; a surviving mutant
+is a fault the tests didn't catch. Use mutmut or cosmic-ray for Python,
+Stryker for JS. High line coverage with a low mutation score means weak
+assertions, not a tested codebase.
+
+Reach for this on critical logic where coverage looks reassuring but you
+want evidence the tests would fail on a regression. It's a diagnostic, not a
+gate on every change — mutation runs are slow.
+
+## Test Organization
+
+Typical layout:
+
+```
+tests/
+ unit/ # One test file per source file
+ integration/ # Multi-component workflows
+ e2e/ # Full system tests
+```
+
+Per-language files may adjust this (e.g. Elisp collates ERT tests into
+`tests/test-<module>*.el` without subdirectories).
+
+### Testing Pyramid
+
+Rough proportions for most projects:
+- Unit tests: 70-80% (fast, isolated, granular)
+- Integration tests: 15-25% (component interactions, real dependencies)
+- E2E tests: 5-10% (full system, slowest)
+
+Don't duplicate coverage: if unit tests fully exercise a function's logic,
+integration tests should focus on *how* components interact — not repeat the
+function's case coverage.
+
+## Integration Tests
+
+Integration tests exercise multiple components together. Two rules:
+
+**The docstring names every component integrated** and marks which are real vs
+mocked. Integration failures are harder to pinpoint than unit failures;
+enumerating the participants up front tells you where to start looking.
+
+Example:
+
+```
+def test_integration_refund_during_sync_updates_ledger_atomically():
+ """Refund processed mid-sync updates order and ledger in one transaction.
+
+ Components integrated:
+ - OrderService.refund (entry point)
+ - PaymentGateway.reverse (MOCKED — returns success)
+ - Ledger.credit (real)
+ - db.transaction (real)
+
+ Validates:
+ - Refund rolls back if ledger write fails
+ - Both tables updated or neither
+ """
+```
+
+**Write an integration test when** multiple components must work together,
+state crosses function boundaries, or edge cases combine. **Don't** when
+single-function behavior suffices, or when mocking would erase the interaction
+you meant to test.
+
+## Naming Convention
+
+- Unit: `test_<module>_<function>_<scenario>_<expected>`
+- Integration: `test_integration_<workflow>_<scenario>_<outcome>`
+
+Examples:
+- `test_cart_apply_discount_expired_coupon_raises_error`
+- `test_integration_order_sync_network_timeout_retries_three_times`
+
+Languages that prefer camelCase, kebab-case, or other conventions keep the
+structure but use their idiom. Consistency within a project matters more than
+the specific case choice.
+
+## Test Quality
+
+### Independence
+- No shared mutable state between tests
+- Each test runs successfully in isolation
+- Explicit setup and teardown
+
+### Determinism
+- Never hardcode dates or times — generate them relative to `now()`
+- No reliance on test execution order
+- No flaky network calls in unit tests
+- Time/clock-mocking helpers must avoid two recurring failure modes:
+ - *Infinite recursion.* The helper must not call the primitive it's
+ replacing. If the mock for `now()` calls `now()`, the test stack
+ overflows. Compute the mock value from a fixed source (a captured
+ instant, an injected fake clock).
+ - *Scope-shadowing without reach.* A mock that only exists inside
+ the test function won't affect production code that reads the
+ symbol through its canonical path. Replace the symbol at its
+ definition site (monkey-patch the module attribute in Python,
+ redefine the global in Lisp, swap the package-level binding in
+ Go, replace the named export in JavaScript) — or inject a fake
+ via dependency-inversion. Don't lean on scope-shadowing
+ primitives (Lisp `let`, Python local rebind, JS shadowed `let`)
+ that fence the mock to the test's lexical scope; production code
+ won't see them and the test passes against the real clock.
+
+### Performance
+- Unit tests: <100ms each
+- Integration tests: <1s each
+- E2E tests: <10s each
+- Mark slow tests with appropriate decorators/tags
+
+### Mocking Boundaries
+Mock external dependencies at the system boundary:
+- Network calls (HTTP, gRPC, WebSocket)
+- File I/O and cloud storage
+- Time and dates
+- Third-party service clients
+
+Never mock:
+- The code under test
+- Internal domain logic
+- Framework behavior (ORM queries, middleware, hooks, buffer primitives)
+
+### Signs of Overmocking
+
+Ask yourself:
+
+- Would this test still pass if I replaced the function body with `raise NotImplementedError` (or equivalent)? If yes, the mocks are doing the work — you're testing mocks, not code.
+- Is the mock more complex than the function being tested? Smell.
+- Am I mocking internal string / parsing / decoding helpers? Those aren't boundaries — they're the work.
+- Does the test break when I refactor without changing behavior? Good tests survive refactors; overmocked ones couple to implementation.
+
+When tests demand heavy internal mocking, the fix isn't better mocks — it's
+restructuring the code (see *If Tests Are Hard to Write* below).
+
+### Testing Code That Uses Frameworks
+
+When a function mostly delegates to framework or library code, test *your*
+integration logic:
+- ✓ "I call the library with the right arguments in the right context"
+- ✓ "I handle its return value correctly"
+- ✗ "The library works in 50 scenarios" — trust it; it has its own tests
+
+For polyglot behavior (e.g., comment handling across C/Java/Go/JS), test 2-3
+representative modes thoroughly plus a minimal smoke test in the others.
+Exhaustive permutations are diminishing returns.
+
+### Test Real Code, Not Copies
+
+Never inline or copy production code into test files. Always `require`/`import`
+the module under test. Copied code passes even when production breaks — the
+bug hides behind the duplicate.
+
+Mock dependencies at their boundary; exercise the real function body.
+
+### Error Behavior, Not Error Text
+
+Test that errors occur with the right type; don't assert exact wording:
+- ✓ Right exception type (`pytest.raises(ValueError)`, `(should-error ... :type 'user-error)`)
+- ✓ Regex on values the message *must* contain (e.g., the offending filename)
+- ✗ `assert str(e) == "File 'foo' not found"` — breaks when prose changes even though behavior is unchanged
+
+Production code should emit clear, contextual errors. Tests verify the
+behavior (raised, caught, returned nil) and values that must appear — not the
+prose.
+
+## If Tests Are Hard to Write, Refactor the Code
+
+If a test needs extensive mocking of internal helpers, elaborate fixture
+scaffolding, or mocks that recreate the function's own logic, the production
+code needs restructuring — not the test.
+
+Signals:
+- Deep nesting (callbacks inside callbacks)
+- Long functions doing multiple things ("fetch AND parse AND decode AND save")
+- Tests that mock internal string / parsing / I/O helpers
+- Tests that break on refactors with no behavior change
+
+Fix: extract focused helpers (one responsibility each), test each in isolation
+with real inputs, compose them in a thin outer function. Several small unit
+tests plus one composition test beats one monster test behind a wall of mocks.
+
+When the untestable function is legacy code you're hardening, this extraction
+**is** the hardening — not a detour around it. A function whose boundary or
+error case can't be exercised without mocking the world (a shell function that
+calls `tmux`/`git` directly, a handler that reaches straight into I/O) can't be
+characterized, so you can't refactor it safely and you can't pin its edge
+behavior. Extracting the pure decision logic into a helper that takes plain
+inputs and returns a plain result makes that logic characterizable with the full
+Normal/Boundary/Error set; the I/O calls become a thin wrapper you cover once
+with a single composition test. "It needs too much mocking to test" is therefore
+never a reason to skip the boundary and error cases — it's the signal to reshape
+the function so those cases are writable.
+
+## Coverage Targets
+
+- Business logic and domain services: **90%+**
+- API endpoints and views: **80%+**
+- UI components: **70%+**
+- Utilities and helpers: **90%+**
+- Overall project minimum: **80%+**
+
+New code must not decrease coverage. PRs that lower coverage require justification.
+
+## TDD Discipline
+
+TDD is non-negotiable. These are the rationalizations agents use to skip it — don't fall for them:
+
+| Excuse | Why It's Wrong |
+|--------|----------------|
+| "This is too simple to need a test" | Simple code breaks too. The test takes 30 seconds. Write it. |
+| "I'll add tests after the implementation" | You won't, and even if you do, they'll test what you wrote rather than what was needed. Test-after validates implementation, not behavior. |
+| "Let me just get it working first" | That's not TDD. If you can't write a failing test, you don't understand the requirement yet. |
+| "This is just a refactor" | Refactors without tests are guesses. Write a characterization test first, then refactor while it stays green. |
+| "I'm only changing one line" | One-line changes cause production outages. Write a test that covers the line you're changing. |
+| "The existing code has no tests" | Start with a characterization test. Don't make the problem worse. |
+| "This is demo/prototype code" | Demos build habits. Untested demo code becomes untested production code. |
+| "I need to spike first" | Spikes are fine — under the protocol below. Throw the spike away, then write the first failing test before productionizing. |
+
+If you catch yourself thinking any of these, stop and write the test.
+
+### The Spike Exception (Disciplined)
+
+TDD stays the default. The one sanctioned way to write code before a test is
+a spike — exploratory code that answers "is this approach even viable?" when
+you can't yet write a meaningful failing test because the shape of the
+solution is unknown. A spike is disciplined only when all three hold:
+
+1. **Timebox it.** Set a limit before starting (an hour, an afternoon) and
+ stop when it's up. An open-ended spike is just untested implementation
+ wearing a different name.
+2. **Do not commit spike code.** The spike is a learning artifact, not a
+ deliverable. It never enters the branch history. Keep it in a scratch
+ file or a throwaway worktree.
+3. **Throw the spike away, then start with a failing test.** Once the spike
+ has answered the viability question, delete it. Write the first failing
+ test against the now-understood behavior, then productionize under normal
+ Red/Green/Refactor. The production code is written test-first even though
+ the exploration wasn't — you don't promote the spike into production by
+ bolting tests on after.
+
+The spike buys understanding, not code. If you find yourself keeping the
+spike because rewriting it feels wasteful, the timebox was too long or the
+problem was tractable enough to TDD from the start.
+
+## Anti-Patterns (Do Not Do)
+
+- Hardcoded dates or timestamps (they rot)
+- Testing implementation details instead of behavior
+- Mocking the thing you're testing
+- Mocking internal helpers (string ops, parsing, decoding) — those are the work
+- Inlining production code into test files — always `require` / `import` the real module
+- Asserting exact error-message text instead of type + key values
+- Shared mutable state between tests
+- Non-deterministic tests (random without seed, network in unit tests)
+- Testing framework behavior instead of your code
+- Ignoring or skipping failing tests without a tracking issue
+
+## Content scope
+
+Test code, fixtures, docstrings, and comments are checked into the repo and visible to the team. They must follow the *Content scope for public artifacts* rule in [`commits.md`](commits.md): no local paths, no private repo names, no personal tooling references.