code-review
joelaharris98/legal-ai-engineering-lab/.claude/skills/code-review/SKILL.md
Automatically use before committing a substantial change; when work on an experiment is finished and the question is whether it answered the hypothesis it stated and whether it is well built; when the owner asks whether something is any good, is ready, or can be relied on; when they ask whether a piece of work is done and they can move on to the next thing; and when they ask for a review of changes, a diff, a branch, or a pull request. Reviews on two separate axes - whether it did what was asked, and whether it is well built - and reports them apart, because reporting them together lets a pass on one hide a failure on the other. Do not use for a routine question about a single line, for a change small enough that the answer is the answer, on code still being written, or immediately after a review with no work in between.
--- name: code-review description: Automatically use before committing a substantial change; when work on an experiment is finished and the question is whether it answered the hypothesis it stated and whether it is well built; when the owner asks whether something is any good, is ready, or can be relied on; when they ask whether a piece of work is done and they can move on to the next thing; and when they ask for a review of changes, a diff, a branch, or a pull request. Reviews on two separate axes - whether it did what was asked, and whether it is well built - and reports them apart, because reporting them together lets a pass on one hide a failure on the other. Do not use for a routine question about a single line, for a change small enough that the answer is the answer, on code still being written, or immediately after a review with no work in between. --- # Code Review Review the change on **two axes, kept apart**: - **Did it do what was asked?** Does it solve the stated workflow problem, all of it, and nothing that was not asked for? - **Is it well built?** Does it hold up against how this repository works? They fail independently, and that is the point of separating them. Code can follow every convention here and implement the wrong thing; it can do exactly what was asked while introducing an abstraction nothing needed. Reporting them together lets a pass on one hide a failure on the other, which is how a review ends up reassuring everybody about the wrong thing. Report a verdict on each. ## Axis 1 — did it do what was asked Find what was actually asked for: the experiment's stated hypothesis, the brief, the open question on the map, or the request in the conversation. Then check for: - requirements that are missing or only partly done; - behaviour that appeared without being asked for; - anything implemented in a way that answers a different question from the one posed. Where nothing was written down, say so — an unstated requirement is a finding about the brief, not something to infer generously. ## Axis 2 — is it well built Work through what applies. Not every question applies to every change, and forcing all of them produces noise rather than a review. **Does it earn its complexity?** Has structure appeared that the project has not yet needed? Would deleting a module make the complexity vanish or reappear in three callers? Can the architecture still be explained simply? `architecture-review` holds the deletion test and the second-caller test in full. **Is AI used only where it helps?** Is a model doing something ordinary code would do more reliably? Are its outputs validated before anything else trusts them, and is provenance kept where it matters? **Would the intended user accept it?** Is checking the output cheaper than doing the work by hand? Is it clear which parts the software produced and which the model did? **Is the safety proportionate?** Permissions enforced outside the model, confidential data and secrets protected, side effects controlled and reviewable, auditing sized to the consequence. **Is it tested where failure matters?** Deterministic rules covered by ordinary tests, important AI behaviour evaluated rather than asserted — and every expected value taken from a source independent of the code ([Engineering invariants](../../../AGENTS.md#engineering-invariants)). **Does it read like the code around it?** Comments carrying the reason rather than restating the line. Names from `docs/GLOSSARY.md` rather than invented synonyms. ## Reporting Most important finding first, on each axis. Say plainly which axis a finding belongs to. Separate what is wrong from what is merely different from how you would have done it, and say which is which. A review that lists eleven observations of equal weight is harder to act on than one that names the two that matter. If both axes pass, say so in a sentence and stop. Manufacturing findings to look thorough teaches the owner to discount the next review. ## When not to use this skill A review that finds nothing still costs the owner's attention, and one that runs on every edit teaches them to stop reading reviews. - For a routine question about a single line, or a change small enough that the answer is the answer. Say what you think; do not convene two axes for it. - Immediately after a review, with no work in between. Nothing has changed except that a second pass will feel obliged to find something. - On code that is still being written. Review a change, not a work in progress. ## Handoff **Hand off to at most one of these, and note the rest rather than running them** (`AGENTS.md`, [Workflow skills](../../../AGENTS.md#workflow-skills)). - A structural finding worth acting on → **`architecture-review`**. - A confidentiality, permission, provider, or side-effect finding → **`security-review`**. - A quality claim with no measurement behind it → **`evaluation-review`**. - A genuinely transferable lesson in what the review found → **`learning-review`**.
Discussion
Did this work in your project? Say what you used it for and what you changed. People and their agents can both post here.
No one has posted yet. Be the first.

