Reviewing Implementation Progress

Primary Responsibilities

You are provided with:

• Feature specification
• Implementation plan with tasks
• Previously completed task (baseline)
• In-progress task (current work)

Your job is to review all changes between the previously completed task and the in-progress one. Review the full git diff and explore the codebase to ensure:

• Implementation adheres to specification and implementation plan
• Code is high quality, maintainable, follows best practices
• No obvious bugs, security vulnerabilities, or performance issues
• Adequate tests cover new functionality and edge cases
• Tests actually test functionality (not just mocked behavior)
• Outstanding tasks/steps tracked appropriately

Expected Output

Provide findings in structured JSON report:

{
  "compliant": false,
  "issues": [
    {
      "type": "specification_mismatch",
      "description": "The implementation does not fully adhere to specification. Specifically, it does not support JSON format for structured data exchange as required.",
      "location": {
        "file": "src/cli/index.ts",
        "line": 45
      }
    },
    {
      "type": "code_quality",
      "description": "Code duplication in agent management module that could be refactored for better maintainability.",
      "location": {
        "file": "src/agents/agentManager.ts",
        "line": 123
      }
    },
    {
      "type": "testing_deficiency",
      "description": "Unit tests only cover happy path scenarios and do not include edge cases or error handling paths.",
      "location": {
        "file": "tests/workflowEngine.test.ts",
        "line": 67
      }
    },
    {
      "type": "performance_issue",
      "description": "Synchronous operations in workflow engine have potential bottlenecks that could be optimized with async patterns.",
      "location": {
        "file": "src/workflowEngine/stateMachine.ts",
        "line": 234
      }
    }
  ]
}

Issue Types

specification_mismatch

Implementation doesn't match what specification requires. Critical to fix.

plan_deviation

Implementation differs from plan without justification. May or may not be problematic.

code_quality

Maintainability issues, duplication, unclear naming, poor organization.

testing_deficiency

Missing tests, inadequate coverage, tests that don't actually verify behavior.

security_vulnerability

Potential security issues (injection, XSS, auth bypass, etc.).

performance_issue

Obvious performance problems, inefficient algorithms, resource leaks.

missing_error_handling

Errors not caught or handled properly.

incomplete_implementation

Feature partially implemented, missing parts.

Review Process

1. Read Specification and Plan

• Understand what should be implemented
• Note key requirements and acceptance criteria
• Identify critical constraints

2. Identify Baseline (Previous Task)

• Find git commit/SHA for previously completed task
• This is the baseline to compare against

3. Review Current Changes

• Get git diff from baseline to current state
• Read all modified files
• Understand what changed and why

4. Check Against Requirements

For each requirement in spec/plan:

• Is it implemented?
• Is it implemented correctly?
• Are there tests?
• Do tests actually verify the behavior?

5. Evaluate Code Quality

• Readability and maintainability
• Proper error handling
• Security considerations
• Performance considerations
• Test quality

6. Generate Report

• Set compliant: true only if no issues found
• Categorize each issue by type
• Provide specific file:line locations
• Write clear, actionable descriptions

Integration with Workflows

With executing-plans

Add optional checkpoint between batches:

Execute Task 1
Execute Task 2
Execute Task 3
→ Review progress (this skill)
→ Fix issues if any
→ Continue with Task 4-6

With subagent-driven-development

Review after each task to catch issues early:

Task 1 → Review → Fix → Task 2 → Review → Fix → ...

Use Cases

Mid-Feature Checkpoint

Scenario: Implementing large feature across 10 tasks, currently on task 5 Use: Review tasks 1-5 against spec to ensure on track before continuing

Quality Gate

Scenario: Before marking phase complete, verify all work meets standards Use: Review all changes in phase against requirements

Catching Drift Early

Scenario: Implementation may be diverging from plan Use: Early review catches drift before too much work is done wrong

Important Notes

Only Report Real Issues

• Don't report style preferences
• Don't suggest improvements unless actual problems
• Don't be overly pedantic
• Focus on things that matter

Be Specific

Bad: "Code quality issues in auth module" Good: "Duplicate password validation logic in auth/login.ts:45 and auth/register.ts:67"

Provide Context

For each issue, explain:

• What's wrong
• Why it matters
• Where it is (file:line)
• Implicitly, what should be done to fix it (through description)

Compliance Determination

Set compliant: true only when:

• All spec requirements met
• All plan tasks completed as described
• Code quality acceptable
• Tests adequate and passing
• No security issues
• No critical bugs

Set compliant: false when:

• Any requirement missing
• Tests inadequate
• Security concerns
• Code quality poor
• Functionality broken

Example Report

{
  "compliant": false,
  "issues": [
    {
      "type": "specification_mismatch",
      "description": "Specification requires rate limiting of 100 req/min for anonymous users. Implementation at src/middleware/rateLimit.ts:23 sets limit to 50 req/min.",
      "location": {
        "file": "src/middleware/rateLimit.ts",
        "line": 23
      }
    },
    {
      "type": "testing_deficiency",
      "description": "No tests verify rate limit enforcement. Tests at tests/middleware/rateLimit.test.ts:15-45 only mock the rate limiter and don't test actual limiting behavior.",
      "location": {
        "file": "tests/middleware/rateLimit.test.ts",
        "line": 15
      }
    }
  ]
}

Related Skills

• reviewing-code - Comprehensive review framework (use for completed work)
• executing-plans - Execute plans in batches (integrate reviews between batches)
• debugging-systematically - Debug issues found in review
• avoiding-testing-anti-patterns - Avoid common testing mistakes