Merge #596: pilot claude plugin eval on code-review-and-quality

This commit is contained in:
Addy Osmani authored and GitHub committed 2026-09-22 20:58:32 -07:00
commit 4374556979
8 files changed
+84

No files matched your search

+15
View File
@@ -37,6 +37,21 @@ Tier 3 supports two behavioral artifact kinds. `execution` is the default: each
The executor runs with an explicit permission mode (`--permission-mode acceptEdits` plus a pre-approved tool list) so execution evals can genuinely edit files, run commands, inspect diffs, and make commits rather than being denied and narrating instead. Traces are fenced as untrusted data in the grader prompt and piped to the grader over stdin (they can be megabytes; argv would hit the OS argument-size limit), executor and grader calls carry timeouts, and grader output is validated as JSON before being written to `evals/results/` (gitignored) in skill-creator's `grading.json` shape. Discipline skills also include pressure cases for time pressure, sunk cost, and authority pressure; these verify that the workflow still holds when the prompt argues for skipping it.
## Plugin evals (Claude Code)
`claude plugin eval` (Claude Code 2.1.269 or later) loads the whole plugin, every skill at once, runs the cases under `evals/plugin/`, and scores each case with and without the plugin. It complements Tier 2 rather than replacing it: Tier 2 checks that a description carries the vocabulary users say; this checks that Claude Code's own router picks the skill on a natural prompt, and what the skill adds to the reply (the `Δ` column).
```bash
claude plugin eval . --no-publish # every case, both arms, 3 runs each
claude plugin eval . --case code-review-fires --runs 1 --ablation none # one cheap iteration on one case
```
A case is a directory holding `prompt.md` (frontmatter: turn and tool limits; body: the prompt, sent verbatim) and one grader per file under `graders/`. Reports land in `evals/results/<timestamp>/`, already gitignored. Every run and every `llm` grader is a real model call on your account, so this runs on demand and never in CI, like Tier 3.
Two rules keep it honest. The command scans all of `evals/` for `prompt.md` or `case.yaml`, so never give a fixture file either name. And a `tool_used: Skill` grader only says whether the skill fired; in a two-arm run it is excluded from the score, so pair it with a grader on the reply itself. `code-review-fires` shows the pattern: the fired indicator, a free `regex` grader that looks for the skill's severity taxonomy (Critical, Required, Nit, Optional, Consider, FYI) used as a heading or as a label, and one `llm` judge on the planted off-by-one. `code-review-stays-quiet` is the precision check: the review skill must not fire on a test-first request, and its `Δ` is expected to be 0; a second, unscored indicator records whether test-driven-development fires instead.
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: "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" never fired the skill in six runs; the model reviewed the diff on its own, competently but without the verdict, the severity tiers or the five-axis pass. Naming the axes ("across correctness, readability, security and performance") is what fires it, so the case uses that; the plainer phrasings are the open finding, and the fix, if any, belongs in the skill's description, not in the prompt. The model may also reach a skill through its slash command (`agent-skills:review`, then `code-review-and-quality`), which costs two turns, so give a case a turn budget of 10 or more.
## Eval case format
One file per skill: `evals/cases/<skill-name>.json`.
@@ -0,0 +1,7 @@
---
type: llm
---
PASS if the review points out that `rows.slice(start, start + size - 1)` returns one row fewer than `size` on every page (the end index of slice is exclusive) and treats it as something that must be fixed before merge, for example by labelling it Critical or leaving it unprefixed as a required change.
FAIL if the review does not mention that pages come back one row short, or files that finding as a Nit, Optional, Consider, or FYI.
@@ -0,0 +1,4 @@
---
type: regex
pattern: '(^|\n)#{1,6} *(Critical|Required|Nit|Optional|Consider|FYI)\b|\b(Critical|Required|Nit|Optional|Consider|FYI):'
---
@@ -0,0 +1,5 @@
---
type: tool_used
tool: Skill
input_match: '"skill"\s*:\s*"(?:[\w-]+:)?code-review-and-quality"'
---
+31
View File
@@ -0,0 +1,31 @@
---
description: A review request that names the axes, with a small diff inline. The skill should fire (through the review command or directly) and the reply should carry its severity labels.
expected_outcome: A structured review with a verdict, findings labelled with the skill's severity taxonomy, and the off-by-one in pageOrders treated as a required change.
max_turns: 12
allowed_tools: [Read, Glob, Grep, Skill]
---
I want a code review of this diff across correctness, readability, security and performance before merging. It adds paging to the orders endpoint.
```diff
diff --git a/src/orders.js b/src/orders.js
--- a/src/orders.js
+++ b/src/orders.js
@@ -12,3 +12,15 @@ 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);
+}
+
+app.get('/orders', (req, res) => {
+ const page = Number(req.query.page) || 1;
+ const size = Number(req.query.size) || 20;
+ res.json(pageOrders(db, page, size));
+});
+
+module.exports = { listOrders, pageOrders };
```
@@ -0,0 +1,8 @@
---
type: tool_used
tool: Skill
input_match: '"skill"\s*:\s*"(?:[\w-]+:)?code-review-and-quality"'
min: 0
max: 0
arm: both
---
@@ -0,0 +1,6 @@
---
type: tool_used
tool: Skill
input_match: '"skill"\s*:\s*"(?:[\w-]+:)?test-driven-development"'
arm: with-only
---
@@ -0,0 +1,8 @@
---
description: A test-first request that belongs to test-driven-development. The review skill must not fire.
expected_outcome: The reply proposes a failing test first; code-review-and-quality is never invoked.
max_turns: 12
allowed_tools: [Read, Glob, Grep, Skill]
---
Write a failing test for this bug before we fix it: splitCents(100, 3) returns [33, 33, 33] and a cent goes missing.