Skip to content
Open
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
6 changes: 3 additions & 3 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ Before answering architecture questions or starting non-trivial work in an unfam
### 1. Plan Mode Default

- Enter plan mode for ANY non-trivial task (3+ steps or architectural decisions). If something goes sideways, STOP and re-plan.
- **Plan format**: atomic tasks with explicit file paths, each independently verifiable. State what changes, where, and how to prove it works.
- **Plan format**: atomic tasks with explicit file paths, each independently verifiable. State what changes, where, and how to prove it works. For any item whose necessity isn't self-evident, also state **what breaks without it** — a task that can't answer that is a task to cut, and the answer becomes the deletion probe the reviewer runs later.
- **Plan the smallest thing that satisfies the request.** Over-engineering is far cheaper to prevent here than to prune later, and the plan is where speculative machinery gets legitimised: once "add a `--strategy` flag" is a numbered task, it reads as a requirement rather than a guess. Name the caller for every parameter, option, and abstraction the plan introduces — if that caller is hypothetical, cut the item. Don't plan extensibility nobody asked for, and don't plan a helper you'd write exactly one call to. Detail in `coding-standards.md` ("Simplicity & Scope (YAGNI)").
- **User checkpoint**: for multi-commit plans, cross-cutting refactors, or anything touching shared infrastructure (`infra-ops.md`), share the plan before implementing.
- **Plan review loop — MANDATORY gate before implementation starts**: review the plan, fix every issue, re-review. Repeat until **three consecutive passes find nothing** (any finding restarts the count at zero). Do NOT create the §1b worktree, enter ExitPlanMode, or write code until this passes. Each pass covers: the six review dimensions (see below); Reuse (§1a); scope discipline (only what was asked?); blast radius (callers, tests, migrations, downstream consumers all listed?); unknowns (verify "verify-first" items NOW, not at implementation time). Per-pass findings go in the plan as a short "review pass N" note.
Expand All @@ -94,7 +94,7 @@ Before answering architecture questions or starting non-trivial work in an unfam
- **Security**: injection, auth bypass, secrets exposure, OWASP top 10, input validation at boundaries?
- **Bugs**: race conditions, null derefs, edge cases, error-handling gaps, resource leaks?
- **Duplication**: re-invents anything already in the project? If yes, reuse/refactor per §1a.
- **Over-engineering**: is every parameter set by a real caller, every abstraction used by more than one consumer, every guard protecting a reachable state, every comment earning its line? Prune what fails, per `coding-standards.md` ("Simplicity & Scope (YAGNI)"), and remove it safely per that file's "Verifying a Refactor". Review this dimension **adversarially**: the author's local justification for a piece of machinery almost always holds up, so ask instead what the calling system actually does and what would break if the machinery were deleted. Correct, well-tested code guarding an unreachable state still comes out.
- **Over-engineering**: is every parameter set by a real caller, every abstraction used by more than one consumer, every guard protecting a reachable state, every comment earning its line? Prune what fails, per `coding-standards.md` ("Simplicity & Scope (YAGNI)"), and remove it safely per that file's "Verifying a Refactor". Review this dimension **adversarially**: the author's local justification for a piece of machinery almost always holds up, so ask instead what the calling system actually does and what would break if the machinery were deleted. Correct, well-tested code guarding an unreachable state still comes out. Where the answer is genuinely arguable, **don't argue it — run a deletion probe** (`coding-standards.md`, "Deletion probes"): delete the candidate, run the verification, and let the result decide. A "nothing broke" that turns out to be a coverage gap rather than dead code is the most valuable finding this dimension produces.

### 1a. Reuse Before Writing — Avoid Duplication

Expand Down Expand Up @@ -123,7 +123,7 @@ Non-trivial work happens in a dedicated git worktree branched off the current br
Every code change the implementer (Sonnet for simpler changes, Opus for non-trivial code, per §2) produces during the implementation phase is reviewed locally by Opus before it counts as done. Local analog of the post-PR CodeRabbit loop (`git-workflow.md`): catch issues in the worktree before the diff is pushed. Does NOT replace the §1 post-implementation review or the §1b merge gate; it runs inside the implementation phase, upstream of both.

The loop:
1. **The implementer (Sonnet or Opus per §2) implements** an atomic task (or one logically complete chunk) per the approved plan. Write the plan's task, not a generalised version of it: no parameter without a caller in this changeset, no abstraction with one consumer, no guard against a state the callers cannot produce, no comment restating the line below it. If the task as written seems to need machinery the plan didn't call for, that is a signal to re-plan rather than to improvise it (§1).
1. **The implementer (Sonnet or Opus per §2) implements** an atomic task (or one logically complete chunk) per the approved plan. Write the plan's task, not a generalised version of it: no parameter without a caller in this changeset, no abstraction with one consumer, no guard against a state the callers cannot produce, no comment restating the line below it. If the task as written seems to need machinery the plan didn't call for, that is a signal to re-plan rather than to improvise it (§1). Before handing the diff to review, **probe your own additions** (`coding-standards.md`, "Deletion probes"): delete each piece whose necessity you couldn't state in one sentence and see whether anything actually fails. Cheaper to find here than in review, and what survives arrives with evidence attached.
2. **Opus reviews the diff locally** across the six review dimensions plus Reuse (§1a) and scope discipline. Spawn a dedicated Opus reviewer subagent (set via `model`) to keep the implementer's context clean; escalate the review to Fable only for the hardest money-path / architecture calls where peak intelligence matters. Emit a concrete findings list (`file:line` + what's wrong + suggested fix), or an explicit "no actionable findings".
3. **The implementer addresses** every finding. Mechanical, decided fixes stay with the implementer; a finding needing a design call escalates that item to Opus (or Fable for a peak call, §2 carve-out), then the decided fix goes back to the implementer.
4. **Opus re-reviews** the updated diff.
Expand Down
15 changes: 15 additions & 0 deletions coding-standards.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,21 @@ A cleanup that changes behaviour is a bug wearing a tidy diff. "Delete aggressiv

If you can't prove an edit is behaviour-preserving, don't make it. A small confident diff beats a large clever one.

### Deletion probes — test necessity, don't argue it

Whether a piece of machinery is needed is an empirical question, so answer it empirically instead of by reasoning about the code. For each candidate: **delete it, run the full verification, and read the result.**

- **Something fails** → it is load-bearing. Restore it, and record *what* failed: that failure is the evidence it earns its place, and it belongs in the PR description or the protected list.
- **Nothing fails** → exactly one of two things is true, and you must say which:
1. the code was genuinely unnecessary — delete it for real; or
2. **the verification is too weak to notice** — restore the code and report a coverage gap.
Comment on lines +78 to +83

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Require a clean baseline before interpreting a deletion probe.

Line 80 treats any post-deletion failure as evidence that the candidate is load-bearing. A pre-existing or flaky failure can produce the same result. Run the same verification on the unchanged code first. Then run it after deleting only the candidate. Treat only a new, reproducible failure as evidence. If either run is incomplete or not comparable, restore the candidate and report the probe as inconclusive.

Proposed probe sequence
-For each candidate: **delete it, run the full verification, and read the result.**
+For each candidate, first run the full verification on the unchanged code and confirm that it completes cleanly. Then delete only that candidate and rerun the same verification.
 
-- **Something fails** → it is load-bearing. Restore it, and record *what* failed: that failure is the evidence it earns its place, and it belongs in the PR description or the protected list.
+- **A new, reproducible failure appears only after deletion** → it is load-bearing. Restore it, and record *what* failed: that failure is the evidence it earns its place, and it belongs in the PR description or the protected list.
 
+- If either verification run is incomplete or not comparable, restore the candidate and report the probe as inconclusive.

As per coding guidelines, verification must be explicit and must not silently accept invalid or incomplete results.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
Whether a piece of machinery is needed is an empirical question, so answer it empirically instead of by reasoning about the code. For each candidate: **delete it, run the full verification, and read the result.**
- **Something fails** → it is load-bearing. Restore it, and record *what* failed: that failure is the evidence it earns its place, and it belongs in the PR description or the protected list.
- **Nothing fails** → exactly one of two things is true, and you must say which:
1. the code was genuinely unnecessary — delete it for real; or
2. **the verification is too weak to notice** — restore the code and report a coverage gap.
Whether a piece of machinery is needed is an empirical question, so answer it empirically instead of by reasoning about the code. For each candidate, first run the full verification on the unchanged code and confirm that it completes cleanly. Then delete only that candidate and rerun the same verification.
- **A new, reproducible failure appears only after deletion** → it is load-bearing. Restore it, and record *what* failed: that failure is the evidence it earns its place, and it belongs in the PR description or the protected list.
- **Nothing fails** → exactly one of two things is true, and you must say which:
1. the code was genuinely unnecessary — delete it for real; or
2. **the verification is too weak to notice** — restore the code and report a coverage gap.
- If either verification run is incomplete or not comparable, restore the candidate and report the probe as inconclusive.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@coding-standards.md` around lines 78 - 83, Update the deletion-probe guidance
around “delete it, run the full verification” to require a clean baseline
verification on unchanged code first, followed by a comparable verification
after deleting only the candidate. Count only new, reproducible failures as
evidence that the candidate is load-bearing; if either run is incomplete,
non-comparable, pre-existing, or flaky, restore the candidate and report the
probe as inconclusive.

Source: Coding guidelines


**Never read "nothing broke" as automatic permission to delete.** If removing a guard that protects a real invariant breaks no test, the honest finding is "this guard is untested", not "this guard is unnecessary". Those cases are usually worth more than the deletions, because they point at the hole that let the risk in.

Restore between probes so they can't interact, and probe one thing at a time — two simultaneous deletions that cancel out will read as a clean run.

This is the cheap way to settle the arguments that stall a cleanup, and it cuts both directions: it deletes machinery whose justification was only ever theoretical, and it produces hard evidence for the pieces that survive. A parameter proven necessary by a failing build is no longer a matter of opinion.

## Preferred Stack

- **Language**: Go for new backend/CLI projects; TypeScript/Node for frontend, lightweight CLIs, or when the ecosystem fit is strong; match the existing language for additions to existing projects
Expand Down