implementation-plan-reviewer

When an implementation plan references existing code, libraries, or interfaces, the plan MUST be based on VERIFIED behavior, not ASSUMED behavior.

A method named assert_model_updated(model, field=value) might:

• Assert ONLY those fields were updated (partial assertion)
• Assert those fields AND REQUIRE all other changes to also be asserted (strict assertion)
• Behave completely differently than the name suggests

YOU DO NOT KNOW WHICH until you READ THE SOURCE.

5.1 The Fabrication Anti-Pattern

When executing agents encounter unexpected behavior, a common failure mode is INVENTING solutions:

# The Fabrication Loop (FORBIDDEN)
1. Plan assumes method does X based on name
2. Agent writes code, code fails because method actually does Y
3. Agent INVENTS a parameter: method(..., partial=True)
4. Code fails because parameter doesn't exist
5. Agent INVENTS another approach: method(..., strict=False)
6. Agent enters debugging loop, never reads the actual source
7. Hours wasted on fabricated solutions

# The Correct Approach (REQUIRED in Plan)
1. Plan explicitly states: "Behavior verified by reading [source location]"
2. Plan includes actual method signatures from source
3. Plan documents constraints discovered from reading source
4. Executing agents follow verified behavior, no guessing needed

5.2 Verification Requirements for Implementation Plans

For every existing interface, library, or codebase utility the plan references:

Flag every ASSUMED entry as a critical gap that will cause agent confusion.

5.3 Dangerous Assumption Patterns in Plans

Flag when the implementation plan:

1. Assumes convenience parameters exist

   • "Pass partial=True to allow partial matching" (VERIFY THIS EXISTS)
   • "Use strict_mode=False to relax validation" (VERIFY THIS EXISTS)
   • "Set ignore_extra=True to skip unknown fields" (VERIFY THIS EXISTS)

2. Assumes flexible behavior from strict interfaces

   • "The test context allows partial assertions" (VERIFY: many require exhaustive assertions)
   • "The validator accepts subset of fields" (VERIFY: many require complete objects)
   • "The mock will ignore unconfigured calls" (VERIFY: many raise on unexpected calls)

3. Assumes library behavior from method names

   • "The update() method will merge fields" (VERIFY: might replace entirely)
   • "The validate() method returns errors" (VERIFY: might raise exceptions)
   • "The save() method is idempotent" (VERIFY: might create duplicates)

4. Assumes existing test utilities work "conveniently"

   • "Our TestContext.assert_model_updated() checks specified fields" (VERIFY: might require ALL changes)
   • "Our mock_service() helper auto-mocks everything" (VERIFY: might require explicit setup)
   • "Our with_fixtures() decorator handles cleanup" (VERIFY: might require manual cleanup)

5.4 Verification Checklist for Plans

For each existing interface/library/utility referenced:

### Interface: [name]

**Behavior claimed in plan:** [what the plan says it does]

**Verification performed by plan author:**
[ ] Docstring/type hints quoted in plan
[ ] Implementation read (if behavior unclear from docs)
[ ] Usage examples from codebase cited
[ ] Confirmed NO invented parameters in plan

**Actual verified behavior:** [what it actually does, with source reference]

**Constraints discovered:** [any strictness, requirements, or limitations]

**Discrepancy from assumed behavior:** [if any - this is a critical finding]

5.5 Loop Detection

If an implementation plan describes iterative debugging approaches like:

• "Try X, if that fails try Y, if that fails try Z"
• "Experiment with different parameter combinations"
• "Adjust until tests pass"

This is a RED FLAG that the plan author did not verify behavior. The plan should instead say:

• "Behavior verified: X is the correct approach because [source reference]"

5.6 Factchecker Escalation

Some claims in implementation plans require deeper verification than plan review can provide. Flag claims for escalation to the factchecker skill when:

For each escalated claim:

### Escalated Claim: [quote from implementation plan]

**Location:** [section/line]
**Category:** [Security / Performance / Test Utility / etc.]
**Why escalation needed:** [quick verification insufficient because...]
**Factchecker depth recommended:** SHALLOW / MEDIUM / DEEP
**Escalate to factchecker?** YES / NO

Phase 6: Definition of Done Verification

For EVERY work item, verify clear acceptance criteria:

### Work Item: [name]

**Definition of Done Present?** YES / NO / PARTIAL

**If YES, verify completeness:**
[ ] Testable criteria (not subjective)
[ ] Measurable outcomes
[ ] Specific outputs enumerated
[ ] Clear pass/fail determination

**If NO or PARTIAL:**
Missing: [what acceptance criteria must be added]

Phase 7: Risk Assessment Verification

7.1 Risk Documentation Check

7.2 Missing Risk Analysis

For each phase or major work item, check:

### Phase/Item: [name]

**Risks documented?** YES / NO

**If NO, identify risks:**
1. [Risk description] - likelihood: H/M/L, impact: H/M/L
2. [Risk description] - likelihood: H/M/L, impact: H/M/L
...

**Mitigations needed:**
1. [Mitigation for risk 1]
2. [Mitigation for risk 2]
...

Phase 8: QA & Testing Verification

8.1 QA Checkpoints

Verify QA is specified at EACH phase:

8.2 Integration Testing Strategy

8.3 Test Execution Requirements

Verify the plan specifies:

## Test Execution Checklist

[ ] When to run tests (at what milestones)
[ ] What test suites to run
[ ] Pass criteria for proceeding
[ ] Failure handling procedure
[ ] Requirement to use green-mirage-audit skill for test quality analysis
[ ] Requirement to use systematic-debugging skill for test failures

If green-mirage-audit not referenced:

**Missing QA Integration**
The plan must specify: "After tests pass, run green-mirage-audit to verify tests actually validate correctness, not just pass."

If systematic-debugging not referenced:

**Missing Debug Integration**
The plan must specify: "When tests fail, use systematic-debugging skill to form hypotheses and run experiments before attempting fixes."

Phase 9: Documentation Updates

Verify documentation requirements are specified:

Phase 10: Agent Responsibility Matrix

10.1 Agent/Role Clarity

For each agent or work stream:

### Agent: [name/identifier]

**Responsibilities:**
1. [Specific deliverable]
2. [Specific deliverable]
...

**Inputs (depends on):**
1. [Deliverable from Agent X]
2. [Deliverable from Agent Y]
...

**Outputs (provides to):**
1. [Deliverable to Agent X]
2. [Deliverable to Agent Y]
...

**Interfaces owned:**
1. [Interface specification]
...

**Clarity assessment:** CLEAR / AMBIGUOUS
**If AMBIGUOUS:** [what needs clarification]

10.2 Dependency Graph Verification

## Dependency Graph

[Represent as ASCII or describe]

Agent A (Setup)
    ↓
Agent B (Core)  →  Agent C (API)
    ↓                  ↓
Agent D (Tests) ← ─ ─ ─ ┘

**All dependencies explicit?** YES / NO
**Circular dependencies?** YES / NO (if yes, flag as critical)
**Missing dependency declarations:** [list]

Phase 11: Findings Report

Summary Statistics

## Implementation Plan Review Score

### By Category
| Category | Complete | Partial | Missing | N/A |
|----------|----------|---------|---------|-----|
| Design Doc Comparison | | | | |
| Timeline Structure | | | | |
| Work Classification | | | | |
| Setup/Skeleton | | | | |
| Interface Contracts | | | | |
| Behavior Verification | | | | |
| Definition of Done | | | | |
| Risk Assessment | | | | |
| QA Checkpoints | | | | |
| Integration Testing | | | | |
| Documentation | | | | |
| Agent Responsibilities | | | | |

### Interface Contract Status
- Total interfaces identified: X
- Fully specified: Y
- Partially specified: Z
- Missing: W

### Critical Gap: Interface contracts at Z% (must be 100%)

### Claims Escalated to Factchecker: Q

Claims Requiring Factchecker Verification

List claims that need deeper verification via the factchecker skill:

| # | Claim | Location | Category | Recommended Depth |
|---|-------|----------|----------|-------------------|
| 1 | [claim text] | [section] | Test Utility | DEEP |
| 2 | [claim text] | [section] | Security | MEDIUM |
...

Critical Findings (Must Fix Before Execution)

**Finding #N: [Title]**

**Location:** [section/line]

**Category:** [Interface Contract / Definition of Done / Risk / etc.]

**Current State:**
[Quote or describe what's in the plan]

**Problem:**
[Why this is insufficient for parallel agent execution]

**What Executing Agent Would Have to Guess:**
[Specific decisions that would be made without guidance]

**Required Addition:**
[Exact specification that must be added]

**Risk if not fixed:**
[What could go wrong during execution]

Important Findings (Should Fix)

Same format, lower priority.

Minor Findings (Nice to Fix)

Same format, lowest priority.

Phase 12: Actionable Remediation Plan

## Remediation Plan

### Priority 1: Critical Gaps (Blocking Parallel Execution)
1. [ ] [Specific interface contract to add]
2. [ ] [Specific type definition to add]
...

### Priority 2: QA/Testing Gaps
1. [ ] [QA checkpoint to add]
2. [ ] [Integration test specification to add]
...

### Priority 3: Documentation & Clarity
1. [ ] [Definition of done to add]
2. [ ] [Risk assessment to add]
...

### Factchecker Verification Required
If claims were escalated, use the Skill tool to invoke the `factchecker` skill before finalizing.
Pre-flag these claims for verification:
1. [ ] [Claim] - [Category] - [Recommended Depth]
2. [ ] [Claim] - [Category] - [Recommended Depth]
...

### Required Skill Integrations
- [ ] Add explicit instruction: "Use green-mirage-audit after test runs"
- [ ] Add explicit instruction: "Use systematic-debugging for failures"
- [ ] Add explicit instruction: "Use factchecker for security/performance/behavior claims"

### Recommended Structure Additions
- [ ] Add section: [Interface Contracts] with [content]
- [ ] Add table: [Agent Responsibility Matrix]
- [ ] Add diagram: [Dependency Graph]