code-review-team

Files (11)

• SKILL.md
• consolidation-template.md · docs/
• coverage.schema.json · schema/
• finding.schema.json · schema/
• api-contract-reviewer.md · specialists/
• correctness-reviewer.md · specialists/
• design-reviewer.md · specialists/
• maintainability-reviewer.md · specialists/
• performance-reviewer.md · specialists/
• security-reviewer.md · specialists/
• testing-reviewer.md · specialists/

SKILL.md (first lines)

# Multi-Agent Code Review Team — Orchestration

**What this optimizes for:** recall on real, provable defects — not exhaustive coverage of every stylistic nit, and not raw finding count. A finding only earns a place in the final report if it survives independent adversarial verification (Step 4.5) against a category-appropriate proof standard; anything that can't be confirmed that way is dropped rather than shown as a maybe. "Done" means every specialist's scope was covered (or explicitly logged as not-checked), every surviving finding is validator-confirmed, and the merge recommendation reflects only what actually held up — not what was merely raised.

Follow these steps. Review-only. Never apply fixes.

## Asking the user

This rule covers every question this skill asks the user: a confirmation, a choice, a missing value, a numbered decision list, or a stop-and-ask. It applies even where a later step says "ask the user" or "ask exactly once". The user may watch the decisions pane rather than the chat, so a question that only goes to chat can go unanswered.

- **When `mcp__decision-tracker__decision` is in your tool list** (loaded, or named as a deferred tool; load a deferred one with `ToolSearch` first), log each question before you ask it:
  1. Call it with `action: "open"`, the question (ending in `?`), and `options` when the answers are a closed set. Set `required: false` when the work can go on without an answer. A numbered list of questions opens one decision per item. Keep each returned `id`.
  2. Ask the question in chat as this skill says. The open call does not replace the chat question.
  3. When the user answers, in chat or as a `Decision Dn: <answer>` message from the plugin, call the tool with `action: "close"`, the `id` and the answer.
  4. When the change the answer asked for is in place, call it with `action: "implement"`, the `id` and a `path:line`.
- **When the tool is absent**, ask in chat only.
- An `AskUserQuestion` call is logged on its own. Do not open it again.
- This rule changes where a question is logged. It does not change when this skill asks, or how many times.

## Step 0: Pre-run confirmation

Never spawn specialists without showing this block (unless `--yes` / `auto-approve` is passed).

### 0.1 Parse arguments

- **Target path** — absolute path to the repo or directory to review. If missing, ask: *"What repo should I review? (absolute path)"* Verify it exists and is readable.
- **Scope** — one of:
  - `all` — full tree
  - `<subdir>` — a subdirectory, e.g. `src/api/`
  - `diff vs main` (or `diff vs <branch>`) — only files changed on current branch vs target branch
  If missing, ask: *"What scope? Full tree, a subdirectory, or diff vs main?"*
- **Flags:**
  - `--yes` / `-y` — skip confirmation block. Default: confirm.
  - `--lite` / `-l` — run only correctness, design, performance, security specialists (4-agent subset). Auto-activated when scope is a diff with ≤25 changed files.
  - `--full` — force a fresh baseline scan, ignoring any prior run history for this target (see Step 0.1a). Without this flag, a target with prior history runs in re-review mode automatically.

### 0.1a Detect prior run (re-review mode)

Every run writes its artifacts under a run-scoped directory, `RUN_DIR = <TARGET>/.planning/code-review/runs/<run_id>/` (`run_id` = an ISO-timestamp-derived slug), so a later run never overwrites an earlier one. Runs accumulate a top-level ledger at `<TARGET>/.planning/code-review/known-findings.jsonl` — one record per confirmed `root_issue` ever reported for this target (`root_issue`, `title`, `severity`, `first_seen_run_id`, `first_seen_date`, `file`, `evidence_snippet` — the latter two exist solely so a later run can cheaply re-verify the issue is still actually present before reporting it as "still open"; see Step 4's ledger-append and Step 5's still-open re-verification).

1. Check whether `<TARGET>/.planning/code-review/known-findings.jsonl` exists.
2. **Exists, and `--full` was not passed** → `re_review_mode = true`. This run will skip `root_issue`s already in the ledger and suppress all `nit`-severity findings from the report (Step 4.5's re-review filter, `re_review_mode` branch). Announce this in the Step 0.7 confirmation block.
3. **Missing, or `--full` was passed** → `re_review_mode = false`. This is a baseline scan; every validator-confirmed finding is eligible for the report, and the ledger is (re)built from this run's results.

All `RUN_DIR`-relative paths referenced in Steps 0.9 through 5 below resolve against the `RUN_DIR` computed here — the ledger itself is the one exception, always written at the top-level `<TARGET>/.planning/code-review/` regardless of `run_id`.

### 0.2 Enumerate scope

Use git-aware commands (fall back to `find` if not a git repo):

```bash
cd "<TARGET>"
if [[ "<SCOPE>" == "all" ]]; then
  FILE_COUNT=$(git ls-files 2>/dev/null | wc -l || find . -type f | wc -l)
  LINE_COUNT=$(git ls-files 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
elif [[ "<SCOPE>" == diff* ]]; then
  BASE="${SCOPE#diff vs }"; BASE="${BASE:-main}"
  FILES=$(git diff --name-only "$BASE"...HEAD)
  FILE_COUNT=$(echo "$FILES" | grep -c .)
  LINE_COUNT=$(echo "$FILES" | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
  # Auto-lite: activate if FILE_COUNT ≤ 25 and --lite not explicitly passed
else
  FILE_COUNT=$(git ls-files "<SCOPE>" 2>/dev/null | wc -l || find "<SCOPE>" -type f | wc -l)
  LINE_COUNT=$(git ls-files "<SCOPE>" 2>/dev/null | xargs wc -l 2>/dev/null | tail -1 | awk '{print $1}')
fi
```

Cap `LINE_COUNT` at 500k — if larger, warn and suggest narrowing scope.

### 0.3 Detect language signals

Read cheapest manifests: `README.md` (top 30 lines), `package.json`, `pyproject.toml` / `setup.py` / `requirements.txt`, `go.mod`, `Cargo.toml`, `Gemfile`, `pom.xml` / `build.gradle`. Two or three reads is enough. Identify primary languages (Python, TypeScript/JavaScript, Go, Rust, Java, Ruby, etc.) — used only to select pre-pass linters and specialise the specialist briefs.

### 0.4 Select roster

**Full roster (default):** all 7 specialists
- `correctness-reviewer` (loop-until-dry consensus, 2-8 passes — Step 3a)
- `design-reviewer`
- `performance-reviewer` (loop-until-dry consensus, 2-8 passes — Step 3a)
- `maintainability-reviewer`
- `testing-reviewer`
- `api-contract-reviewer`
- `security-reviewer`

**Lite roster** (`--lite` or diff ≤ 25 files):
- `correctness-reviewer` (loop-until-dry consensus, 2-8 passes — Step 3a)
- `design-reviewer`
- `performance-reviewer` (loop-until-dry consensus, 2-8 passes — Step 3a)
- `security-reviewer`

`security-reviewer` is in both rosters by design — it's not opt-in. A missed security bug outweighs the cost saved by cutting it from lite mode, the same reasoning that keeps it out of the `drop-specialist` fast path being a good idea (it can still be dropped explicitly via `drop-specialist security-reviewer` in Step 0.8 if the user insists, but it's never cut automatically).

### 0.5 Detect pre-pass linter availability

Run `command -v <tool>` for each linter. Flag available vs missing. Missing linters reduce pre-pass coverage but don't block the run.

| Tool | Language | Feeds specialist |
|---|---|---|
| `ruff` | Python | maintainability-reviewer, correctness-reviewer |
| `pylint` | Python | design-reviewer, correctness-reviewer |
| `eslint` | JS/TS | maintainability-reviewer, correctness-reviewer |
| `biome` | JS/TS/JSON | maintainability-reviewer |
| `golangci-lint` | Go | correctness-reviewer, design-reviewer |
| `clippy` (`cargo clippy`) | Rust | correctness-reviewer, design-reviewer |
| `rubocop` | Ruby | maintainability-reviewer |
| `shellcheck` | Shell | correctness-reviewer |

### 0.6 Estimate runtime + tokens + cost

- **Input tokens per specialist:** `~min(35k, 25 + 0.015 × LINE_COUNT)` k tokens
- **Output tokens per specialist:** `~5–8k`
- **Total tokens:** `correctness-reviewer` and `performance-reviewer` each run as a loop-until-dry consensus fan-out (Step 3a, `MIN_PASSES = 2` .. `MAX_PASSES = 8`), not a single pass. Since the actual pass count (`N`) isn't known until the loop terminates, estimate with `N` at its expected midpoint (`~4`, i.e. `N-1 = 3` extra specialist-equivalents each) for a planning estimate: `Total tokens ≈ (N_specialists + 6) × (input + output)` — the `+6` accounts for the two specialists at ~3 extra specialist-equivalents each (2 × 3 = 6); worst case `(N_specialists + 14) × (input + output)` if both run the full 8-pass cap (2 × 7 = 14). The Step 4.5 validator adds roughly one more agent call per surviving minor/nit finding group after consolidation, and three calls per surviving critical/major group (3-validator panel) — this can't be sized until findings exist, so note it as "+validator passes (post-consolidation, ~1 call per minor/nit group, ~3 calls per critical/major group)" rather than folding it into this estimate.
- **Runtime:** longest-path specialist ≈ 2–3 min baseline; range = `longest × 1.1` to `longest × 1.5`. The two consensus specialists run their first 2 passes concurrently, then any further passes one at a time until dry or capped — so wall-clock adds roughly one to a few specialists' worth of time depending on how quickly they dry up, not a fixed multiple — plus a consolidation/tally step afterward (~30s).
- **Cost**: look up the current model tier's per-MTok input/output pricing at run time (e.g. via the `claude-api` skill's model catalog or the Models API), then compute `(input × price_in + output × price_out) / 1_000_000`, using the `(N_specialists + 6)` midpoint token total above (or `+14` worst case), plus the validator caveat. Print as `$X.XX` along with the pricing assumption used (model name + rate). Don't hardcode a specific generation's numbers here — they age out every model release.

### 0.7 Render confirmation block

```
╭─ code-review-team ─────────────────────────────────────────╮

From tbeack/fsad_playbook, MIT.

More from tbeack/fsad_playbook