review

Files (1)

• SKILL.md

SKILL.md (first lines)

# Review

Review is evidence gathering, not rubber-stamping

## Independent review routing

For every nontrivial plan review, nontrivial review request, and nontrivial code change, use the six base angles below by default. Build a coverage map before fanout

For rare contained plan or implementation changes with understood impact, the parent may select a relevant subset and briefly explain the task-specific reason. Use all six when impact is uncertain. Independent review and final claim-bound verification remain required

An explicit deep code-quality review uses the same six angles over the wider user-approved target

Count an angle as covered only when:
- a fresh independent reviewer was explicitly assigned that exact angle, with its complete description and a distinct evidence target
- the result addresses the current approved behavior and the exact item or effective change relevant to that angle
- its conclusion and evidence remain usable
- no later correction changed that angle’s approved contract, evidence target, relevant item or effective change, conclusion, or proof

Treat uncertain, implicit, parent-only, combined-angle, stale, inaccessible, or superseded evidence as uncovered. Launch one fresh independent reviewer for each uncovered selected base angle, assigned to exactly that one angle. One reviewer cannot satisfy two uncovered base angles. Do not relaunch an angle with valid current coverage only because the overall diff changed outside the evidence it covers. When none of the selected angles has valid coverage, launch one base reviewer for each selected angle. Record the subset reason when applicable and each angle’s status, evidence boundary, and review artifact before synthesizing `PASS`, `FAIL`, or `INCONCLUSIVE`

The parent also inspects the target, validates findings, synthesizes the result, and owns every decision

Every selected reviewer receives the approved behavior, non-goals, relevant decisions, actual target/effective change, required proof and available evidence, a distinct evidence target, and a stop condition. Each base reviewer additionally receives exactly one complete assigned base angle description, its defect target, and review questions. Each specialist receives exactly one additional-surface assignment. Reviewers never edit, become writers, vote, amend the behavioral contract, authorize scope expansion, or invent findings to fill a lane

The parent may launch a reviewer at any point when its exact target is ready. Prefer background execution and keep unrelated work moving. Early coverage remains valid only while its contract, target, evidence, and conclusion remain unchanged

### Six base angles

1. **Contract, user impact, and approved scope**
   - Defect target: work that is plausible or correct in isolation but solves the wrong problem or expands scope
   - Evidence: approved requirements, non-goals, user/caller behavior, public contracts, and reached compatibility effects
   - Questions: Does the result match the approved intent? Is behavior missing or extra? Are reached user, caller, API, or compatibility consequences intentional?
   - Stop: conclude when the approved contract and every reached public consequence are accounted for

2. **Reachable correctness**
   - Defect target: behavior that fails under states the inspected producers and contracts can create
   - Evidence: real producers, call paths, types, state transitions, consumers, failure evidence, and concurrency when reached
   - Questions: Does ordinary behavior remain correct? Are state changes coherent? Does the change introduce a real regression or violate an established invariant?
   - Stop: conclude when the changed behavior and its reachable consequences are accounted for

3. **Fail-fast and defensive code**
   - Defect target: unapproved validation, fallback, retry, recovery, coercion, normalization, compatibility, sanitization, security hardening, optionality, defaults, filtering, repair, or custom error wrapping in the changed logic
   - Evidence: the approved contract, real producers, owned types and invariants, raw failure behavior, and the effective change
   - Questions: Which defensive mechanisms were newly added by this change? Which are required by the approved contract? Which unapproved new mechanisms should be removed? Does the change modify or remove existing approved defensive behavior without user approval?
   - Stop: conclude when every new defensive mechanism is required or identified for removal, and every proposed change to existing approved defensive behavior is a user choice

4. **Architecture, ownership, integration, and consumers**
   - Defect target: locally correct work placed at the wrong owner or not reached by the real system
   - Evidence: canonical sources of truth, effective runtime/discovery/activation paths, callers, consumers, dependencies, and integration seams
   - Questions: Does the effective resource win at runtime? Is this the canonical owner? Are all real consumers covered? Does the change create or broaden an abstraction, move responsibility, or choose between materially different owners without user approval?
   - Stop: conclude when ownership, reachability, integration, and consumers are established from current evidence

5. **Simplicity, maintainability, and local fit**
   - **Simplicity and cleanliness:** inspect unnecessary concepts, abstraction, duplication, indirection, wrappers, branches, weak type boundaries, AI slop, and future-change cost
   - Defect target: correct work that creates avoidable structural cost or conflicts with repository patterns
   - Evidence: the named structural surface, current code patterns, ownership boundaries, and effective diff
   - Questions: Is this the smallest coherent design? Can a concept, branch, layer, or duplicate owner be deleted without losing behavior? Does it fit local style and architecture?
   - Stop: conclude at concrete behavior-preserving findings
     - do not manufacture style comments

6. **Claim-bound tests and proof**
   - **Test and evidence quality:** inspect whether tests, live/integration/manual evidence, static checks, and artifacts prove the changed claim
   - Defect target: implementation accepted on stale, partial, non-rerunnable, wrong-boundary, or implementation-only evidence
   - Evidence: proof contract, exact commands/flows, outputs, unavailable boundaries, and current claim identity
   - Questions: Could this evidence disprove a wrong implementation? Does it reach the claimed boundary? Are operational, rollback, or readiness checks needed on this path?
   - Stop: conclude when every material claim has sufficient current evidence or an explicit unavailable boundary

### Explicit deep code-quality review

When the user asks for a deep code-quality, structure, or simplification review, use the same six base angles over the complete user-approved target

Hold the approved behavior fixed. Read beyond the diff when ownership, duplication, or integration cannot be judged from changed lines alone. Require a real location or relationship, a concrete cost, a smaller safe direction, and evidence that the direction preserves behavior or a clear statement of what remains unverified

Do not report cosmetic or formatter-only comments, speculative rewrites, generic patterns with no current need, or changes with no concrete benefit. **No findings** is valid

Do not launch a second overlapping quality-reviewer set. Increase the target depth of the relevant six angles instead

### Conditional specialists

Select fresh specialists dynamically for concrete additional questions, opportunities, or risks. Relevant surfaces can include the following, not a default checklist:
- security, privacy, or supply chain
- runtime operations, deployment, rollback, performance, capacity, or observability
- data/schema migration or persisted compatibility
- concurrency or distributed protocols
- accessibility or internationalization
- domain or regulatory correctness
- platform lifecycle or API behavior
- ML/data quality, evaluation, or fairness

Specialists are not part of the six base angles and remain outside base-angle coverage reuse

Before fanout, confirm that every selected reviewer can independently access the exact target. Supply the complete target inline, an identified readable repository or scratch path with its validity boundary, fixed Git refs and the effective diff, or an existing artifact with retrieval instructions. A reference only to visible or latest parent-session content is not a target. If the exact target cannot be supplied, return `INCONCLUSIVE` before fanout. Do not create a manifest or receipt only for this preflight

After a coherent review-fix group, choose the follow-up by effective risk:
- **Tiny mechanical fix:** no behavior, contract, reachability, or proof meaning changed. The parent inspects the final diff and runs the narrowest check
  - no child reviewer is required
- **Contained correction:** one localized behavior, correctness, or proof defect is corrected without changing a public contract, security/data boundary, shared abstraction, cross-owner behavior, or new reachable consumer. Use one fresh targeted reviewer or validator
- **Broad or high-risk correction:** architecture or ownership, public contracts, security/data boundaries, concurrency, compatibility, shared abstractions, several consumers, or a materially changed proof strategy. Reassess which base-angle coverage the correction invalidated, launch one fresh reviewer for each now-uncovered base angle, and add any fresh change-triggered specialists

Use the broader tier when classification is unclear. Continue only while a new validated required finding produces a material correction. Stop when review is clean, only user choices or rejected suggestions remain, evidence stalls, a blocker appears, or another approval is required

A final `PASS` requires current coverage of all selected base angles and every validated required finding to be fixed or explicitly deferred by the user

Automatic continuation does not select the writer. Final claim-bound verification is still required

## Review Modes

Choose the mode explicitly

### 1. Spec Compliance Review

Check whether the implementation matches the approved task/plan exactly

Flag:
- missing requirements,
- extra behavior beyond scope,
- logic at the wrong canonical owner or the wrong public API shape,
- tests that do not prove the specified behavior,
- deviations from explicit constraints

From OrestesK/pi, MIT.

More from OrestesK/pi