Ywc Impl Review

---
name: ywc-impl-review
description: >-
  (ywc) Use when implementation is complete and before creating a PR, to validate code against the spec or run a comprehensive quality review. Triggers: "구현 검증", "impl review", "implementation review", "사양 적합성", "코드 리뷰", "구현 리뷰", "PR 전 검증", "check my implementation", "実装レビュー". Do not use for active code generation, spec-only review (use ywc-spec-validate), product/business review (use ywc-product-review), durable review-preference capture (use ywc-review-learnings), or a security-only audit of auth/external-input/sensitive-data code (use ywc-security-audit).
---

# 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: 3 Sonnet + 1 Haiku + 1 Opus for Security) and escalates only ambiguous findings from the Sonnet/Haiku lanes to a short Opus advisor pass (Phase 2). Security is the exception to the two-phase split: it runs at Opus from Phase 1 given its CRITICAL-severity, merge-blocking stakes, and does not produce Phase 2 advisor candidates — there is no higher-tier model in this catalog to escalate to. The remaining code-review aspects (architecture / design / devex) 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 <path>` | `--spec docs/outline/02-api.md` | Specification file path (**optional**). Omitted → all five lanes run and `### Spec Traceability` emits only "No spec available" (valid). Supplied but missing/unreadable → BLOCKED. Exactly one code target (`--code` / `--git-range` / `--working-tree`) is still required |
| `--code` | `--code <path>` | `--code api/src/routes/` | Code path to review. **Exactly one** of `--code` / `--git-range` / `--working-tree` is required; they are mutually exclusive |
| `--git-range` | `--git-range <sha>..<sha>` | `--git-range abc1234..HEAD` | Git range to derive the review target. Run `git diff --name-only <range>` to obtain the changed-file list. Mutually exclusive with `--code` and `--working-tree` |
| `--working-tree` | flag | | Review the current repository's staged, unstaged, and untracked source changes without creating a commit. Mutually exclusive with `--code` and `--git-range` |
| `--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 |
| `--non-interactive` | flag | | Skip Step 7's learnings-promotion confirmation prompt — do not open it; instead emit a `### Learning candidates (not promoted — non-interactive)` block in the report. Step 0 loading, Phase 1, Phase 2, and report generation are unaffected. Default: unset = interactive (opt-in flag). Orthogonal to `--skip-learnings` — combined, both Step 0 and Step 7 are skipped and no candidate block is emitted (nothing was collected to report) |
| `--advisor-budget` | `--advisor-budget <n>` | `--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

0. **Load Review Learnings** (skip if `--skip-learnings`) — Invoke `ywc-review-learnings --mode read --target <changed files / --code path>` 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.

2. **Read Spec + Code** — Select exactly one review target:
   - `--code`: read the supplied path.
   - `--git-range`: run `git diff --name-only <range>` to obtain the changed-file list.
   - `--working-tree`: derive the changed-file list without committing — combine `git diff --name-only --diff-filter=ACMRD`, `git diff --cached --name-only --diff-filter=ACMRD`, and `git ls-files --others --exclude-standard`; deduplicate paths; then apply the repository's ignore / generated-path rules to the **combined** list, so a tracked-but-ignored generated file is excluded exactly like an untracked one (`--exclude-standard` covers only the untracked leg). Capture the staged and unstaged diffs for tracked files, and treat each untracked file's full contents as its patch. **`D` (deletion) is included deliberately**: a deleted file is a reviewable change, and dropping it would let the removal of a critical-path module (auth, payment, crypto) pass the gate unseen. For a deleted path, forward the deletion diff and the pre-deletion contents.

   If the selected target has no reviewable source files, return `NEEDS_CONTEXT` and do not report an all-clear review. Read the specification file and every target code file. For range and working-tree targets, provide the bounded relevant diff plus final file contents to the subagents so they can judge both correctness and scope. This context stays with the parent; do not forward it wholesale to Phase 1 or Phase 2. Phase 1's five subagents receive only the bounded dispatch payload defined in Step 3 — not the spec or code full text read here.

3. **Phase 1 — Parallel Executor Review** — Use the Task tool to spawn five subagents in parallel. Pass `model` explicitly on each call — Architecture / Design / Devex / QA stay at Sonnet or Haiku cost; Security runs at Opus:
   - **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: opus`) — OWASP Top 10 analysis. Reference: `references/security-agent.md`. Runs at Opus from Phase 1 (not Sonnet-then-escalate) because a missed or misjudged security finding is CRITICAL severity and merge-blocking, and the bounded diff/path scope keeps the cost delta small relative to the stakes. 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 Opus-tier worker persona (`tools/claude-code/agents/ywc-security-engineer.md`). Security returns **Confirmed findings only** (no Advisor candidates) — see the return-artifacts note below.
   - **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.

   **Dispatch payload (bounded)**: each subagent receives exactly three items — (a) the changed-file list and the spec file **paths** (not full file contents), (b) that subagent's own `references/*-agent.md`, and (c) the Step 0 learnings filtered to that subagent's aspect (see the next paragraph). The subagent **Reads whatever file contents it needs itself**. Do not forward another subagent's results, the full project context, or another aspect's rubric — each lane stays scoped to its own aspect.

   **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.

   **Inject the Return-payload contract directive** into every one of the five subagent prompts, quoted **verbatim** from [`../references/subagent-status-actions.md`](../references/subagent-status-actions.md) §Return Payload Contract:

   > **Return-payload contract**: Reply with `Status | 1-line summary | artifact paths | (Concerns ≤ 10 lines | Blocker ≤ 5 lines | Missing-context bullets)`. Do not return generated code, full findings, full diffs, restated prompt content, or chain-of-thought. Write those to files and return the paths. The orchestrator will read the files only when it needs to.

   The Architecture, Design, Devex, and QA subagents must each produce two artifacts, **written to a file with only the path returned to the parent** (per the directive above):
   - **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. The bounded snippet stays inside the candidate file; the parent reads it only for candidates that survive the Step 4 budget cut.

   The **Security** subagent returns **Confirmed findings only** and never produces Advisor candidates: it already runs at Opus in Phase 1, so there is no higher-tier model to escalate an ambiguous finding to. If a finding is genuinely irresolvable because required context was not forwarded (e.g. an exemption list living outside the scoped diff), it returns `NEEDS_CONTEXT` for that item per its own Return Contract rather than escalating.

   Each reference file (`references/*-agent.md`) explains what "confident" vs "needs advisor" means for that category. The 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.

4. **Aggregate and Select Phase 2 Candidates** — Combine candidate lists from the four Sonnet/Haiku subagents (Architecture / Design / Devex / QA). Security is excluded from this pool — it already ran at Opus in Phase 1 and returns Confirmed findings only:
   - 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 Architecture > Design > Devex > QA. (This 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.

5. **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 <500 words" — 200 is an operational choice for this skill. If a genuinely complex candidate needs more room, invoke the §6 override: exceed the cap and justify the overrun in the final report's `Advisor Budget Report` section.
   - Opus calls are sequential, not parallel — each is small and fast, and sequential execution keeps the budget enforcement simple and auditable.

6. **Merge and Output Report** — Combine Phase 1 confirmed findings with Phase 2 verdicts. Mark each finding in the final report with its provenance (Phase 1 vs Phase 2) so the user can see which decisions involved frontier judgment.

7. **Capture Learnings** (skip if `--skip-learnings`) — After the report:
   - **Interactive** (default): offer to promote durable lessons into `docs/review-learnings.md` via `ywc-review-learnings --mode update --source review`: confirmed findings that **recur** (same class across files or recent reviews) become `DO`/`DO-NOT` learnings caught earlier next time, and any finding the user dismisses as a false positive becomes a `FALSE-POSITIVE` learning **with the dismissal reason**. This is what makes the next review on this repository sharper — do not write learnings without the user-confirmation CHANGESET that `ywc-review-learnings` enforces.
   - **Non-interactive** (`--non-interactive`): do not open the promotion offer. Instead, end the review by emitting a `### Learning candidates (not promoted — non-interactive)` block in the report (see Output Format) — the candidates the interactive path would have offered, with nothing written.
   - **No mode ever writes to `docs/review-learnings.md` or `references/recurring-defects.md` without the user-confirmation CHANGESET** — `--non-interactive` removes the prompt, not the write gate; it only surfaces candidates instead of asking.
   - **Both flags together** (`--non-interactive --skip-learnings`): Step 0 and Step 7 are both skipped, so nothing was collected to report — the `Learning candidates` block is not emitted.

## Output Format

```text
## Implementation Review Result: {spec} vs {code}

### Summary
- Phase 1 findings (Sonnet/Haiku executor for Architecture/Design/Devex/QA, Opus for Security): Architecture A, Design D, Devex V, Security M, QA K
- Phase 2 advisor calls (Opus): X of Y budget used
- Phase 2 adjustments: N confirmed as-is, M severity-adjusted

### Architecture
1. [severity] [P1|P2] {file}:{line} — Description
   (if P2) Advisor verdict: {one-line rationale}

### Design
1. [severity] [P1|P2] {file}:{line} — Description
   (if P2) Advisor verdict: {one-line rationale}

### Developer Experience (Devex)
1. [severity] [P1|P2] {file}:{line} — Description
   (if P2) Advisor verdict: {one-line rationale}

### Security
1. [severity] [P1|P2] {file}:{line} — Description
   (if P2) Advisor verdict: {one-line rationale}

### Testing (QA)
1. [severity] [P1|P2] — Description
   (if P2) Advisor verdict: {one-line rationale}

### Spec Traceability
(Only when `--spec` is supplied and readable. `--spec` omitted → emit only `No spec available — Spec Traceability skipped`, no criterion rows. `--spec` supplied but missing/unreadable → do not emit this section; return BLOCKED per the Completion Status rules.)

| Criterion | Status | Evidence | Scope-creep note |
|-----------|--------|----------|------------------|
| AC1 — {text} | Implemented | {file:line / named test / command output} | — |
| AC2 — {text} | Partial | {file:line} | {behavior beyond spec, if any} |
| AC3 — {text} | Missing | — | — |
| AC4 — {text} | Not Verifiable | (no admissible evidence) | — |

### Fix Priority
1. (Sorted by Critical first, with [P1|P2] markers preserved)

### Advisor Budget Report
- Used: X of Y calls
- Dropped (over budget): {list, if any}

### Learning candidates (not promoted — non-interactive)
(Only when `--non-interactive` is supplied and `--skip-learnings` is not. Never emitted in interactive mode — Step 7 offers promotion there instead. Also never emitted when both flags are combined, since Step 0/Step 7 were both skipped and nothing was collected.)
[<aspect>] Occurrences in this review: <n> — <finding 1-line summary> (severity: <value>) — would promote to <target file> as <learning type>
(none)   <- when no candidate qualifies; the block itself is still emitted

### Completion Status
(One of: DONE | DONE_WITH_CONCERNS | BLOCKED | NEEDS_CONTEXT)
```

**Spec Traceability rules** (aggregate-only — no sixth subagent): populate the matrix by aggregating the Architecture subagent's "Structural Spec Conformance" and the Design subagent's "Contract Spec Conformance" findings from Phase 1. One row per spec Acceptance Criterion.

- **Status vocabulary**: `Implemented` (fully satisfied) / `Partial` (partially satisfied) / `Missing` (no implementation found) / `Not Verifiable` (the AC exists but no admissible evidence substantiates it).
- **Evidence rule**: `Implemented` and `Partial` rows MUST cite a `file:line`, a named test/output, or command output. Inferring implementation from a task name or commit message is FORBIDDEN.
- **`Not Verifiable` vs "No spec available"**: `Not Verifiable` applies only when a specific AC lacks evidence; it is never used when `--spec` is omitted (that case emits only "No spec available", with no rows).
- **Scope creep** (behavior with no backing AC) is reported per-row in `Scope-creep note`, or as a separate sub-bullet when it maps to no criterion.
- The Evidence rule is the same bar as the `## Confidence Gate` **Evidence quality** dimension — a row without primary evidence lowers that score.

**Learning candidates rules** (non-interactive only, Step 7): field order is fixed — `[<aspect>] Occurrences in this review: <n> — <finding 1-line summary> (severity: <value>) — would promote to <target file> as <learning type>`. `<aspect>` is one of Architecture / Design / Devex / Security / QA. `Occurrences in this review` counts **only within this single invocation** — it is not cross-invocation recurrence, which only the human-confirmed Step 7 promotion flow in interactive mode establishes; there is no data source for cross-run counting here. Two Confirmed findings count as the same occurrence when they share both the same aspect and the same defect class, per the assembling step's own classification judgment (no controlled-vocabulary tag exists in `references/*-agent.md` to automate this). Findings from different aspects are never merged, even when the symptom looks similar. An item with `<n> = 1` is not listed — the block exists to surface within-run **repetition**. When zero items qualify, the block is still emitted with `(none)` — it is never omitted (Auditability NFR). Neither this mode nor the interactive one ever writes to `docs/review-learnings.md` or `references/recurring-defects.md`; this block is report surface only.

**Reporting Symbols**: Replace the bracketed `[severity]` placeholder above with the corresponding symbol from [symbols.md](../references/symbols.md). Severity vocabulary is shared across all `ywc-*` review skills — use the same symbol for the same tier.

| Symbol | Severity | Replaces |
|--------|----------|----------|
| `🚨` | Critical | `[Critical]` |
| `🔴` | High | `[High]` |
| `🟡` | Medium | `[Medium]` |
| `🔵` | Low | `[Low]` |
| `ℹ️` | Info | `[Info]` |

Example finding line:

```
🚨 [P2] src/api/users.ts:42 — SQL injection: input concatenated into query.
   Advisor verdict: confirm Critical; remediation requires parameterization.
```

The `[P1]` / `[P2]` marker is preserved unchanged — it describes Phase 1 vs Phase 2 escalation history, while the severity symbol describes finding impact. Both pieces of information are needed; do not collapse them.

For the Confidence Gate score in the report header, use the band marker from [symbols.md](../references/symbols.md): `✅` for PROCEED, `⚠️` for REVIEW, `❌` for STOP.

**Completion Status rules:**

| Status | When to use |
|--------|------------|
| `DONE` | Review complete, no Critical or High findings |
| `DONE_WITH_CONCERNS` | Review complete but Critical/High findings were identified — the report details them; human action required before merging |
| `BLOCKED` | Review cannot proceed — a supplied `--spec` path is missing or unreadable (omitting `--spec` is valid, not BLOCKED), code unreadable, or a Phase 2 escalation returned an inconclusive verdict |
| `NEEDS_CONTEXT` | Spec and code paths are ambiguous; cannot determine what conformance means without clarification |

`[P1]` marks findings confirmed entirely by the Phase 1 executor. `[P2]` marks findings that went through the Phase 2 advisor. This distinction matters when the user calibrates trust in the output — Phase 2 items represent the decisions the executor deemed genuinely ambiguous.

> **HTML mode (`--format html`)** — emits the same findings, **including the `### Spec Traceability` matrix**, 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. Security runs at Opus in Phase 1 and does not have advisor-escalation triggers (see the file's Confirmed-Findings-Only note)
- `references/qa-agent.md` — QA's coverage analysis and advisor-escalation triggers

If a reference file does not yet contain an "Advisor Candidate Criteria" section, fall back to the three-property test in [advisor-pattern.md §5](../references/advisor-pattern.md): objective trigger, irreversibility, ambiguity. A finding must satisfy all three to be a Phase 2 candidate.

**CodeRabbit-derived methodology** — the verbosity dial (`--profile`), the verify-before-surface precision gate, the intent/context phase, static-analysis-as-evidence, and the per-project learnings loop are specified in [`references/coderabbit-methodology.md`](./references/coderabbit-methodology.md). These are the techniques imported to raise review quality toward bot-reviewer parity, in a runtime-agnostic form (no CodeRabbit dependency). The learnings half pairs with `ywc-review-learnings`, which owns `docs/review-learnings.md`.

**Recurring real-world defects catalog** — every reviewer agent additionally consults [`references/recurring-defects.md`](./references/recurring-defects.md), a derived-from-data catalog of the defect classes that production bot reviewers (CodeRabbit, Codex Review) flag most often: data-layer access-boundary (ownership / tenant isolation), data-integrity / `NULL` handling, race condition / concurrent write safety, transaction boundary / partial write prevention, durable idempotency for retryable side effects, error-swallowing, external-call resilience, validation / fail-fast, HTTP-status semantics, and test fidelity. Each agent's "High-frequency real-world checks" section points at its catalog slice. The catalog tells the reviewer *where real bugs cluster*; the per-aspect rubric still governs severity and escalation. This is the same surface the executors' `--review` flag targets, so these classes get caught before the PR opens — reducing bot-review round-trips after the PR is created.

**Code smell baseline (maintainability)** — the Architecture and Design subagents additionally consult [`references/code-smell-baseline.md`](./references/code-smell-baseline.md), a Fowler-derived catalog of recurring structural/naming smells a language-agnostic reviewer can spot in a diff. It complements `recurring-defects.md` (runtime correctness) rather than duplicating it — when a smell has a runtime-defect twin, this file delegates to that catalog instead.

## Confidence Gate

This skill applies the [Confidence Gate](../references/confidence-gate.md) to the aggregated review output before emitting the final report. The gate sits between Phase 2 advisor consolidation and report emission.

In addition, when the report's gate band lands in **PROCEED** and findings include a `DONE` (or `DONE_WITH_CONCERNS`) completion claim against the implementation, the surface must follow `ywc-verify-done`: the verification block (command, output excerpt, exit code) appears before the status line, no `should` / `probably` / `seems` wording appears in the conclusion, and any "this finding's fix was verified" claim cites the fresh command output that proves it. A report that reads "all clear, looks good" without an evidence block is not a Confidence-Gate PROCEED — downgrade to REVIEW until the evidence is attached.

**Required dimensions** (must each score ≥ 70):

- **Evidence quality** — Every finding must cite a verified source: file path, line number, or test output. Findings phrased as "this *might* be wrong" without primary evidence reduce the score even if the finding itself is plausible.
- **Root cause identified** — A finding that names only the symptom ("test fails") without identifying the underlying cause ("test fixture is shared across cases") cannot be remediated correctly. The reviewer must reach root cause before recording the finding.

**Band-to-status mapping** for this skill:

| Gate band | Completion status | Action |
|-----------|-------------------|--------|
| PROCEED (≥ 90) | DONE or DONE_WITH_CONCERNS | Emit the report; status follows from the existing Critical/High count rule. |
| REVIEW (70 – 89) | NEEDS_CONTEXT | Emit the report with the gate score and weakest dimension flagged at the top. Reviewer judgment required before merge. |
| STOP (< 70) | BLOCKED | Do not emit findings as authoritative. Report which dimensions failed and what additional Phase 2 advisor calls would be needed to raise the score. |

The gate score must appear in the report header. Per-finding `[P1]` / `[P2]` markers remain unchanged — they describe escalation history, while the gate describes overall report confidence.

## Integration

- **upstream**: `ywc-sequential-executor` or `ywc-parallel-executor` (auto-invoked via `--review` flag)
- **downstream**: PR creation
- **pairs with**: `ywc-review-learnings` — loaded in Step 0 (`read`) to apply accumulated review preferences and called in Step 7 (`update --source review`) to capture new ones; `docs/review-learnings.md` is the per-project memory that compounds review quality
- **pattern source**: [`references/advisor-pattern.md`](../references/advisor-pattern.md), [`references/coderabbit-methodology.md`](./references/coderabbit-methodology.md), [`references/subagent-status-actions.md`](../references/subagent-status-actions.md)