Leahnaya/sendou.ink
1
0
Fork 0
mirror of https://github.com/Sendouc/sendou.ink.git synced 2026-08-26 13:22:50 -05:00
Code Issues Actions 3 Packages Projects Releases Wiki Activity

Improve code review skill

Browse Source
This commit is contained in:
Kalle
2026-08-03 18:47:05 +03:00
parent cf627a61ea
commit f0521b9358
1 changed files with 254 additions and 81 deletions
Download Patch File Download Diff File Expand all files Collapse all files

335
.claude/skills/sendou-code-review/SKILL.md
View File

@@ -1,9 +1,9 @@
---
name: sendou-code-review
description: Multi-agent code review that checks the current diff from 7 angles (spec compliance, modernization, bugs, CLAUDE.md rules, abstraction reuse, security, DB query performance) and produces a unified review. Works on branch diffs vs main or staged changes.
description: Multi-agent code review that checks the current diff from multiple angles (spec compliance, modernization, bugs from two lenses, CLAUDE.md rules, abstraction reuse, security, DB query performance, test coverage, English copy proofreading) plus mechanical pre-checks, adversarially verifies every finding, and produces a unified review. Works on branch diffs vs main or uncommitted changes.
---
Review the current code changes from multiple angles using parallel sub-agents, then synthesize into a single high-quality review. If the diff contains no changes to Repository files or SQL/Kysely code, skip Agent 7 (DB Query Performance).
Review the current code changes from multiple angles using parallel sub-agents, adversarially verify each finding, then synthesize into a single high-quality review. If the diff contains no changes to Repository files or SQL/Kysely code, skip Agent 8 (DB Query Performance).
## Step 1: Determine what to review
@@ -11,39 +11,85 @@ Run these commands to figure out what to review:
```
git branch --show-current
git status --porcelain -uall
```
- If NOT on `main`: get the diff with `git diff main...HEAD` and also `git diff` for any unstaged changes on the branch. Combine them — this is the **full diff**.
- If on `main`: get staged changes with `git diff --cached`. If there are no staged changes, tell the user: "Nothing to review. Either check out a feature branch or stage some changes." and stop.
- If NOT on `main`: get the diff with `git diff main -- ':(exclude)locales/'` — this compares the working tree against main, so it covers committed, staged, AND unstaged changes in one diff.
- If on `main`: get uncommitted changes with `git diff HEAD -- ':(exclude)locales/'` (covers both staged and unstaged).
- **Translation files are excluded**: the `':(exclude)locales/'` pathspec keeps `locales/` JSON churn (mostly `i18n:sync` output) out of the review — it is noise, not reviewable code. Apply the same exclusion to every diff/stat command below.
- **But collect the English copy separately**: also run `git diff main -- 'locales/en/'` (or `git diff HEAD -- 'locales/en/'` on main), plus untracked files under `locales/en/`. This is the **copy diff** — it is NOT part of the full diff and goes only to Agent 10 (Copy Editor).
- **Untracked files**: plain diffs never show untracked files. For every untracked file in the `git status --porcelain -uall` output (these are not gitignored — git status already excludes ignored files), skip anything under `locales/`, then read the file's full content and append it to the review material clearly marked as "NEW UNTRACKED FILE: {path}". These files get reviewed like any other changed code.
- If the diff is empty and there are no untracked files, tell the user: "Nothing to review. Either check out a feature branch or make some changes." and stop.
The diff plus untracked file contents is the **full diff** referenced below.
Also run these in parallel:
- `cat .nvmrc` to get the Node.js version
- Read the `CLAUDE.md` and `AGENTS.md` files for the project rules
- `git diff main...HEAD --stat` (or `git diff --cached --stat` on main) to get the list of changed files
- `git diff main --stat -- ':(exclude)locales/'` (or `git diff HEAD --stat -- ':(exclude)locales/'` on main) to get the list of changed files
## Step 2: Collect context from the user
## Step 2: Mechanical pre-checks (no agents)
The user may have provided a GitHub issue URL or description of what the code should do as an argument. If they did, this will be used by the Spec Compliance agent. If not, the Spec Compliance agent will be skipped.
These rules are greppable and don't need an LLM. Run them yourself now; each hit becomes a **pre-confirmed finding** passed straight to the summarizer (they skip the verification stage).
## Step 3: Launch up to 7 parallel review agents
1. **Typecheck**: run `pnpm run typecheck`. If it fails on changed files, record each error as a pre-confirmed finding — the review should lead with these rather than agents flagging downstream symptoms. Continue with the review regardless.
2. **Undefined CSS variables**: for every `var(--...)` in added lines of CSS/TSX, verify the variable is defined in `app/styles/vars.css` or locally in one of the changed files. Undefined → finding.
3. **Unregistered routes**: for every new file added under a `routes/` directory (e.g. `app/features/*/routes/*.tsx`), grep `app/routes.ts` for its path. Not referenced → finding.
4. **Missing CSS module pairing**: a new `.tsx` file that defines components with non-trivial styling but has no matching `.module.css` file. Use judgement — files with no styling needs are fine.
5. **Leftover debug code**: grep added lines for `console.log`, `test.only`, `describe.only`, `it.only`.
## Step 3: Collect the spec
The user may have provided a GitHub issue URL or description of what the code should do as an argument. If they did, use it for the Spec Compliance agent.
If they did NOT, derive the intent instead of skipping:
- On a branch: run `git log main..HEAD --format="%s%n%b"` for commit messages. If a commit or the branch name references an issue (`#123`), fetch it with `gh issue view 123`. If a PR exists, fetch `gh pr view --json title,body`.
- Combine whatever you found into the spec. "Does the code do what the commits claim" is still a valuable check.
- Only skip the Spec Compliance agent if there is genuinely nothing: no user input, no commits (e.g. uncommitted changes on main), no PR.
## Step 4: Launch parallel finder agents
Launch these as parallel Agent calls. Each agent receives:
- The full diff
- The full diff (including untracked file contents)
- The list of changed files
- Access to read the full files for surrounding context
**Sharding for large diffs**: if the full diff exceeds ~2000 lines, do not warn and shrug — shard. Split the changed file list into roughly equal chunks (by line count) and run one instance of each finder agent per chunk, giving each instance its chunk's diff plus the full file list for context. Findings from all instances merge into the same pool.
**Agent budget — sharding must not explode usage on a massive branch:**
- Cap at **3 shards** no matter the diff size.
- If the diff exceeds ~4000 lines, only shard the high-signal finders (Agents 1, 3, 4, 7 — spec, both bug lenses, security). Run the rest (modernizer, CLAUDE.md, abstraction, test coverage) as a single instance each over the full diff; they degrade gracefully with size.
- Hard ceiling: **at most 20 finder agent calls** total. If the plan would exceed that, drop shards from the lowest-priority finders first (modernization, then CLAUDE.md, then abstraction).
- If the diff exceeds ~8000 lines, stop and ask the user before launching anything: proceed with the capped review (state the approximate agent count), or narrow the review to a subset they pick (e.g. specific directories/features), or review only the riskiest files (Repositories, actions/loaders, permission-related code) — recommend the last option.
**Important for all agents:**
- Only flag issues in the NEW code (lines added/changed in the diff). Do not flag pre-existing issues.
- Be specific: cite file paths and describe the exact problem and a concrete suggestion.
- If you find nothing meaningful, say so — do not manufacture issues.
- Be specific: cite file paths and line numbers, describe the exact problem and a concrete suggestion.
- If you find nothing meaningful, return an empty list — do not manufacture issues.
### Agent 1: Spec Compliance (skip if no issue/description provided)
**Structured output — every finder returns JSON**, an array of findings shaped:
```json
{
"file": "app/features/foo/FooPage.tsx",
"line": 42,
"category": "bug | security | db-performance | spec | claude-md | abstraction | modernization | test-coverage | copy",
"severity": "critical | warning | info",
"claim": "one-sentence statement of the problem",
"detail": "how it manifests / why it matters, 1-3 sentences",
"suggestion": "concrete fix"
}
```
Tell each agent to output ONLY the JSON array as its final message. Agent-specific extra fields are noted per agent below.
### Agent 1: Spec Compliance (skip only if no spec could be collected in Step 3)
```
You are reviewing code changes for spec compliance.
The user described the expected behavior as:
{user_provided_spec}
The expected behavior (from user description, commit messages, and/or linked issue/PR):
{spec}
Here is the diff:
{diff}
@@ -57,7 +103,7 @@ Read the full changed files for context. Check whether the implementation actual
Only flag real gaps between spec and implementation. Do not flag things the spec doesn't mention.
Return a list of issues, each with: file path, description of the gap, and what the spec expected.
Return ONLY a JSON array of findings: [{file, line, category: "spec", severity, claim, detail (what the spec expected vs what the code does), suggestion}]
```
### Agent 2: Modernizer
@@ -82,13 +128,36 @@ Read the full changed files for context. Check for:
Only flag cases where the modern approach is clearly better (more readable, shorter, or more performant). Do not flag stylistic preferences that are a wash.
Return a list of suggestions, each with: file path, the current code pattern, and the modern replacement.
Return ONLY a JSON array of findings: [{file, line, category: "modernization", severity: "info", claim (the current pattern), detail, suggestion (the modern replacement)}]
```
### Agent 3: Bug Finder
### Agent 3: Bug Finder — client lens
```
You are reviewing code changes for bugs.
You are reviewing code changes for bugs, focused on the CLIENT side: React components, hooks, state, data flow, and UI logic.
Here is the diff:
{diff}
Changed files: {file_list}
Read the full changed files for context. Look for (not a comprehensive list):
- Logic errors in components and hooks (wrong conditions, off-by-one, incorrect comparisons)
- Null/undefined access that could crash at render time
- State management bugs (stale closures, state derived incorrectly from props, missing dependency arrays — but note this project doesn't use useMemo/useCallback)
- Broken data flow between components (props passed but ignored, wrong prop, stale loader data)
- Form handling and optimistic UI mistakes
- Rendering bugs (wrong keys in lists, conditional rendering that hides errors)
Focus on bugs that would actually manifest in practice. Do not flag theoretical issues that are prevented by the surrounding code or type system.
Return ONLY a JSON array of findings: [{file, line, category: "bug", severity, claim, detail (how it manifests), suggestion}]
```
### Agent 4: Bug Finder — server/data lens
```
You are reviewing code changes for bugs, focused on the SERVER side: loaders, actions, Repository code, SQL, validation, and authorization-adjacent logic.
Here is the diff:
{diff}
@@ -98,19 +167,19 @@ Changed files: {file_list}
Read the full changed files for context. Look for (not a comprehensive list):
- Logic errors (wrong conditions, off-by-one, incorrect comparisons)
- Null/undefined access that could crash at runtime
- Race conditions or ordering issues
- Incorrect type assumptions
- Race conditions or ordering issues (concurrent actions, non-atomic read-then-write)
- Incorrect type assumptions at boundaries (request params, DB rows)
- Missing error handling at system boundaries (external APIs, user input)
- State management bugs (stale closures, missing dependency arrays — but note this project doesn't use useMemo/useCallback)
- Incorrect SQL queries (wrong joins, missing WHERE clauses, SQL injection)
- Broken data flow between components
- Incorrect SQL queries (wrong joins, missing WHERE clauses, wrong aggregation)
- Transactions missing where multiple writes must be atomic
- Data returned to the client that doesn't match what the component expects
Focus on bugs that would actually manifest in practice. Do not flag theoretical issues that are prevented by the surrounding code or type system.
Return a list of bugs, each with: file path, description of the bug, how it would manifest, and a suggested fix.
Return ONLY a JSON array of findings: [{file, line, category: "bug", severity, claim, detail (how it manifests), suggestion}]
```
### Agent 4: CLAUDE.md Compliance
### Agent 5: CLAUDE.md Compliance
```
You are reviewing code changes for compliance with the project's CLAUDE.md rules.
@@ -125,10 +194,10 @@ Changed files: {file_list}
Only flag clear violations. If a rule says "prefer" or "avoid", use judgement — a minor deviation in context is not a violation.
Return a list of violations, each with: file path, the rule violated (quote it), and the offending code.
Return ONLY a JSON array of findings: [{file, line, category: "claude-md", severity: "warning", claim (quote the rule violated), detail (the offending code), suggestion}]
```
### Agent 5: Abstraction Police
### Agent 6: Abstraction Police
```
You are reviewing code changes for proper reuse of existing abstractions and avoiding excessive copy-paste.
@@ -145,17 +214,17 @@ Your job is to search the codebase to check:
Use the "three strikes" rule: a small amount of duplication (2 instances) is acceptable. Three or more instances of the same pattern means it should be abstracted.
Search broadly — check `app/utils/`, `app/components/`, `app/hooks/`, and files adjacent to the changed files.
Search broadly — check `app/utils/`, `app/components/`, `app/hooks/`, `app/modules/`, and files adjacent to the changed files.
Do NOT flag:
- Simple one-liners that happen to look similar (e.g., `if (!user) return null`)
- Standard patterns that are idiomatic and don't benefit from abstraction
- Duplication that exists only in the old code (not introduced by this diff)
Return a list of issues, each with: file path, the duplicated pattern, where it already exists in the codebase (with file paths), and a suggestion for how to share it.
Return ONLY a JSON array of findings: [{file, line, category: "abstraction", severity, claim (the duplicated pattern), detail (where it already exists — cite file paths), suggestion (how to share it)}]
```
### Agent 6: Security
### Agent 7: Security
```
You are reviewing code changes for security vulnerabilities.
@@ -165,28 +234,36 @@ Here is the diff:
Changed files: {file_list}
Read the full changed files for context. This is a Remix/React Router web application with SQLite (via Kysely). Check for:
Read the full changed files for context. This is a Remix/React Router web application with SQLite (via Kysely).
Project auth conventions — check against these real patterns, not generic boilerplate:
- Server-side auth: `requireUser()` from `app/features/auth/core/user.server.ts` returns the authenticated user or throws.
- Role/permission guards: `requireRole(role)` and `requirePermission(...)` from `app/modules/permissions/guards.server.ts`. Helpers like `isAdmin`/`isStaff` live in `app/modules/permissions/utils.ts`.
- Client-side hooks `useHasRole`/`useHasPermission` (`app/modules/permissions/hooks.ts`) are for UI display only — they are NOT security boundaries. Every mutation (action) and sensitive loader must enforce auth server-side.
Check for:
- Loaders/actions that read or mutate data without the appropriate `requireUser`/`requireRole`/`requirePermission` guard, or that guard with the wrong scope (IDOR — can user A access or mutate user B's data by changing an id?)
- SQL injection (even with Kysely, check for raw queries or string interpolation in SQL)
- XSS (unescaped user input rendered as HTML, dangerouslySetInnerHTML with user data)
- CSRF vulnerabilities
- Authorization bypasses (missing permission checks, IDOR — can user A access user B's data?)
- Sensitive data exposure (tokens, passwords, or PII in logs, responses, or client-side code)
- Sensitive data exposure (tokens, passwords, or PII in logs, loader responses, or client-side code — remember everything a loader returns is visible to the client)
- Path traversal (user-controlled file paths)
- Insecure redirects (open redirect via user-controlled URLs)
- Missing input validation at system boundaries
Focus on real, exploitable vulnerabilities in the new code. Do not flag:
- General best-practice advice that isn't a concrete vulnerability
- Issues in frameworks/libraries (Remix, React) that handle security themselves
- Issues in frameworks/libraries (Remix, React) that handle security themselves (e.g. React's default escaping, framework CSRF handling)
- Pre-existing issues not introduced by this diff
Return a list of vulnerabilities, each with: file path, vulnerability type (e.g., "SQL Injection"), description, attack scenario, and suggested fix.
Return ONLY a JSON array of findings: [{file, line, category: "security", severity, claim (vulnerability type + summary), detail (attack scenario), suggestion}]
```
### Agent 7: DB Query Performance (skip if no Repository/Kysely changes in diff)
### Agent 8: DB Query Performance (skip if no Repository/Kysely changes in diff)
```
You are reviewing code changes for database query performance. This is a Remix/React Router web app using SQLite via Kysely. The dev database is at `db.sqlite3`.
You are reviewing code changes for database query performance. This is a Remix/React Router web app using SQLite via Kysely.
IMPORTANT: run all analysis against `db-prod.sqlite3` — a copy of the production database (~2GB, safe to experiment with). Do NOT use `db.sqlite3` if `db-prod.sqlite3` exists (the dev db is tiny, so its query plans and table sizes are meaningless for performance analysis).
Here is the diff:
{diff}
@@ -197,82 +274,178 @@ Your job:
1. **Identify new or changed DB queries** in the diff. These live in `*Repository.server.ts` files and use Kysely. Read the full changed Repository files for context.
2. **For each query**, do the following:
2. **Get the real SQL** — do not mentally compile Kysely. Write a small throwaway script at `scripts/tmp-review-compile.ts` (it must live inside the project so vite-node resolves the `~` alias; delete it when you are done) that rebuilds the query with the project's `db` instance and prints the compiled SQL:
a. **Run EXPLAIN QUERY PLAN** against the dev database (`db.sqlite3`) using the Bash tool:
```ts
import { db } from "~/db/sql";
const compiled = db
.selectFrom(/* ...rebuild the query from the Repository code... */)
.compile();
console.log(compiled.sql);
console.log(compiled.parameters);
```
Run it with: `DB_PATH=db-prod.sqlite3 VITE_PROD_MODE=true pnpm vite-node scripts/tmp-review-compile.ts` (this matches how the project's own `bench:db` script runs). Substitute realistic parameter values from the printed parameters.
3. **For each query**:
a. **Run EXPLAIN QUERY PLAN** against the prod copy:
```
sqlite3 db.sqlite3 "EXPLAIN QUERY PLAN <the SQL query>"
sqlite3 db-prod.sqlite3 "EXPLAIN QUERY PLAN <the compiled SQL with parameters substituted>"
```
To get the raw SQL from Kysely, read the query and mentally compile it. Substitute realistic placeholder values for any parameters.
b. **Check for missing indexes**: Look at the EXPLAIN output for "SCAN TABLE" (full table scan) vs "SEARCH TABLE ... USING INDEX" or "USING COVERING INDEX". A SCAN on a large table in a hot path is a red flag.
b. **Measure table sizes — do not guess**: `sqlite3 db-prod.sqlite3 "SELECT COUNT(*) FROM <table>"` for every table the query touches.
c. **Check existing indexes**: Run `sqlite3 db.sqlite3 ".indexes <table_name>"` and `sqlite3 db.sqlite3 "PRAGMA index_info(<index_name>)"` to see what indexes exist.
c. **Check for missing indexes**: Look at the EXPLAIN output for "SCAN <table>" (full table scan) vs "SEARCH <table> ... USING INDEX" or "USING COVERING INDEX". A SCAN on a large table in a hot path is a red flag. Check existing indexes with `sqlite3 db-prod.sqlite3 ".indexes <table_name>"` and `PRAGMA index_info(<index_name>)`.
d. **Assess query context** — reason about:
- **Table size**: Is this a table with thousands/millions of rows (e.g., SplatoonPlayer, Build, GroupMatch) or a small config-like table (e.g., CalendarEventTag, TournamentBadgeOwner)?
- **Call frequency**: Is this query in a hot path (page loader hit on every page view, API called frequently) or a cold path (admin action, background routine, rare user action)?
- **N+1 patterns**: Is the query called inside a loop when it could be batched?
Use the route file or caller to determine how the Repository function is invoked.
d. **Assess call frequency**: Is this query in a hot path (page loader hit on every page view, API called frequently) or a cold path (admin action, background routine, rare user action)? Use the route file or caller to determine how the Repository function is invoked. Also check for N+1 patterns — a query called inside a loop that could be batched.
3. **Severity assessment**: Weight your findings by impact:
- **Critical**: Full table scan on a large table in a hot path, or N+1 query pattern
- **Warning**: Full table scan on a medium table, or missing index on a frequently-filtered column
- **Info**: Scan on a small table or infrequent query — note it but don't flag as a problem
4. **Severity**:
- **critical**: Full table scan on a large table (measured, not guessed) in a hot path, or N+1 query pattern
- **warning**: Full table scan on a medium table, or missing index on a frequently-filtered column
- **info**: Scan on a small table or infrequent query — note it but don't flag as a problem
4. **Do NOT flag**:
5. **Do NOT flag**:
- Queries that already use appropriate indexes
- Scans on tiny tables (< ~100 rows) that are accessed infrequently
- Scans on tiny tables (< ~100 measured rows) that are accessed infrequently
- Pre-existing queries not changed in this diff
Return a list of findings, each with: file path, the query (or a description of it), EXPLAIN QUERY PLAN output, table size assessment (small/medium/large), call frequency assessment (hot/warm/cold), severity (critical/warning/info), and a concrete suggestion if action is needed (e.g., "add index on X(Y)" or "batch these N queries into one with WHERE IN").
Return ONLY a JSON array of findings: [{file, line, category: "db-performance", severity, claim, detail (EXPLAIN output + measured row counts + call frequency assessment), suggestion (e.g. "add index on X(Y)" or "batch these N queries into one with WHERE IN")}]
```
## Step 4: Summarize
After all agents complete, launch a single summarizer agent that receives ALL agent outputs.
### Agent 9: Test Coverage
```
You are the final reviewer synthesizing code review feedback from up to 7 specialized agents.
You are reviewing code changes for test coverage gaps. This project uses Vitest for unit/browser tests and Playwright for e2e tests.
Here are their findings:
Here is the diff:
{diff}
{all_agent_outputs}
Changed files: {file_list}
Compare what logic changed against what tests changed:
1. **Changed pure-logic Modules** (files imported as `* as Module`, typically with JSDoc'd functions): do they have unit tests (`*.test.ts` alongside or in a nearby location)? Were the tests updated when the logic changed?
2. **New Repository READ functions**: the project convention is that these get benchmark cases in `scripts/benchmark-db` (run via `pnpm run bench:db`). Flag new read functions with no benchmark case.
3. **New routes or significant new user flows**: is there any e2e spec covering them? Check the e2e test directory for related specs.
4. **Bug-fix-shaped changes** (a conditional tightened, an edge case handled): is there a regression test proving the fix?
Do NOT flag:
- Trivial or purely presentational changes (styling, copy, layout)
- Code that is impractical to unit test and already covered transitively by e2e
- Pre-existing untested code not touched by this diff
Return ONLY a JSON array of findings: [{file, line, category: "test-coverage", severity, claim (what changed without coverage), detail, suggestion (what kind of test to add and where)}]
```
### Agent 10: Copy Editor (skip if the copy diff is empty AND the full diff adds no English user-facing strings)
This agent receives the **copy diff** (the `locales/en/` changes collected in Step 1) instead of the full diff, plus any added English user-facing strings from the full diff (JSX text, aria-labels, error messages, notification texts). It is never sharded.
```
You are proofreading English texts that will be shown to users of sendou.ink, a competitive Splatoon community website.
New/changed English strings from translation files (locales/en/*.json):
{copy_diff}
New English user-facing strings found in code (if any):
{code_strings}
Only review ADDED or CHANGED strings, not pre-existing ones. Check each for:
- Typos and misspellings
- Grammar mistakes (subject-verb agreement, articles, tense)
- Broken i18next interpolation: mismatched or misspelled `{{variable}}` placeholders, wrong plural key forms — compare against how the key is used in the code if unsure
- Unclear or confusing phrasing — would a user reading this out of context understand what to do?
- Unnecessarily technical or jargon-y wording where plainer language would be friendlier (e.g. "Invalid input" vs telling the user what to fix)
- Inconsistent terminology or capitalization vs the rest of the site — read the existing `locales/en/*.json` files to learn the established terms (e.g. how features like SendouQ, scrims, tournaments are referred to) and match them
Severity: typos, grammar mistakes, and broken interpolation are "warning". Clarity/friendliness rewording is "info".
Do NOT flag:
- Established community/game terminology that only looks unusual (Splatoon terms, ability names, mode names)
- Intentionally short labels where brevity fits the UI
- Style preferences that are a wash — only suggest a rewording if it is clearly better
Return ONLY a JSON array of findings: [{file, line, category: "copy", severity, claim (the problematic text, quoted), detail, suggestion (the corrected/improved text, quoted)}]
```
## Step 5: Adversarial verification
Collect all findings from the finder agents into one pool (parse their JSON outputs). **Do not verify** mechanical pre-check findings from Step 2 (already confirmed by tooling), db-performance findings that include measured EXPLAIN/COUNT evidence (already confirmed by measurement), or copy findings (proofreading is judgement, not a falsifiable claim — the summarizer filters these instead). Everything else gets verified.
For each remaining finding, launch a verifier agent. Run them in parallel. If there are more than ~12 findings, batch them by file so each verifier handles 2-4 findings from the same file. Hard ceiling: **at most 10 verifier agent calls** — with more findings than fits, grow the batch size rather than the agent count (a verifier can handle up to ~6 findings if they share files).
```
You are a skeptical senior engineer. Your job is to REFUTE the following code review finding(s). Assume each is a false positive until the code proves otherwise.
Finding(s):
{finding_json}
Relevant diff hunks:
{diff_excerpt}
Read the actual files involved. For each finding, check:
- Does the claimed problem actually exist in the code as written?
- Is it prevented by the surrounding code, the type system, validation upstream, or the framework?
- For bugs: can you trace a concrete input/state that triggers it?
- For abstraction findings: does the cited existing code actually fit this use case?
- For spec findings: does the code actually diverge from the spec, or did the finder misread one of them?
- Is the finding about NEW code in the diff, or a pre-existing issue?
If uncertain, default to refuted — a review's value depends on zero false positives.
Return ONLY a JSON array, one entry per finding: [{claim: "<copy of the finding's claim>", verdict: "confirmed" | "refuted", reason: "1-2 sentences"}]
```
Discard refuted findings. Confirmed findings (plus the exempted pre-checked/measured ones) proceed to the summarizer.
## Step 6: Summarize
Launch a single summarizer agent that receives all surviving findings.
```
You are the final reviewer synthesizing verified code review findings from multiple specialized agents plus mechanical pre-checks.
Here are the surviving findings (all have passed adversarial verification, mechanical checking, or direct measurement):
{surviving_findings_json}
Your job:
1. **Deduplicate**: Multiple agents may flag the same issue from different angles. Merge these into a single finding.
2. **Filter**: Remove low-quality suggestions that are:
- Nitpicks that wouldn't matter in practice
- False positives or theoretical issues unlikely to occur
- Suggestions that would make the code worse or more complex
- Pre-existing issues not introduced by the diff
3. **Prioritize** using this order: Security > Bugs > DB Query Performance (critical/warning only) > Spec Violations > Abstraction Issues > CLAUDE.md Violations > Modernization Suggestions > DB Query Performance (info)
4. **Format** the output as a single cohesive review
1. **Deduplicate**: Multiple agents may flag the same issue from different angles. Merge these into a single finding, keeping the best explanation.
2. **Filter**: Even verified findings can be nitpicks. Remove suggestions that wouldn't matter in practice or would make the code worse or more complex.
3. **Prioritize** using this order: Typecheck failures > Security > Bugs > DB Query Performance (critical/warning only) > Spec Violations > Test Coverage > Abstraction Issues > CLAUDE.md Violations > Copy (warning: typos/grammar/broken interpolation) > Modernization Suggestions > Copy (info: clarity rewordings) > DB Query Performance (info)
Output format:
Return ONLY a JSON array of the final findings, in priority order, each shaped: {file, line, category, severity, claim, detail, suggestion}
```
## Step 7: Report
Report the summarizer's final findings with the **ReportFindings tool** (you, the main agent, call it — not a subagent): one call, findings ranked most-severe first, each with `file`, `line`, `summary` (claim + suggestion), `short_summary`, `category`, `failure_scenario` (from detail), and `verdict: "CONFIRMED"` for findings that passed verification or measurement, `"PLAUSIBLE"` otherwise. Do not also print the findings list as text — after the ReportFindings call, write only a short prose wrap-up (issue count, overall shape of the review, anything notable like sharding or skipped agents).
If the ReportFindings tool is not available in the session, fall back to printing this format instead:
### Code Review
**{N} issues found** (or "No issues found — looks good!" if none survive filtering)
For each surviving issue, in priority order:
For each finding, in priority order:
**{priority_number}. [{category}] {brief title}**
`{file_path}`
`{file_path}:{line}`
{description — 1-3 sentences explaining the problem and a concrete suggestion}
---
At the end, if there were modernization suggestions that survived filtering, group them under a separate "Suggestions" section (these are nice-to-haves, not blockers).
```
At the end, group surviving modernization suggestions and info-level copy rewordings under a separate "Suggestions" section (these are nice-to-haves, not blockers).
## Important notes
- Use `subagent_type: "Explore"` for Agent 5 (Abstraction Police) since it needs to search the codebase broadly
- Use `subagent_type: "general-purpose"` for Agent 7 (DB Query Performance) since it needs to run sqlite3 commands via Bash
- Use `subagent_type: "general-purpose"` for the other agents
- Use `model: "sonnet"` for agents 1-7 and `model: "opus"` for the summarizer
- Use `subagent_type: "Explore"` for Agent 6 (Abstraction Police) since it needs to search the codebase broadly
- Use `subagent_type: "general-purpose"` for Agent 8 (DB Query Performance) since it needs to run sqlite3 commands and vite-node scripts via Bash
- Use `subagent_type: "general-purpose"` for the other finder agents, the verifiers, and the summarizer
- Use `model: "sonnet"` for finder and verifier agents and `model: "opus"` for the summarizer
- Pass the actual diff content and file list to each agent — do not tell them to run git commands themselves
- If the diff is very large (>2000 lines), mention this to the user and note that the review may miss some issues
- Present the summarizer's output directly to the user as the final review
- If a finder agent returns malformed JSON, salvage what you can by reading its output; do not re-run it
- Present the review via ReportFindings as described in Step 7