reviewing-code

Files (50)

• SKILL.md
• evals.json · evals/
• feat-session-sort.md · evals/fixtures/phasewave/scenario-1-clean/.opencode/plans/
• AGENTS.md · evals/fixtures/phasewave/scenario-1-clean/
• change.diff · evals/fixtures/phasewave/scenario-1-clean/
• expected-hashes.json · evals/fixtures/phasewave/scenario-1-clean/
• package.json · evals/fixtures/phasewave/scenario-1-clean/
• verify-review.js · evals/fixtures/phasewave/scenario-1-clean/scripts/
• session-display.js · evals/fixtures/phasewave/scenario-1-clean/src/lib/
• session-list.js · evals/fixtures/phasewave/scenario-1-clean/src/lib/
• session-sort.test.js · evals/fixtures/phasewave/scenario-1-clean/tests/
• feat-offline-queue.md · evals/fixtures/phasewave/scenario-2-scope-breach/.opencode/plans/
• AGENTS.md · evals/fixtures/phasewave/scenario-2-scope-breach/
• change.diff · evals/fixtures/phasewave/scenario-2-scope-breach/
• schema.sql · evals/fixtures/phasewave/scenario-2-scope-breach/db/
• expected-hashes.json · evals/fixtures/phasewave/scenario-2-scope-breach/
• package.json · evals/fixtures/phasewave/scenario-2-scope-breach/
• verify-review.js · evals/fixtures/phasewave/scenario-2-scope-breach/scripts/
• offline-queue.ts · evals/fixtures/phasewave/scenario-2-scope-breach/src/lib/
• focus-timer.original.ts · evals/fixtures/phasewave/scenario-2-scope-breach/src/stores/
• focus-timer.ts · evals/fixtures/phasewave/scenario-2-scope-breach/src/stores/
• database.types.ts · evals/fixtures/phasewave/scenario-2-scope-breach/types/
• feat-session-sort.md · evals/fixtures/phasewave/scenario-3-weakened-net/.opencode/plans/
• AGENTS.md · evals/fixtures/phasewave/scenario-3-weakened-net/
• change.diff · evals/fixtures/phasewave/scenario-3-weakened-net/
• expected-hashes.json · evals/fixtures/phasewave/scenario-3-weakened-net/
• package.json · evals/fixtures/phasewave/scenario-3-weakened-net/
• verify-review.js · evals/fixtures/phasewave/scenario-3-weakened-net/scripts/
• session-list.js · evals/fixtures/phasewave/scenario-3-weakened-net/src/lib/
• session-sort.test.js · evals/fixtures/phasewave/scenario-3-weakened-net/tests/
• session-sort.test.js.original · evals/fixtures/phasewave/scenario-3-weakened-net/tests/
• feat-session-sort.md · evals/fixtures/phasewave/scenario-4-design-defect/.opencode/plans/
• AGENTS.md · evals/fixtures/phasewave/scenario-4-design-defect/
• change.diff · evals/fixtures/phasewave/scenario-4-design-defect/
• expected-hashes.json · evals/fixtures/phasewave/scenario-4-design-defect/
• package.json · evals/fixtures/phasewave/scenario-4-design-defect/
• verify-review.js · evals/fixtures/phasewave/scenario-4-design-defect/scripts/
• session-display.js · evals/fixtures/phasewave/scenario-4-design-defect/src/lib/
• session-list.js · evals/fixtures/phasewave/scenario-4-design-defect/src/lib/
• session-sort.test.js · evals/fixtures/phasewave/scenario-4-design-defect/tests/
• feat-shared-sessions.md · evals/fixtures/phasewave/scenario-5-rls/.opencode/plans/
• AGENTS.md · evals/fixtures/phasewave/scenario-5-rls/
• change.diff · evals/fixtures/phasewave/scenario-5-rls/
• schema.sql · evals/fixtures/phasewave/scenario-5-rls/db/
• expected-hashes.json · evals/fixtures/phasewave/scenario-5-rls/
• package.json · evals/fixtures/phasewave/scenario-5-rls/
• verify-review.js · evals/fixtures/phasewave/scenario-5-rls/scripts/
• sessions.ts · evals/fixtures/phasewave/scenario-5-rls/src/stores/
• database.types.ts · evals/fixtures/phasewave/scenario-5-rls/types/
• vue-supabase.md · references/stacks/

SKILL.md (first lines)

# Reviewing Code

The verdict discipline of the core loop — the final stage of triage →
design → implement → verify → **review**. It consumes what an
`implementing-features` pass hands off (a diff/changeset plus, when
present, the plan artifact that authorized it) and produces a verdict
with actionable findings. It is **read-only on the code**: it diagnoses,
it never patches. CI/CD workflow changes are in scope too: the review
checks a workflow against a `designing-cicd` plan and flags secret /
supply-chain posture per `securing-ci` (those two are the producer and the
security-discipline siblings). A review is **done** when every changed hunk has been
assessed, findings are classified by severity, each finding cites a
location + a concrete reason, and a verdict is issued. "Looks fine" is
not a review — a finding a third party cannot act on is not a finding.

This skill is the **consumer** side of two sibling contracts: the plan
artifact (defined in
[plan-format.md in designing-architecture](../designing-architecture/references/plan-format.md)
— the plan + ACs the review checks the diff against; link it, never
duplicate it) and the implement handoff (defined in
[../implementing-features/SKILL.md](../implementing-features/SKILL.md) —
files-in-scope, `## History` deviations, follow-ups, AC status; the
review consumes exactly this). Scope-discipline is a first-class review
criterion: an out-of-scope edit with no `## History` deviation is the
auditability breach the implement skill exists to prevent, and the
review is where it is caught.

Relationship to the council: `agents/council.md` (shipped) runs
  security/performance/ux/architecture/product lenses on the **Go lane** (one
  family per seat; free-tier models cannot serve a nested subagent). This skill
  is the **single-reviewer** discipline — one reviewer, one verdict, against a
  plan. Validate any delegated lens result against
  `reference/delegated-result-contract.md` (`council-lens`): an **empty or
  malformed** result is a **failed run**, never clean; a **transient** failure
  is retried, a **deterministic** one is retried once then the vehicle is
  switched or the work taken over inline; the outcome is **recorded**. Note the
  relationship; do not absorb the council.

## The review pass

Copy this checklist and check off items as you complete them.

```
Review Progress:
- [ ] 1. Resolve the change-under-review (the diff) + the plan (if any)
- [ ] 2. Detect stack & load the matching review-time stack reference
- [ ] 3. Plan-conformance sweep (when a plan is in play)
- [ ] 4. Inspect the diff in priority order (the checklist below — includes
      doc-completeness for a merging PR)
- [ ] 5. Classify findings by severity
- [ ] 6. Issue the verdict
- [ ] 7. Write REVIEW.md at the repo root; STOP
```

### Step 1 — Resolve the diff and the plan

A review starts from a **diff/changeset** plus, when present, the plan
artifact that authorized it. Resolve the diff first; do not review from
prose. The review unit is the PR: implement passes deliver a branch +
PR per `reference/git-workflow.md`, and this review is the gate that
precedes the (user-initiated) merge to protected `main`.

- **Staged diff** → `git diff --staged` (or the harness equivalent).
- **A branch range** → `git diff <base>...HEAD` the user named; if the
  base is ambiguous, ask for the merge-base / PR base.
- **An implement pass's reported changes** → the files the pass lists in
  its handoff; reconstruct the diff via `git diff` against the pass's
  starting point, or read the reported files directly if no VCS range is
  available.
- **No diff is resolvable** (the user says "review my changes" but no
  staged files, no branch range, no file list) → **ask how to obtain it**
  (git range, staged, file paths). Do NOT guess a diff and do NOT review
  from prose — a review without a diff is not a review.

When a plan is in play, read it **before** inspecting the diff: the
frontmatter `status` (a `draft` plan under review is a finding — the
change was built against an un-approved contract), the `## Goal /
Approach`, every `## Acceptance Criteria`, `## Files to Modify`,
`## Scope` (Included + Excluded), `## Schema / Type Impacts`, and
`## Verification`. The plan is the contract the diff is checked against.
When no plan is in play (a standalone diff with no plan artifact), skip
Step 3 and proceed to Step 4 — review against engineering quality alone,
and note in REVIEW.md that no plan-conformance sweep was possible.

### Step 2 — Detect stack & load the matching review-time reference

Identify the project stack from the rules file (`AGENTS.md` /
`.opencode/agents.md`). If this skill bundles a matching stack reference
under `references/stacks/` (resolved against **this skill's own
directory** — not the project's), read it now and apply its review-time
concerns. If none exists, proceed generically and flag the gap to the
user in REVIEW.md (a missing stack reference is a finding, not a silent
"I'll improvise").

Available stack references:

- vue-supabase → [references/stacks/vue-supabase.md](references/stacks/vue-supabase.md)
  Read when the project is Vue 3 + Pinia + Supabase (Postgres + Auth +
  RLS) with Vuetify 3 + Playwright e2e. Applies review-time checks for
  RLS-policy widening on schema/policy diffs, generated-types never
  hand-edited (flag a diff that touches `types/database.types.ts` by
  hand), Vuetify semantic variables vs raw CSS, Pinia store pattern
  conformance, and e2e selector/timeout discipline in changed specs.

### Step 3 — Plan-conformance sweep (when a plan is in play)

This is the first-class check. For every hunk in the diff:

1. **Map it to `Files to Modify`.** A hunk in a file not on the list is
   out of scope. Check the plan's `## History` and `### Follow-ups` for
   a recorded deviation: a mechanical deviation (file moved/renamed,
   path alias corrected) recorded in History is legitimate; an
   **unrecorded** out-of-scope edit is a finding (see Severity → blocker).
   An out-of-scope edit recorded as a follow-up is a minor (the process
   held; the follow-up is honest).
2. **Check it against `Scope → Included`.** The change should deliver
   what Included says; a hunk that delivers something else is a finding.
3. **Check it against `Scope → Excluded`.** A hunk that delivers an
   Excluded item is a finding regardless of recording — Excluded is the

From hierovision/ai-framework, MIT.

More from hierovision/ai-framework