Deep Review
Perform thorough code review using three perspectives with opposing mindsets. Their disagreement surfaces issues; their agreement signals confidence.
When to Use
- Architectural changes, multi-file refactors, or security-sensitive code
- PRs that are too important for single-pass review
- When you suspect confirmation bias in a standard review
- High-stakes merges where the cost of a missed issue is high
When NOT to Use
- Routine single-file edits (use standard
code-review skill)
- Documentation-only PRs
- Formatting/linting changes
The Three Perspectives
| Agent |
Mindset |
Question |
Owns |
| Advocate |
"Why is this correct?" |
Trust boundaries, design rationale, false-positive defense |
Correctness defense |
| Skeptic |
"How can I break this?" |
Bugs, edge cases, code smells that indicate bugs |
Correctness attack |
| Architect |
"Is this the right direction?" |
System impact, scope, structural smells, tech debt |
Direction |
Workflow
Phase 1: Gather Context
- Identify the changes — PR diff, local changes, or specific files
- Collect context — related files, tests, recent history of changed modules
- Note observations — anything unusual before analysis begins
Phase 2: Parallel Analysis
Run all three perspectives independently. Each sees the same context but asks different questions.
Advocate Analysis
- What problem does this solve?
- What design decisions are intentional (not accidental)?
- Where are the trust boundaries correctly placed?
- What would break if we rejected this PR?
- Defend against false-positive concerns raised by Skeptic
Skeptic Analysis
- What inputs could break this? (null, empty, overflow, concurrent, malicious)
- What error paths are unhandled?
- What assumptions are undocumented?
- What would a fuzzer find?
- What code smells indicate deeper bugs? (naming lies, magic numbers, commented-out code)
- What works in tests but would fail in production?
Architect Analysis
- Does this fit the existing architecture or fight it?
- What's the blast radius if this fails?
- Does this increase or decrease coupling?
- Is there scope creep disguised as "while I'm here"?
- What precedent does this set for future changes?
- Is there tech debt being introduced? Is it intentional and documented?
Phase 3: Synthesis
3.1 Agreement Analysis
What do multiple perspectives agree on? → High-confidence findings.
3.2 Conflict Resolution
When perspectives disagree, apply these rules:
| Conflict |
Resolution |
| Skeptic finds bug, Advocate defends |
Does Advocate cite file:line that refutes? If not, Skeptic wins |
| Advocate says intentional, Skeptic says bug |
If Skeptic shows reproducible path → it's a bug regardless of intent |
| Architect says blocking, Skeptic disagrees on priority |
Skeptic's priority on correctness issues; Architect's on direction |
| No evidence either way |
Mark as "Disputed" for human decision |
Core rule: Evidence beats assertion. A file:line citation wins over "probably."
3.3 Final Output
## Deep Review: <title>
### Summary
<1-2 sentence overview of the change and verdict>
### Perspectives
**Advocate** (Design Rationale)
<key defenses and intentional design decisions>
**Skeptic** (Risk Analysis)
<bugs found, edge cases, concerns with evidence>
**Architect** (Architectural Impact)
<patterns, debt, direction, system-level concerns>
### Consolidated Findings
| # | Issue | Priority | Advocate | Skeptic | Architect |
|---|-------|----------|----------|---------|-----------|
| 1 | <issue> | Critical/High/Medium/Low | <view> | <view> | <view> |
### Disputed (if any)
<issues where perspectives disagree and human must decide>
### Recommendations
<prioritized actions>
### Follow-up Items
<non-blocking concerns worth tracking>
Priority Classification
| Priority |
Criteria |
Action |
| Critical |
Data loss, security vulnerability, crash in production path |
Block merge |
| High |
Incorrect behavior under realistic conditions |
Block merge |
| Medium |
Code smell, missing test, unclear naming, minor debt |
Request fix or accept with note |
| Low |
Style, nitpick, suggestion for future |
Comment only |
Example
PR: Add rate limiting middleware to API gateway
Advocate: "Rate limiting prevents resource exhaustion. The sliding window approach handles burst traffic better than fixed windows. The 429 response includes Retry-After header per RFC 6585."
Skeptic: "The window reset logic at middleware/rate-limit.ts:47 uses Date.now() but the TTL in Redis uses seconds — off by 1000x. Under load, the counter will never expire. Also: no test covers the window boundary."
Architect: "Rate limiting belongs at this layer (before auth, after TLS). But the config is hardcoded — should use env vars for per-deployment tuning. This sets precedent that all middleware reads config from constants."
Synthesis: Skeptic's timing bug is Critical (blocks merge). Architect's config concern is Medium (fix in follow-up). Advocate's design rationale is sound.
Integration with ACT
- Tenet II (Disconfirmation): The Skeptic's entire job is disconfirmation
- Tenet III (Multiple Hypotheses): Three perspectives prevent anchoring on first interpretation
- Tenet VIII (Adversarial Self-Probe): The review structure IS adversarial by design
- Materiality Gate: Use Deep Review for high-stakes; standard
code-review for routine
1---2name: deep-review-33description: Adversarial code review with three parallel perspectives — Advocate, Skeptic, Architect — that create productive tension. Use for high-stakes PRs, architectural changes, or when single-pass review would miss issues. Surfaces findings through disagreement, not consensus.4---56# Deep Review78Perform thorough code review using three perspectives with opposing mindsets. Their disagreement surfaces issues; their agreement signals confidence.910## When to Use1112- Architectural changes, multi-file refactors, or security-sensitive code13- PRs that are too important for single-pass review14- When you suspect confirmation bias in a standard review15- High-stakes merges where the cost of a missed issue is high1617## When NOT to Use1819- Routine single-file edits (use standard `code-review` skill)20- Documentation-only PRs21- Formatting/linting changes2223---2425## The Three Perspectives2627| Agent | Mindset | Question | Owns |28| ----- | ------- | -------- | ---- |29| **Advocate** | "Why is this correct?" | Trust boundaries, design rationale, false-positive defense | Correctness defense |30| **Skeptic** | "How can I break this?" | Bugs, edge cases, code smells that indicate bugs | Correctness attack |31| **Architect** | "Is this the right direction?" | System impact, scope, structural smells, tech debt | Direction |3233---3435## Workflow3637### Phase 1: Gather Context38391. **Identify the changes** — PR diff, local changes, or specific files402. **Collect context** — related files, tests, recent history of changed modules413. **Note observations** — anything unusual before analysis begins4243### Phase 2: Parallel Analysis4445Run all three perspectives independently. Each sees the same context but asks different questions.4647#### Advocate Analysis4849- What problem does this solve?50- What design decisions are intentional (not accidental)?51- Where are the trust boundaries correctly placed?52- What would break if we rejected this PR?53- Defend against false-positive concerns raised by Skeptic5455#### Skeptic Analysis5657- What inputs could break this? (null, empty, overflow, concurrent, malicious)58- What error paths are unhandled?59- What assumptions are undocumented?60- What would a fuzzer find?61- What code smells indicate deeper bugs? (naming lies, magic numbers, commented-out code)62- What works in tests but would fail in production?6364#### Architect Analysis6566- Does this fit the existing architecture or fight it?67- What's the blast radius if this fails?68- Does this increase or decrease coupling?69- Is there scope creep disguised as "while I'm here"?70- What precedent does this set for future changes?71- Is there tech debt being introduced? Is it intentional and documented?7273### Phase 3: Synthesis7475#### 3.1 Agreement Analysis7677What do multiple perspectives agree on? → High-confidence findings.7879#### 3.2 Conflict Resolution8081When perspectives disagree, apply these rules:8283| Conflict | Resolution |84| -------- | ---------- |85| Skeptic finds bug, Advocate defends | Does Advocate cite `file:line` that refutes? If not, Skeptic wins |86| Advocate says intentional, Skeptic says bug | If Skeptic shows reproducible path → it's a bug regardless of intent |87| Architect says blocking, Skeptic disagrees on priority | Skeptic's priority on correctness issues; Architect's on direction |88| No evidence either way | Mark as "Disputed" for human decision |8990**Core rule: Evidence beats assertion.** A `file:line` citation wins over "probably."9192#### 3.3 Final Output9394```markdown95## Deep Review: <title>9697### Summary98<1-2 sentence overview of the change and verdict>99100### Perspectives101102**Advocate** (Design Rationale)103<key defenses and intentional design decisions>104105**Skeptic** (Risk Analysis)106<bugs found, edge cases, concerns with evidence>107108**Architect** (Architectural Impact)109<patterns, debt, direction, system-level concerns>110111### Consolidated Findings112113| # | Issue | Priority | Advocate | Skeptic | Architect |114|---|-------|----------|----------|---------|-----------|115| 1 | <issue> | Critical/High/Medium/Low | <view> | <view> | <view> |116117### Disputed (if any)118<issues where perspectives disagree and human must decide>119120### Recommendations121<prioritized actions>122123### Follow-up Items124<non-blocking concerns worth tracking>125```126127---128129## Priority Classification130131| Priority | Criteria | Action |132| -------- | -------- | ------ |133| **Critical** | Data loss, security vulnerability, crash in production path | Block merge |134| **High** | Incorrect behavior under realistic conditions | Block merge |135| **Medium** | Code smell, missing test, unclear naming, minor debt | Request fix or accept with note |136| **Low** | Style, nitpick, suggestion for future | Comment only |137138---139140## Example141142**PR**: Add rate limiting middleware to API gateway143144**Advocate**: "Rate limiting prevents resource exhaustion. The sliding window approach handles burst traffic better than fixed windows. The 429 response includes Retry-After header per RFC 6585."145146**Skeptic**: "The window reset logic at `middleware/rate-limit.ts:47` uses `Date.now()` but the TTL in Redis uses seconds — off by 1000x. Under load, the counter will never expire. Also: no test covers the window boundary."147148**Architect**: "Rate limiting belongs at this layer (before auth, after TLS). But the config is hardcoded — should use env vars for per-deployment tuning. This sets precedent that all middleware reads config from constants."149150**Synthesis**: Skeptic's timing bug is Critical (blocks merge). Architect's config concern is Medium (fix in follow-up). Advocate's design rationale is sound.151152---153154## Integration with ACT155156- **Tenet II** (Disconfirmation): The Skeptic's entire job is disconfirmation157- **Tenet III** (Multiple Hypotheses): Three perspectives prevent anchoring on first interpretation158- **Tenet VIII** (Adversarial Self-Probe): The review structure IS adversarial by design159- **Materiality Gate**: Use Deep Review for high-stakes; standard `code-review` for routine
gabrielmoreira