Ywc Impl Review

ywc-impl-review

Announce at start: "I'm using the ywc-impl-review skill to run a five-axis (architecture / design / devex / security / QA) implementation review."

Implementation conformance review skill. Runs five parallel reviewers (Phase 1: 4 Sonnet + 1 Haiku) and escalates only ambiguous findings to a short Opus advisor pass (Phase 2). The four code-review aspects (architecture / design / devex / security) follow the gstack review-army pattern; QA stays as a separate axis because coverage analysis is mechanical and benefits from a lighter model. See [Advisor Pattern](../references/advisor-pattern.md) for why this shape is used.

Rationalization Defense

When tempted to skip a step, check this table first:

| Excuse | Reality | |---|---| | "Phase 1 had no findings, the spec is satisfied" | Absence of findings ≠ proof of conformance. State what was checked, not what was missing. | | "This finding looks ambiguous, send it all to Opus" | Phase 2 budget is 5. Reserve for genuinely ambiguous cases — not first-pass uncertainty. | | "Forwarding the full spec gives Opus better context" | Never forward full files or full project context. Only the finding text + bounded snippet + spec excerpt. | | "Severity feels somewhere between High and Medium" | Pick based on impact, not feeling. If truly between, that is a Phase 2 candidate. | | "--no-advisor saves time on this review" | Skip Phase 2 only on throwaway/prototype code. Production review needs ambiguity escalation. | | "Reviewer agents agree, so the finding is correct" | Three Sonnet agents drawing the same wrong conclusion is still wrong. Escalate when stakes are high. | | "User wants a quick review, severity ratings are optional" | Without severity, the user cannot triage. Always rate Critical / High / Medium / Low. | | "Surface every nitpick to be thorough" | A review that buries one real bug under ten style nits trains the reader to ignore all of them. chill is the default for a reason — suppress the Style/Docs/polish tail unless --profile assertive. Thoroughness on correctness, restraint on nitpicks. | | "This finding is plausible, surface it with a 'might be'" | Verify-before-surface: a finding without primary evidence (file:line, traced symbol, command output) is dropped, not hedged. A hedged finding is noise that costs the reader a round-trip to disprove. | | "There's a docs/review-learnings.md FALSE-POSITIVE entry but I'll flag it anyway to be safe" | A FALSE-POSITIVE learning carries the team's reason it is a non-issue here. Re-raising it ignores accumulated project knowledge and reintroduces the exact noise the learning was created to stop. |

Violating the letter of these rules is violating the spirit. Review without honest severity is theater.

Arguments

| Parameter | Format | Example | Description | |-----------|--------|---------|-------------| | --spec | --spec | --spec docs/outline/02-api.md | Specification file path (required) | | --code | --code | --code api/src/routes/ | Code path to review (required). --code and --git-range are mutually exclusive | | --git-range | --git-range .. | --git-range abc1234..HEAD | Git range to derive the review target. Run git diff --name-only to obtain the changed-file list. --code and --git-range are mutually exclusive | | --no-advisor | flag | | Skip Phase 2 entirely. Use when running on throwaway or prototype code where frontier judgment on ambiguous findings is not worth the latency | | --profile | --profile chill\|assertive | Verbosity dial (default chill). chill surfaces only correctness / security / logic / runtime-risk findings and suppresses the Style/Docs/Devex-polish nitpick tail (Low/Info); assertive emits those too. Critical/High/Medium are never suppressed. See [coderabbit-methodology.md §1](./references/coderabbit-methodology.md) | | --skip-learnings | flag | | Skip Step 0 (loading docs/review-learnings.md). Use when no learnings file exists or a clean-room review is wanted | | --advisor-budget | --advisor-budget | --advisor-budget 3 | Maximum number of Phase 2 Opus calls. Default: 5. Applies across all categories combined | | --format | --format markdown\|html | --format html | Output format. Default markdown. With html, writes a self-contained HTML report to claudedocs/. See [html-output.md](../references/html-output.md) |

Advisor Pattern

This skill uses Pattern B (Two-Phase Review) from [advisor-pattern.md](../references/advisor-pattern.md). The rationale: review findings range from mechanical (hardcoded secret, missing null check, trivial OWASP match) to genuinely ambiguous (architectural judgment call, severity debate between two OWASP categories, spec-conformance question with more than one reasonable reading). Running every reviewer on Opus wastes frontier capacity on the mechanical cases; running every reviewer on Sonnet undersells the ambiguous ones. Phase 1 handles the mechanical cases at Sonnet/Haiku cost; Phase 2 escalates only the ambiguous ones to Opus with tightly bounded context.

Budget discipline (see advisor-pattern.md §6): default cap is 5 Opus calls per invocation, shared across all categories. Use fewer when possible. Never forward full files or full project context to the Phase 2 advisor — only the finding text, a bounded snippet, and the relevant spec excerpt.

Execution Steps

1. Load Review Learnings (skip if --skip-learnings) — Invoke ywc-review-learnings --mode read --target to load the project's accumulated review preferences from docs/review-learnings.md. The result is a compact "Applicable Review Learnings" block: DO / DO-NOT entries become extra checks injected into the matching reviewer subagent (Step 3), and FALSE-POSITIVE entries tell that reviewer to stop raising a known non-issue. If the file is absent, proceed with an empty set — never block a review on missing learnings. This is the compounding mechanism that makes review quality rise per repository over time (see [coderabbit-methodology.md §7](./references/coderabbit-methodology.md)).

1. Collect Project Context — Read CLAUDE.md, package.json to identify conventions, tech stack, and PR gate conditions. If docs/ubiquitous-language.md exists, read it — the Reviewer subagent must flag identifiers that match a "Synonyms to Avoid" entry instead of the canonical term. Per [coderabbit-methodology.md §3](./references/coderabbit-methodology.md), treat the spec / PR description as the statement of intent and trace each changed symbol to its callers/callees before judging it — a locally-fine change can still break a caller's contract.

1. Read Spec + Code — When --git-range is provided instead of --code: run git diff --name-only to obtain the changed-file list and treat those files as the review target. Read the specification file and all target code files. This context stays with the parent; do not forward it wholesale to Phase 2.

1. Phase 1 — Parallel Executor Review — Use the Task tool to spawn five subagents in parallel. Pass model explicitly on each call so the executor layer stays at Sonnet or Haiku cost:

• Architecture subagent (model: sonnet) — Module boundaries, layering, structural patterns, dependency direction, simplicity / over-abstraction, structural spec conformance. Reference: references/architecture-agent.md. When the diff touches DB schema or migrations, also apply the shared schema review checklist ([../references/schema/core.md](../references/schema/core.md) Part C); raise cascade ↔ API status (B2) and multi-tenant scope (B6) gaps as one-line cross-references to the Security subagent rather than duplicating them.
• Design subagent (model: sonnet) — API/interface design, naming, signatures, error models, return shapes, public-surface discipline, contract spec conformance. Reference: references/design-agent.md.
• Devex subagent (model: sonnet) — Readability, error messages, logging, documentation, debuggability, config UX. The operator-experience dimension. Reference: references/devex-agent.md.
• Security subagent (model: sonnet) — OWASP Top 10 analysis. Reference: references/security-agent.md. When the Claude Code named-agent catalog at tools/claude-code/agents/ is installed, prefer subagent_type: ywc-security-engineer so the subagent carries the dedicated worker persona (tools/claude-code/agents/ywc-security-engineer.md).
• QA subagent (model: haiku) — Test coverage gaps and missing test cases. Reference: references/qa-agent.md. Haiku is appropriate because coverage-gap detection is largely mechanical (file enumeration, assertion counting, branch enumeration) and does not typically require frontier reasoning.

Inject the Step 0 learnings into each subagent prompt, filtered to that aspect's category: a DO/DO-NOT learning is an extra check; a FALSE-POSITIVE learning is an explicit instruction not to raise that finding here. Apply the --profile dial: in chill (default), a subagent suppresses its Style/Docs/Devex-polish nitpick tail (Low/Info) and surfaces only correctness/security/logic/runtime-risk findings; assertive emits the tail too (Critical/High/Medium are never suppressed). Verify before surfacing (the precision lever, [coderabbit-methodology.md §2](./references/coderabbit-methodology.md)): every finding must cite primary evidence — exact file:line, the symbol traced to its definition/callers, or fresh command output — and a finding that cannot be substantiated by re-reading the code or running a scoped check is dropped, not surfaced with a hedge. Where the project ships linters/scanners (ruff, eslint, golangci-lint, shellcheck, semgrep, gitleaks, ast-grep), run them and feed their output to the relevant subagent as evidence to triage — not as the verdict ([§4](./references/coderabbit-methodology.md)). Surgical-changes check (all aspects, incl. Tier-2 language reviewers): flag any changed hunk that does not trace to a spec line or PR-description intent as an out-of-scope finding — drive-by refactors, formatter/style churn, and edits outside the change's stated purpose. Every changed line should trace to the request.

Each subagent must return two artifacts:

• Confirmed findings — issues the executor is confident about (Phase 1 complete, no escalation needed).
• Advisor candidates — findings the executor flags for Phase 2 review. Each candidate must include: the finding text, a bounded code snippet (≤100 lines), the relevant spec excerpt (if any), and a one-sentence reason the executor wants a second opinion.

Each reference file (references/*-agent.md) explains what "confident" vs "needs advisor" means for that category. The four code-review aspects (architecture / design / devex / security) stay in their own lanes: an Architecture finding does not include naming polish (Design) or error-message wording (Devex). Cross-aspect concerns surface as one-line cross-references, never as duplicated findings.

Language-specific reviewer dispatch (Tier 2) — when the changed-file list is dominated by a single language and the Claude Code named-agent catalog at tools/claude-code/agents/ is installed, replace the Design and Devex generic model: sonnet subagents with the matching language reviewer for sharper findings. Currently shipped:

• TypeScript / TSX / Vue / Svelte → subagent_type: ywc-typescript-reviewer (covers type-system depth, async correctness, framework idioms, tsconfig strictness, ESM/CJS interop)
• Python / .py / .pyi / pyproject.toml → subagent_type: ywc-python-reviewer (covers type-system depth, asyncio correctness, framework idioms — Django / FastAPI / Pydantic v2 / Flask, mypy strict mode, GIL implications, lifecycle patterns)
• Go / .go / go.mod / go.sum → subagent_type: ywc-go-reviewer (covers goroutine lifecycle, channel patterns, interface design — accept interfaces return concrete, error wrapping %w / errors.Is / errors.As, pointer vs value semantics, generics post 1.18, defer + sync primitives)

Tier 2 reviewers for other languages (Swift / Rust) are follow-up PRs; in the meantime continue with the generic Design / Devex subagent prompts for those files.

1. Aggregate and Select Phase 2 Candidates — Combine candidate lists from all five subagents:

• Deduplicate findings that share {file}:{line} across categories.
• Cap the total at --advisor-budget (default 5). If candidates exceed the cap, prioritize: Critical > High > Medium, and within the same severity prefer Security > Architecture > Design > Devex > QA. (Architecture / Design / Devex order reflects irreversibility — structural decisions are hardest to walk back, then contracts, then operator UX.)
• Log the candidates that were dropped due to the cap in the final report so the user can see what was not escalated.

1. Phase 2 — Advisor Pass (skip entirely if --no-advisor) — For each surviving candidate, spawn a short Opus subagent via the Task tool with model: opus. When the candidate's source category is Architecture and the Claude Code named-agent catalog at tools/claude-code/agents/ is installed, prefer subagent_type: ywc-architect so the advisor carries the dedicated architectural-decision persona (tools/claude-code/agents/ywc-architect.md). When the candidate's ambiguous axis is performance (latency / throughput / memory / bundle-size / Web Vitals regression on an Architecture or Devex candidate), prefer subagent_type: ywc-performance-engineer (tools/claude-code/agents/ywc-performance-engineer.md) so the advisor carries the dedicated Performance persona with concrete remediation (specific query rewrite, missing-index DDL, dynamic-import split point, Web Vitals fix path, profiler invocation recommendation). This is a routing hint — Phase 2 subagent count and budget are unchanged. Other categories use the generic model: opus dispatch.

• Context payload: only the candidate's finding text, the bounded snippet, the spec excerpt, and the category-specific severity rubric from the matching reference file. Do not forward the full spec, the full file, or the Phase 1 transcripts.
• Expected output: a short verdict (≤200 words) containing: confirmed severity, a one-line rationale, and either "confirmed" or "adjusted" (with the adjustment if any). The 200-word cap is tighter than [advisor-pattern.md §3](../references/advisor-pattern.md)'s observed ceiling of "typically HTML mode (--format html) — emits the same findings as a self-contained HTML report: severity color coding, tab navigation, and a Copy as Markdown button. Structure and conventions follow [html-output.md](../references/html-output.md). The Markdown surface is preserved inside the file, so downstream integration is unaffected.

Agent Prompt References

Read the corresponding reference file when spawning each subagent and include the relevant section in the subagent prompt. The reference files describe both the standard review dimensions and the "advisor candidate" criteria specific to each category:

• references/architecture-agent.md — Architecture's review dimensions (structural spec conformance, pattern consistency, module interface, simplicity, surgical changes) + advisor triggers
• references/design-agent.md — Design's review dimensions (contract spec conformance, naming, signatures, error model, return shapes, public-surface discipline) + advisor triggers
• references/devex-agent.md — Devex's review dimensions (readability, error messages, logging, documentation, debuggability, config UX) + advisor triggers
• references/security-agent.md — Security's OWASP Top 10 checklist and advisor-escalation triggers
• references/qa-agent.md — QA's coverage analysis and advisor-escalation triggers

If a reference file does not yet contain an "Advisor Candidate C

…

Source & license

This open-source skill is cataloged on AgentStack and links to its original source — we do not rehost the code.

• Author: yongwoon
• Source: yongwoon/ywc-agent-toolkit
• License: MIT

Install and usage instructions live in the source repository linked above.