Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 7 additions & 5 deletions .claude/commands/finish.md
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,10 @@ Verify:

## Step 2: Parse the body section structure

The issue body should have these seven sections, in this order, with heading names preserved exactly:
The issue body should have these eight sections, in this order, with heading names preserved exactly:

- `## Context` — orientation for where this issue sits in the cascade
- `## Assumptions` — explicit calibration surface for ambiguity the rough-in author filled in (per Addy Osmani's "good spec for AI agents" pattern); each gap tagged `[ASSUMPTION:]` so plan mode can enumerate them. Valid contents include `- None — all parameters explicit from the framing intent and acceptance criteria.` when no assumptions were made; the section must be present even when empty.
- `## Implementation` — the load-bearing section you will use as the primary plan-mode anchor
- `## Acceptance criteria` — observable, verifiable outcomes the work must satisfy
- `## Test plan` — named tests the implementer writes red-first; one per acceptance criterion for logic-regime modules (see `.claude/rules/testing.md` for regime classification)
Expand Down Expand Up @@ -59,6 +60,7 @@ This isn't comprehensive — Claude Code can't detect every in-progress state. B
Construct a plan-mode prompt using the issue body. The primary anchor is the `## Implementation` section. The other sections are supporting context:

- `## Context` tells plan mode why this issue exists and where it sits in the cascade
- `## Assumptions` lists `[ASSUMPTION:]`-tagged calibration points the rough-in author surfaced. Plan mode **must enumerate each tagged item** in the plan output as a "Confirm or correct" line so the user can revise during plan iteration. **Do not silently resolve assumptions at plan time** — even when plan mode has a strong default, surface it so the user can push back. Resolved assumptions (those the user confirms or corrects) inline into the plan's relevant Implementation step rather than persisting as a separate "Resolved" subsection. If the section says `- None — ...` or contains no `[ASSUMPTION:]` lines, proceed silently.
- `## Acceptance criteria` become the contract the plan must satisfy
- `## Test plan` names the tests the plan must scaffold red-first (Step 6) before implementing — these are the executable form of the acceptance criteria
- `## Done signal` is the verification command the plan must produce as its final step
Expand Down Expand Up @@ -113,13 +115,13 @@ Once the implementation is complete and `mise run check` passes:

**Quick summary of the rubric for orientation** (the rules file is authoritative when in doubt):

- **Apply** — behavior-preserving fix with concrete evidence of need: defects with reproducible failing path, factually wrong comments / rot, dead code / unused imports, existing-doc clarity improvements, defensive additions with concrete evidence, local-symbol renames when fact-based, missing docstrings on entirely-undocumented surfaces. One focused commit per finding with a Conventional Commits message that names it.
- **Apply** — behavior-preserving fix with concrete evidence of need (defects with reproducible failing path, factually wrong comments / rot, dead code / unused imports, existing-doc clarity improvements, defensive additions with concrete evidence, local-symbol renames when fact-based, missing docstrings on entirely-undocumented surfaces) **OR low-risk small changes** (≤ ~15 LOC, confined, no design call required, plausibly an improvement — typo fixes, redundant assertion drops, inlining one-shot helpers, extracting constants the agent already named, simplifying obvious-once-named expressions). One focused commit per finding with a Conventional Commits message that names it.
- **Apply with care** — execute as its own commit but flag in the hand-off summary: cross-file refactors >~50 LOC, new module outside named files, materially altering changes, non-obvious correctness fixes.
- **Surface** — do not apply, note with agent's rationale verbatim: stylistic disagreement, structural refactors, speculative defensive guards (no concrete failing scenario), doc *expansions* of existing-but-thin sections, rule-of-three not hit on suggested abstractions, alternative approaches the agent prefers but the existing one is also fine.
- **Surface** — do not apply, note with agent's rationale verbatim. **Genuine design or taste calls only**: structural refactors that change surface area, speculative defensive guards (no concrete failing scenario), doc *expansions* of existing-but-thin sections, rule-of-three not hit on suggested abstractions, alternative approaches the agent prefers but the existing one is also fine. **NOT Surface**: small low-risk improvements (those go to Apply); style nits without strong rationale (those go to Reject with a one-line dismissal — surfacing them inflates the user's review surface).
- **Defer** — conflicts with an ADR or out-of-scope.
- **Reject** — agent factually misunderstood.
- **Reject** — agent factually misunderstood, OR style nit suggested without strong rationale (one-line dismissal is enough).

**When uncertain, Surface — don't Apply.** The Apply gate is **behavior-preservation + concrete evidence**, not "the agent's reasoning is sound." Speculative findings without a demonstrated path go to Surface. Auto-applying speculative changes trains future humans (and agents) to ignore the bot.
**When uncertain about a small low-risk change, Apply. When uncertain about a design or taste call, Surface.** Atomic-commit history makes any wrong Apply call cheap to revert during draft review, and the user-side cost of triaging Surface items routinely exceeds applying-and-showing the diff. Reserve Surface for genuine taste calls and design decisions where reasonable people disagree. **Medium-confidence findings follow the same triage as high-confidence findings** — confidence is about whether the finding is real, not whether to apply it. **Don't bias away from being defensive**: defensive hardening (race guards, missing cleanups, future-edit foot-guns) is a correctness concern even when the test passes today — those go in Apply, not Surface.

**Pre-filter generated files, lock files, vendored deps, and build artifacts** before the agents read them — see `pr-review.md` § Pre-filters.

Expand Down
13 changes: 9 additions & 4 deletions .claude/rules/pr-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -41,13 +41,17 @@ Every finding goes into exactly one of four classes. The Apply class is narrow a

| Class | Action | Triggers (see § Apply/Surface calibration below for category-by-category) |
|---|---|---|
| **Apply** | Execute the fix; commit as its own focused commit on the branch with a Conventional Commits message that names the finding. | Behavior-preserving fix with concrete evidence of need. Includes: defects with reproducible failing path; factually wrong comments / rot; dead code / unused imports; existing-doc improvements (clarity, precision, accuracy); defensive additions with concrete evidence; local-symbol renames when fact-based; missing docstrings on entirely-undocumented surfaces. |
| **Apply** | Execute the fix; commit as its own focused commit on the branch with a Conventional Commits message that names the finding. | Behavior-preserving fix with concrete evidence of need. Includes: defects with reproducible failing path; factually wrong comments / rot; dead code / unused imports; existing-doc improvements (clarity, precision, accuracy); defensive additions with concrete evidence; local-symbol renames when fact-based; missing docstrings on entirely-undocumented surfaces. **Also includes low-risk small changes**: ≤ ~15 LOC delta, confined to one file or symbol, no design call required, plausibly an improvement per the agent's rationale (e.g. typo fixes, redundant assertion drops, inlining a one-shot helper, extracting a constant the agent already named, simplifying an obvious-once-named expression). The user-side cost of triaging 5 small Surface items routinely exceeds applying them and presenting the diff. |
| **Apply with care** | Execute as its own focused commit, but flag in the hand-off summary so the user knows to scrutinize. | Cross-file refactors >~50 LOC; introducing a new module/file outside the issue's named files; changes that materially alter test or implementation strategy; correctness fix whose implementation is non-obvious. |
| **Surface** | Do **not** apply. Note in hand-off summary with the agent's rationale verbatim. | Stylistic disagreement; structural refactors that change surface area; speculative defensive guards (no concrete failing scenario); doc *expansions* of existing-but-thin sections; rule-of-three not hit on suggested abstractions; alternative implementation approaches the agent prefers but the existing one is also fine. |
| **Surface** | Do **not** apply. Note in hand-off summary with the agent's rationale verbatim. | **Genuine design or taste calls only**: stylistic disagreement where the existing form is also fine; structural refactors that change surface area; speculative defensive guards (no concrete failing scenario); doc *expansions* of existing-but-thin sections; suggested abstractions where the existing duplication is small (rule-of-three not yet hit); alternative implementation approaches the agent prefers but the existing one is also fine. **NOT Surface**: small low-risk improvements (those go to Apply); style nits the agent suggested without strong rationale (those go to Reject with a one-line dismissal — don't surface them with verbatim rationale, that just inflates the user's review surface). |
| **Defer** | Note in hand-off summary; do not modify the issue body or open follow-up issues without user direction. | Conflicts with an ADR or with the issue's intentional design; finding is explicitly listed under the issue's `## Out of scope`. |
| **Reject** | Note in hand-off summary with one-line justification. | Agent factually misunderstood the codebase or the spec; finding contradicts project rules or an existing ADR. |

The Apply gate is **behavior-preservation + concrete evidence**, adapted from Sourcery's auto-apply split (refactorings that don't change behavior auto-apply; suggestions that may change behavior never auto-apply). The "concrete evidence" half adopts PhotoStructure's "prove it or discard it" rule — speculative findings without a demonstrated failing path go to Surface.
The Apply gate is **behavior-preservation + concrete evidence**, adapted from Sourcery's auto-apply split (refactorings that don't change behavior auto-apply; suggestions that may change behavior never auto-apply). The "concrete evidence" half adopts PhotoStructure's "prove it or discard it" rule — speculative findings without a demonstrated failing path go to Surface. The Apply class also extends to low-risk small changes that are plausibly improvements (≤ ~15 LOC, confined, no design call) — the calibration learning here is that user-side triage cost on small Surface items routinely exceeds the cost of applying-and-showing the diff, and atomic-commit history makes any wrong call cheap to revert.

**When uncertain about a small low-risk change, Apply. When uncertain about a design or taste call, Surface.** The split: if the agent's suggestion is a small, confined, no-design-call edit and the rationale is plausible, prefer Apply — atomic-commit history makes the fix easy to revert and the user-side cost of triaging Surface items routinely exceeds applying-and-showing. Reserve Surface for genuine taste calls and design decisions where reasonable people disagree. **Medium-confidence findings from review agents follow the same triage as high-confidence findings** — confidence is about whether the finding is real, not whether to apply it. Don't default mediums to Surface just because they're medium-confidence; that converts the bot's "I'm not sure this is real" into the user's "now you have to triage 5 items." The Apply bar is "would a future maintainer be wrong without this fix, OR is the fix small + low-risk + plausibly an improvement?" — defects, ADR/standards/logging-contract violations, defensive hardening, factual rot, and small low-risk improvements meet that bar; genuine design or taste calls do not.

**Don't bias away from being defensive.** Defensive hardening (e.g., adding a race guard, a missing cleanup, a setup that prevents a future edit from silently breaking a test) is a correctness concern even when the test passes today — those go in Apply, not Surface. Conservatism here means *defending the codebase against future drift*, not *being conservative about applying defenses*.

## Apply / Surface calibration

Expand Down Expand Up @@ -93,7 +97,8 @@ The calibration that distinguishes this rubric from a generic "Apply correctness
| Sub-category | Class | Why |
|---|---|---|
| Formatter / linter violations not auto-caught by `mise run check` | **Apply** | If the project's check task should have caught it, fixing the lint is part of finishing. |
| Stylistic preference (one-liner vs multi-line, function vs method, etc.) | **Surface** | Genuine taste; not the bot's call. |
| Stylistic preference with a substantive rationale (e.g., consistency with a project pattern in `STANDARDS.md`, readability impact the agent can articulate) | **Surface** | Genuine taste call; human's judgment. |
| Stylistic nit the agent suggested without a strong rationale (one-liner vs multi-line, function vs method, naming flavor) | **Reject** | One-line dismissal in the hand-off. Surfacing these with verbatim rationale just inflates the user's review surface — that's the exact pattern that trains humans to ignore the bot. |

## What NOT to flag — the exclusion list

Expand Down
4 changes: 3 additions & 1 deletion .claude/skills/rough-in/SKILL.md
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ Rough-in's full flow (target identification → context gathering → research

**Detect-then-confirm** at session start: propose a mode in one sentence based on the user's opening message and any prior rough-in runs visible in `README.md` index. Let the user override in one word. If the opening message gives no signal, default to **standard mode** for first-time users.

**What rough-in must NOT skip even in light mode**: reading all required inputs in full, the deferred meta-issues pre-flight check, at least one HITL gate on the final sub-sub-issue set before commit, and the planning-backend commit atomic transition discipline.
**What rough-in must NOT skip even in light mode**: reading all required inputs in full, the deferred meta-issues pre-flight check, at least one HITL gate on the final sub-sub-issue set before commit, the planning-backend commit atomic transition discipline, and **framing-invited judgment calls** — if the framing's milestone notes explicitly invite a rough-in-time decision (e.g. "may compress 1+2", "rough-in shapes the final boundaries", "decision deferred to rough-in"), the issue plan gate is non-collapsible regardless of rigor mode. These are the calls light mode is wrong about by default — the framing flagged that judgment lives at this layer.

## Step 1 — Inheritance and pre-flight checks

Expand Down Expand Up @@ -129,6 +129,8 @@ For each approved issue plan entry, draft the full sub-sub-issue spec using the
7. **Dependencies** — explicit list of prior R-issues that must be complete before this one can start
8. **Done signal** — a specific, observable outcome that means this issue is done. Usually the same as the acceptance criteria's top-line check but stated as a verification command the user can run.

**Section names + ordering on disk are project-specific.** The 8 properties above are conceptual — *what* every spec must express, not *what headings* to use. The literal heading names and ordering are parser-driven by the project's executor (`/finish` slash command or equivalent — see the project's `cbk-conventions.md` for the canonical list). Before drafting spec bodies, check the executor's body-parser section to confirm the required headings; mismatched headings will fail the parser even when the content is correct. Concepts like "Technical detail" and "Plan-mode prompt" typically merge into a single `## Implementation` section in the executor's template; "Intent" typically maps to `## Context`. Express the 8 properties in whatever heading shape the project's executor expects.

**The Claude Code plan-mode prompt is the most important part of each spec.** It's the thing that makes rough-in valuable over just "framing plus a wish list." A plan-mode prompt has these properties:

- Written in second person ("Implement the Verifier trait...") so Claude Code reads it as an instruction
Expand Down