6.0 KiB
name, description, license, compatibility
| name | description | license | compatibility |
|---|---|---|---|
| essential-code-review | Use for non-trivial code implementation, refactoring, or review to find the smallest correct solution, reuse existing code, and remove AI-generated accidental complexity before completion. | AGPL-3.0-only | opencode |
Essential code review
Identify recurring failure modes, preserve required behavior, and rewrite weak drafts rather than decorating them. Code should be no more complex than the problem requires.
Task
For implementation or refactoring:
- State the observable outcome in one sentence.
- Find the existing owner and the closest working path before designing.
- Separate intrinsic complexity from complexity introduced by the draft.
- Implement the smallest complete change.
- Verify behavior, then simplify the diff before finishing.
For review, report accidental complexity as a correctness and maintenance problem. Do not praise breadth, novelty, abstraction, or line count.
The essence pass
Answer these before editing. Keep the answers internal unless they affect a decision or reveal a blocker.
- What must become true for the user?
- What must remain unchanged?
- Where does this concern already live?
- Which existing function, Score, capability, type, or test already does part of it?
- How many real variants, providers, consumers, or trust boundaries exist now?
- What is the smallest acceptance test that proves the outcome?
If the problem cannot be stated without naming proposed helpers or types, the solution is driving the problem. Restate it in user-visible terms.
Complexity budget
Every new concept must pay for itself against current requirements.
- Add a branch for a real behavioral variant.
- Add a type when it prevents an invalid state or names a domain concept.
- Add a helper when it makes one cohesive operation clearer or has a second real caller.
- Add a trait when multiple implementations or a required capability boundary exist now.
- Add configuration only when a caller needs to choose.
- Add a crate only when dependency direction or independent reuse requires it.
One hypothetical future does not count. A long function is not automatically wrong if it expresses one linear operation. Several short functions are not automatically clean if they scatter one concern.
Patterns to remove
Look for clusters, not isolated syntax.
Existing feature blindness
The draft creates a new path instead of composing the existing one. Search by behavior and domain vocabulary, not only by the proposed name. Prefer extending the owning Score or module over a parallel helper, harness, or CLI path.
Speculative layers
Typical signs:
- a trait with one implementation and no required boundary;
- an enum with one live variant;
- a builder for a small value constructed once;
- a wrapper that only forwards calls;
- a config option no caller sets;
- a registry, plugin seam, or generic parameter for an imagined consumer.
Delete the layer and use the concrete operation until reality supplies the second case.
Helper confetti
Tiny helpers can hide the operation's shape and force readers to jump between files. Keep a sequence together when the steps share one purpose, state, and error boundary. Split only an independently meaningful operation.
Conflated ownership
The opposite failure is one function or type owning unrelated reasons to change. Split by concern when the parts have different callers, lifecycles, trust boundaries, or tests. Do not split merely to shorten a function.
Duplicate knowledge
Two copies of the same condition, default, mapping, or deployment recipe in one trust domain are a defect. Choose one authoritative home and compose it. Validation at separate trust boundaries is not duplication.
Framework-shaped application code
Do not move code into the framework because it looks reusable. Require a real second consumer or an established domain capability. Keep product-specific policy in its deploy crate.
Flashy PR scope
Reject opportunistic redesign, broad renames, future variants, unrelated cleanup, and large documentation claims around a small behavior change. A PR should be easy to describe in terms of the problem it closes.
Tests for the implementation rather than behavior
Do not add a helper solely so it can have unit tests. Test the observable contract at the lowest reliable boundary. Avoid duplicating production logic in fixtures.
Narrative comments and names
Comments explain a constraint or reason. Names identify domain meaning. Remove comments that narrate statements and names that advertise patterns rather than the problem.
False positives
Preserve complexity when it belongs to the problem:
- protocol state machines with real failure transitions;
- retries, rollback, and idempotency required by external systems;
- duplicate validation across trusted and untrusted boundaries;
- typed wrappers that enforce a meaningful invariant;
- provider differences already exercised by supported deployments;
- deployment sequencing imposed by generated credentials or readiness.
Do not flatten these into code that is shorter but wrong. Instead, keep the complexity in one cohesive owner and make the invariant visible.
Simplification pass
After tests pass, inspect only the diff and ask:
- Can existing code replace anything added here?
- Can any new type, trait, option, helper, or module disappear?
- Is the same condition or knowledge represented twice?
- Did application policy leak into framework code?
- Did test setup reimplement production behavior?
- Can control flow become linear and local?
- Does every changed file need to be in this PR?
Rewrite when the answer exposes accidental complexity. Run verification again.
Review output
Lead with concrete findings and file references. For each issue, state:
- the user-visible or maintenance risk;
- the accidental complexity pattern;
- the smaller correction using existing code where possible.
If the implementation already matches the problem, say so. Do not invent a simplification to justify the review.