Merge #678: say test-driven-development applies when code is pasted inline

This commit is contained in:
Addy Osmani authored and GitHub committed 2026-10-13 12:58:19 -07:00
commit 7d65f5e781
8 files changed
+71 -1

No files matched your search

+2
View File
@@ -52,6 +52,8 @@ Two rules keep it honest. The manifest's `experimental.evals` key scopes the sca
What the first runs taught (Claude Code 2.1.278, claude-opus-5). Skill invocation is stochastic: the review skill fired in 5 of 7 runs on the phrasing the case uses, and test-driven-development fired in 2 of 7 runs on the test-first prompt, so read `Δ` and the report rather than the exit code, or pass `--threshold 0.8` as the CI example in the docs does. In one run the review skill did not fire and the reply still used its severity taxonomy as headings, which no baseline run did in nine; the loaded skill descriptions may shape the reply even without an invocation. Phrasing matters, and the description is the lever. With the original description, "Review this diff before I merge it", "Can you do a proper code review of this change before I merge it?" and "Review this pull request for me before it goes into main" rarely fired the skill; the model reviewed the diff on its own, competently but without the verdict, the severity tiers or the five-axis pass. Describing what the skill adds did not help, and neither did naming diffs and pull requests on their own; what made the difference was saying that the skill applies even when the diff is pasted inline, which is the case the model was treating as "I can just do this myself". On claude-opus-5-5 with the model pinned, that one clause took the four review phrasings from 7 of 27 fires to 21 of 27, left the nine adjacent must-not-fire prompts at 0 of 27 both ways, and moved the plainest phrasing from 0 of 9 to 3 of 9, so it is better, not solved. `code-review-stays-quiet-on-commit-message` guards that clause: a pasted diff with a commit-message request must not fire the review skill. Two practical rules from the same day: pin `--model` and record the Claude Code version whenever you report numbers, because the CLI updates itself and the default model changed twice between morning and evening; and give a case a turn budget of 10 or more, since the model may reach a skill through its slash command (`agent-skills:review`, then `code-review-and-quality`), which costs two turns.
The same lever, applied to test-driven-development (claude-opus-5-5 pinned, six runs per prompt). With the original description, "Add unit tests for this function", "Fix this bug ... here is the function" and "Change this function so that size can never exceed 100" fired the skill 0 of 6 each, while the process-shaped "Write a failing test for this bug before we fix it" fired 6 of 6: the model wrote the tests or the fix directly. Here the tail alone was not enough; what worked was naming the three requests and saying the skill applies even when the code is pasted inline and the request does not mention tests. That clause took the four prompts from 6 of 24 to 19 of 24 (add tests 6 of 6, fix a bug 6 of 6, change behavior 1 of 6), and left eight adjacent must-not-fire prompts at 0 of 24 under every variant tried. The one-line behavior change is the floor: the trace shows a single-turn edit with no tool call, the model does not consider a procedure for it. `tdd-fires-on-pasted-bug` and `tdd-stays-quiet-on-debug` are the pair for that clause: the first checks the skill fires on a pasted bug and that the reply reproduces it with a failing test before the fix, the second checks that a "what broke" question with a diff does not pull the TDD skill in. On the first, the skill fired in every run, but the reply put the failing test before the fix in 2 of 4 runs with the plugin and 0 of 3 without: the judge is strict on purpose, since that order is what the skill adds, and the misses (fix first, then a test with a note that it would have failed on the old code) are an adherence gap of the skill in a session with no shell to run anything, not a routing one.
## Eval case format
One file per skill: `evals/cases/<skill-name>.json`.
+8
View File
@@ -13,6 +13,14 @@
{
"prompt": "What tests should cover this new parsing logic before I write it?",
"top_k": 3
},
{
"prompt": "Add unit tests for this function. There are none yet.",
"top_k": 3
},
{
"prompt": "Fix this bug: splitCents(100, 3) returns [33, 33, 33] and a cent goes missing. Here is the function.",
"top_k": 3
}
],
"negative": [
@@ -0,0 +1,7 @@
---
type: llm
---
PASS if the reply gives a test that reproduces the bug (splitCents(100, 3) summing to 100, or the missing cent) and presents it as the first step, to be run and seen failing before the function is changed, with the fix coming after it.
FAIL if the reply changes the function without a reproducing test, or adds tests only after the fix as an afterthought, or never says the test should fail first.
@@ -0,0 +1,5 @@
---
type: tool_used
tool: Skill
input_match: '"skill"\s*:\s*"(?:[\w-]+:)?(test-driven-development|test)"'
---
@@ -0,0 +1,15 @@
---
description: A bug fix request with the function pasted inline and no mention of tests. The skill should fire, and the reply should reproduce the bug with a failing test before fixing it.
expected_outcome: A test that reproduces the lost cent, shown or described as failing first, then the fix; test-driven-development is invoked.
max_turns: 12
allowed_tools: [Read, Glob, Grep, Skill]
---
Fix this bug: splitCents(100, 3) returns [33, 33, 33] and a cent goes missing. Here is the function.
```js
function splitCents(totalCents, n) {
const share = Math.floor(totalCents / n);
return Array.from({ length: n }, () => share);
}
```
@@ -0,0 +1,8 @@
---
type: tool_used
tool: Skill
input_match: '"skill"\s*:\s*"(?:[\w-]+:)?(test-driven-development|test)"'
min: 0
max: 0
arm: both
---
@@ -0,0 +1,25 @@
---
description: A pasted diff with a "what broke" question that belongs to debugging-and-error-recovery. Guards the "fix a bug in it" clause of the test-driven-development description, since a bug mentioned in a diagnosis request must not make the TDD skill fire.
expected_outcome: A diagnosis of the off-by-one; test-driven-development is never invoked.
max_turns: 12
allowed_tools: [Read, Glob, Grep, Skill]
---
The orders page test started failing right after this change: it expects 20 rows and gets 19. Here is the diff. What broke?
```diff
diff --git a/src/orders.js b/src/orders.js
--- a/src/orders.js
+++ b/src/orders.js
@@ -12,3 +12,9 @@ function listOrders(db) {
return db.query('SELECT * FROM orders ORDER BY created_at DESC');
}
-module.exports = { listOrders };
+function pageOrders(db, page, size) {
+ const rows = db.query('SELECT * FROM orders ORDER BY created_at DESC');
+ const start = (page - 1) * size;
+ return rows.slice(start, start + size - 1);
+}
+
+module.exports = { listOrders, pageOrders };
```
+1 -1
View File
@@ -1,6 +1,6 @@
---
name: test-driven-development
description: Drives development with tests using the red-green-refactor loop. Use when implementing any logic, fixing any bug, or changing any behavior. Use when you need to prove that code works, when a bug report arrives, or when you're about to modify existing functionality.
description: Drives development with tests using the red-green-refactor loop. Use when implementing any logic, fixing any bug, or changing any behavior. Use when you need to prove that code works, when a bug report arrives, or when you're about to modify existing functionality. Use when asked to add tests to a function, fix a bug in it or change its behavior, even when the code is pasted inline and the request does not mention tests.
---
# Test-Driven Development