From 2cd56be2bfd19b283e6a6913f8006882c1f0c312 Mon Sep 17 00:00:00 2001 From: Danny Brown Date: Thu, 1 Oct 2026 20:09:28 -0400 Subject: [PATCH] fix: rename duplicated /code-review to /two-axis-review; add an eval for it --- README.md | 6 +- evals/README.md | 2 + evals/two-axis-review/01-both-axes/case.yaml | 55 ++++++++++++++ .../two-axis-review/01-both-axes/scaffold.sh | 72 +++++++++++++++++++ scripts/skill_tree_cli.py | 2 +- skills/implement/SKILL.md | 4 +- .../{code-review => two-axis-review}/SKILL.md | 6 +- 7 files changed, 138 insertions(+), 9 deletions(-) create mode 100644 evals/two-axis-review/01-both-axes/case.yaml create mode 100755 evals/two-axis-review/01-both-axes/scaffold.sh rename skills/{code-review => two-axis-review}/SKILL.md (95%) diff --git a/README.md b/README.md index 88aa4e5..fc80907 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ Claude, `/` in Copilot); `skill-tree help` prints the same map. ```mermaid flowchart LR - grill[grill-for-planning] --> domain([domain-modeling]) --> proto([prototype]) --> design([codebase-design]) --> spec[to-spec] --> tickets[to-tickets] --> impl[implement] --> review[code-review] + grill[grill-for-planning] --> domain([domain-modeling]) --> proto([prototype]) --> design([codebase-design]) --> spec[to-spec] --> tickets[to-tickets] --> impl[implement] --> review[two-axis-review] handoff[handoff] -.-> tickets & impl verify[verify] -.-> review ``` @@ -28,7 +28,7 @@ Rounded nodes are optional but usually worth it. | 5. Write it down | `to-spec` | Spec (synthesis only, no interview) | | 6. Slice it | `to-tickets` | Vertical-slice tickets with blocking edges | | 7. Build | `implement` | TDD by default, verified proof it works, HITL by default | -| 8. Review | `code-review` | Standards + Spec axes, in parallel | +| 8. Review | `two-axis-review` | Standards + Spec axes, in parallel | Alongside: `handoff` (carry state across sessions), `verify` (back done-claims with evidence). `domain-modeling` stays live after step 2 — revisit it whenever a term drifts. @@ -67,7 +67,6 @@ Each lives at `skills//` with its own `SKILL.md`, `scripts/`, and `referen | `adversarial-review` | Manually-triggered red-team review of the current branch's diff. Constructs real failing inputs/races/states rather than checklist-verifying. | | `bro` | Restate the last message in a more grokable way | | `cli-ergonomics` | CLIs should be incredibly easy for humans to run. Invoke when creating a new CLI or subcommand a human will run (incl. a script promoted out of one-off use), or when the user asks for a CLI review. Not for edits to existing commands. Covers the argument-handling ladder (help over error, TTY-guarded prompts, fzf selection, echoing the replayable command) and the hard `--version` requirement. Not for pure-library or single-purpose CI-only scripts. | -| `code-review` | Two-axis review of changes since a fixed point (commit, tag, HEAD~N, or default HEAD~1): Standards (does the code follow this repo's conventions?) and Spec (does the code do what it should?). Axes run as parallel sub-agents so neither masks the other. Use when the user says \"review\", \"code review\", \"check my changes\", or before pushing. | | `codebase-design` | Shared vocabulary for designing deep modules. Invoke when designing or improving a module's interface, finding deepening opportunities, deciding where a seam goes, making code more testable, or when another skill needs the deep-module vocabulary — \"is this the right seam\", \"design this interface\", \"why does this feel shallow\". | | `debug-ci` | Fix a GitHub Actions run that has failed — e.g. \"why did CI fail\", \"the build is red\", \"check the Actions run\", \"/debug-ci\". Fetches the failure logs via `gh`, diagnoses the root cause, and fixes it locally. User will review and push. | | `debug-hooks` | Invoke to list or debug hooks — \"what hooks do I have\", \"is my hook firing\", \"why did that run twice\". Merges every hook source (settings, plugins, Copilot) into one list. | @@ -89,6 +88,7 @@ Each lives at `skills//` with its own `SKILL.md`, `scripts/`, and `referen | `to-spec` | A conversation has settled on what to build and it's time to turn it into a written spec — \"turn this into a spec\", \"write this up\", \"spec this out\". Synthesis only: no interview. Pairs well with skill-tree:grill-for-planning, which is where the settled understanding usually comes from. | | `to-tickets` | Break a spec, plan, or conversation into tracer-bullet vertical-slice tickets with blocking edges — \"turn this into tickets\", \"break this down\", \"what's the work?\". Pairs with skill-tree:to-spec upstream and skill-tree:handoff downstream. | | `tui-screenshots` | Use for generating or refreshing TUI screenshots for docs — \"regenerate the screenshots\", \"the README screenshots are stale\". Drives a Textual/TUI app headlessly and exports SVG. Not for reading a user's screenshot — that is `screenshot`. | +| `two-axis-review` | Two-axis review of changes since a fixed point (commit, tag, HEAD~N, or default HEAD~1): Standards (does the code follow this repo's conventions?) and Spec (does the code do what it should?). Axes run as parallel sub-agents so neither masks the other. Use when the user says \"review against the spec\", \"does this follow our conventions\", \"standards and spec review\", or \"two-axis review\". Not a correctness bug hunt — that's the built-in `/code-review`. | | `ui-designer` | Design or restyle a web UI — landing pages, dashboards, docs sites, app chrome — or when the user asks why a site \"looks generic\", \"looks like a template\", or wants it to feel intentional. A running list of design lessons learned, applied as rules rather than suggestions. | | `verify` | Verify before answering whether something works, is gone, is used, or is correct — and whenever the user says \"can you confirm\", \"are you sure\", \"did that actually work\", or reports something is \"still\" broken. Produces the answer plus the evidence that would have falsified it. | | `wait-what` | Stop. Your plan/idea did not land. Re-pitch it. | diff --git a/evals/README.md b/evals/README.md index 446eee6..7a6489a 100644 --- a/evals/README.md +++ b/evals/README.md @@ -9,6 +9,7 @@ just eval ui-designer # 2 cases, ~2.5 min, ~$0.55–0.7 just eval debug-ci # 3 cases, ~1.5 min, ~$0.60 just eval repo-audit # 5 cases just eval handoff # 6 fixtures, skill arm only +just eval two-axis-review # 1 case just eval debug-ci --runs 3 # more samples per case just eval ui-designer --ablation with-without # also run without the plugin, report the delta ``` @@ -18,6 +19,7 @@ just eval ui-designer --ablation with-without # also run without the plugin, re | `debug-ci` | `claude plugin eval` | diagnose from the CI log, fix, verify locally, no git writes | | `ui-designer` | `claude plugin eval` | the skill's design rules, as greppable checks on a built page and a review | | `repo-audit` | `claude plugin eval` | runs the checker, read-only, report format, catches the seeded gaps | +| `two-axis-review` | `claude plugin eval` | both axes run as sub-agents, each seeded issue lands on its axis, read-only | | `handoff` | `handoff/run.py` | write a handoff, resume it in a fresh session, judge what survived | For `claude plugin eval` suites, the recipe defaults to one run per case, no baseline arm, diff --git a/evals/two-axis-review/01-both-axes/case.yaml b/evals/two-axis-review/01-both-axes/case.yaml new file mode 100644 index 0000000..d60901c --- /dev/null +++ b/evals/two-axis-review/01-both-axes/case.yaml @@ -0,0 +1,55 @@ +schema_version: "1.0" +name: two-axis-review/01-both-axes +description: >- + The last commit breaks one documented convention (no type hints) and misses + one spec requirement (compound durations). Each issue lands on exactly one + axis, so a review that runs only one axis, or merges them, misses one. +tags: [two-axis-review] +context: + scaffold_script: scaffold.sh +execution: + prompt: >- + Review my last commit against docs/spec.md and against this repo's + conventions. + max_turns: 25 + timeout_seconds: 420 + allowed_tools: [Read, Glob, Grep, Bash, Edit, Write, Skill, Agent] +runs: 1 +graders: + - type: tool_used + name: skill-fired + tool: Skill + input_match: two-axis-review + - type: tool_used + name: spawned-both-axes + tool: Agent + min: 2 + - type: tool_used + name: read-only-no-edit + tool: Edit + min: 0 + max: 0 + - type: tool_used + name: read-only-no-write + tool: Write + min: 0 + max: 0 + - type: tool_used + name: no-git-writes + tool: Bash + input_match: '\bgit\s+(add|commit|push|reset|checkout|restore|stash)\b' + min: 0 + max: 0 + - type: regex + name: axes-as-headings + target: last_message + pattern: '##\s*Standards[\s\S]*##\s*Spec' + - type: llm + name: each-issue-on-its-axis + criteria: >- + Pass only if the final message (1) under a Standards heading flags that + parse_duration has no type hints and ties it to the repo's CLAUDE.md + rule, (2) under a Spec heading flags that compound durations such as + "1h30m" are not supported though docs/spec.md requires them, and + (3) keeps the two axes as separate reports rather than merging them + into one combined ranking. Extra findings do not fail the case. diff --git a/evals/two-axis-review/01-both-axes/scaffold.sh b/evals/two-axis-review/01-both-axes/scaffold.sh new file mode 100755 index 0000000..0f7722d --- /dev/null +++ b/evals/two-axis-review/01-both-axes/scaffold.sh @@ -0,0 +1,72 @@ +#!/usr/bin/env bash +set -euo pipefail + +commit() { + git -c user.name=eval -c user.email=eval@example.invalid add -A + git -c user.name=eval -c user.email=eval@example.invalid commit -qm "$1" +} + +mkdir -p docs + +cat >CLAUDE.md <<'EOF' +# Conventions + +- Every Python function has type hints on all parameters and on its return value. +- Invalid input raises `ValueError`; never return `None` to signal failure. +EOF + +cat >docs/spec.md <<'EOF' +# parse_duration + +`durations.parse_duration(text)` turns a human duration into whole seconds. + +- Accept a single unit: `90s`, `5m`, `2h`. +- Accept compound durations, units in descending order: `1h30m`, `2m15s`. +- Ignore surrounding whitespace. +- Raise `ValueError` for anything else, including negative amounts and unknown units. +EOF + +cat >README.md <<'EOF' +# durations +Parse human-readable durations. +EOF + +git init -q -b main +commit 'chore: init' + +cat >durations.py <<'EOF' +import re + +UNITS = {"s": 1, "m": 60, "h": 3600} + + +def parse_duration(text): + match = re.fullmatch(r"(\d+)([smh])", text.strip()) + if not match: + raise ValueError(f"bad duration: {text!r}") + amount, unit = match.groups() + return int(amount) * UNITS[unit] +EOF + +cat >test_durations.py <<'EOF' +import unittest + +from durations import parse_duration + + +class ParseDurationTest(unittest.TestCase): + def test_single_units(self) -> None: + self.assertEqual(parse_duration("90s"), 90) + self.assertEqual(parse_duration(" 5m "), 300) + self.assertEqual(parse_duration("2h"), 7200) + + def test_rejects_unknown_unit(self) -> None: + with self.assertRaises(ValueError): + parse_duration("3d") + + +if __name__ == "__main__": + unittest.main() +EOF + +commit 'feat: add parse_duration' diff --git a/scripts/skill_tree_cli.py b/scripts/skill_tree_cli.py index 58f9f29..1d6f3ea 100755 --- a/scripts/skill_tree_cli.py +++ b/scripts/skill_tree_cli.py @@ -466,7 +466,7 @@ def cmd_help(root: Path, _args: list[str]) -> int: ' grill-for-planning → [domain-modeling] → [prototype]' ' → [codebase-design]', ) - print(' → to-spec → to-tickets → implement → code-review') + print(' → to-spec → to-tickets → implement → two-axis-review') print(' ([] = optional; domain-modeling stays live throughout)') print(' ├─ handoff maintain state between sessions') print(' └─ verify shore up done-claims before closing') diff --git a/skills/implement/SKILL.md b/skills/implement/SKILL.md index ed9c39e..7ba5397 100644 --- a/skills/implement/SKILL.md +++ b/skills/implement/SKILL.md @@ -90,7 +90,7 @@ When the work is complete (one ticket in default mode, the frontier in autonomou 3. **Run `pre-commit`** (or `prek`) if the project has it. Fix what it reports. 4. **Invoke [[verify]]** on each done-claim. Run the thing, not just its tests — every claim in the summary ships with the check that could have refuted it and its output. -5. **Invoke [[code-review]]** on the diff. Fix anything it finds before presenting to the user. +5. **Invoke [[two-axis-review]]** on the diff. Fix anything it finds before presenting to the user. 6. **Present the work.** Summary of what changed, the proof from step 4, the verification commands, and any open questions. Don't commit — the user does that. @@ -99,6 +99,6 @@ When the work is complete (one ticket in default mode, the frontier in autonomou - **[[to-tickets]]** or **[[to-spec]]** produce the input this skill consumes - **[[handoff]]** maintains session state as you work - **[[verify]]** proves each done-claim at finish, and before writing it to the narrative -- **[[code-review]]** runs at the end before presenting to the user +- **[[two-axis-review]]** runs at the end before presenting to the user - **[[codebase-design]]** — deep-module vocabulary for any interface this work adds - **[[domain-modeling]]** — use `CONTEXT.md` vocabulary in code, tests, and commit-ready summaries diff --git a/skills/code-review/SKILL.md b/skills/two-axis-review/SKILL.md similarity index 95% rename from skills/code-review/SKILL.md rename to skills/two-axis-review/SKILL.md index 65a69a9..769c0ac 100644 --- a/skills/code-review/SKILL.md +++ b/skills/two-axis-review/SKILL.md @@ -1,11 +1,11 @@ --- -name: code-review -description: "Two-axis review of changes since a fixed point (commit, tag, HEAD~N, or default HEAD~1): Standards (does the code follow this repo's conventions?) and Spec (does the code do what it should?). Axes run as parallel sub-agents so neither masks the other. Use when the user says \"review\", \"code review\", \"check my changes\", or before pushing." +name: two-axis-review +description: "Two-axis review of changes since a fixed point (commit, tag, HEAD~N, or default HEAD~1): Standards (does the code follow this repo's conventions?) and Spec (does the code do what it should?). Axes run as parallel sub-agents so neither masks the other. Use when the user says \"review against the spec\", \"does this follow our conventions\", \"standards and spec review\", or \"two-axis review\". Not a correctness bug hunt — that's the built-in `/code-review`." user-invocable: true allowed-tools: Read, Grep, Glob, Bash, Agent --- -# Code Review +# Two-Axis Review Two-axis review: **Standards** and **Spec**, run in parallel so one can't mask the other.