Conduct 5-facet code review covering quality, security, conventions, tests, and requirements with P1/P2/P3 finding synthesis, deduplication by file:line, and requirements compliance mapping. Use when reviewing code changes or pull requests.
From flownpx claudepluginhub synaptiai/synapti-marketplace --plugin flowThis skill is limited to using the following tools:
Searches, retrieves, and installs Agent Skills from prompts.chat registry using MCP tools like search_skills and get_skill. Activates for finding skills, browsing catalogs, or extending Claude.
Searches prompts.chat for AI prompt templates by keyword or category, retrieves by ID with variable handling, and improves prompts via AI. Use for discovering or enhancing prompts.
Compares coding agents like Claude Code and Aider on custom YAML-defined codebase tasks using git worktrees, measuring pass rate, cost, time, and consistency.
Domain skill for structured, multi-faceted code review.
FIRST VERIFY IT WORKS, THEN VERIFY IT'S GOOD. Never review code quality on code that doesn't function correctly.
Spec compliance is Stage 1. Code quality is Stage 2. Reviewing style on broken logic is wasted effort.
Stage 1 — Spec Compliance: Does the code do what the issue/acceptance criteria require? Map each criterion to implementation evidence. If Stage 1 fails, stop — no point reviewing quality on code that doesn't meet requirements.
Stage 2 — Code Quality (in priority order):
Do NOT flag maintainability issues if security or correctness issues exist. Fix the important things first.
Every review evaluates these facets (parallelizable):
| Facet | Focus | Agent |
|---|---|---|
| Security | OWASP top 10, secrets, auth/authz, input validation | security-reviewer |
| Quality | Logic correctness, edge cases, error handling | code-reviewer |
| Conventions | Commit format, branch naming, PR structure, patterns | convention-checker |
| Tests | Coverage, quality commands pass, test quality | test-runner |
| Requirements | Acceptance criteria compliance | main thread |
After all facets complete, synthesize:
file:line — same location = same finding, keep highest priorityMap each acceptance criterion to evidence:
| Status | Meaning |
|---|---|
| Met | Directly implemented and testable |
| Interpreted | Criterion was ambiguous, implementation reflects interpretation |
| Partially Met | Some aspects done, others pending |
| Not Addressed | Not implemented in this change |
### P1 - Critical
| # | Category | Location | Issue | Fix |
|---|----------|----------|-------|-----|
| 1 | security | auth.rb:42 | SQL injection via string interpolation | Use parameterized query |
### P2 - Should Fix
| # | Category | Location | Issue | Fix |
|---|----------|----------|-------|-----|
### P3 - Consider
| # | Category | Location | Issue | Fix |
|---|----------|----------|-------|-----|
For each finding, assess confidence:
Only P1 findings with High confidence should block merge.
| Signal Type | Confidence | Include In Review? |
|---|---|---|
| Verified by running code/test | High | Always |
| LSP diagnostic (error/warning from language server) | High | Always — language server has full project context |
| LSP find-references (verified all callers handled) | High | Always for P1/P2 — semantic, not text-based |
| Verified by reading code path | Medium | Always for P1/P2 |
| Pattern-match only (looks like a bug) | Low | Only if P1, flag as "needs investigation" |
| Style preference | N/A | Only as P3, never blocks merge |
Noise filter: If a finding cannot be explained with a file:line citation and a concrete scenario where it causes harm, it is noise. Drop it.
When reviewing, recognize improve: commits as legitimate Boy Scout cleanup:
improve: commits that pass the proximity test (file already modified, self-evidently correct, <10 lines, no API change, no explanation needed)Check review history to understand cycle count:
Parse FLOW_REVIEW_CYCLE and FLOW_RESOLUTION_CYCLE markers from prior PR comments to build cycle context:
REPO=$(gh repo view --json nameWithOwner --jq '.nameWithOwner')
# Parse prior review findings (from review bodies)
gh api repos/$REPO/pulls/$PR_NUM/reviews --jq '
[.[] | select(.body | test("FLOW_REVIEW_CYCLE")) | {
cycle: (.body | capture("FLOW_REVIEW_CYCLE:(?<n>[0-9]+)") | .n),
findings: (.body | capture("FINDINGS:\\[(?<f>[^\\]]+)\\]") | .f)
}]'
# Parse prior resolution outcomes (from issue comments posted via gh pr comment)
gh api repos/$REPO/issues/$PR_NUM/comments --jq '
[.[] | select(.body | test("FLOW_RESOLUTION_CYCLE")) | {
cycle: (.body | capture("FLOW_RESOLUTION_CYCLE:(?<n>[0-9]+)") | .n),
resolved: (.body | capture("RESOLVED:\\[(?<r>[^\\]]*?)\\]") | .r),
deferred: (.body | capture("DEFERRED:\\[(?<d>[^\\]]*?)\\]") | .d)
}]'
Cross-reference for each prior finding:
git diff?### Previous Feedback Status
| Cycle | Finding | Priority | Claimed Status | Verified |
|-------|---------|----------|----------------|----------|
If a finding was claimed resolved but the code at that location is unchanged, flag it as "Not verified — code unchanged".
improve: commits in already-modified files are NOT out-of-context)| Findings | Decision |
|---|---|
| P1 findings (any) | REQUEST_CHANGES |
| P2 findings only | COMMENT (suggest fixes) |
| P3 findings only | APPROVE with comments |
| No findings | APPROVE |
When agent teams are enabled, use adversarial synthesis from team-coordination skill (skills/team-coordination/SKILL.md). Reviewers work independently, share findings, challenge each other, and disputed findings escalate to human.
| Excuse | Response |
|---|---|
| "It looks correct to me" | Looking is not verifying. Trace the data flow. |
| "This is just a style issue" | Then it's P3 at most. Don't flag it as P2. |
| "I don't have time for all 5 facets" | Then prioritize: Security > Correctness > the rest. Never skip security. |
| "The tests pass so the logic is fine" | Tests prove what's tested. Review proves what's not. |
| "This is too small to review thoroughly" | Small changes, same process. Small bugs cause big outages. |