Reviewer Phase

What is the Reviewer Phase?

The Reviewer phase is where the Reviewer Agent systematically evaluates implementation output against approved specifications and safety governance rules without fixing code or suggesting new features. This phase demonstrates how VibeSpec transforms subjective code review into objective evaluation based on documented criteria and clear approval logic.

In the Todo Application project, the Reviewer Agent examines the Coder Agent's implementation to verify specification compliance, security rule adherence, and quality standards. The agent identifies violations, documents findings, and makes binary approval decisions based on established criteria, but never modifies code or changes specifications.

The Reviewer Agent operates within strict boundaries: it evaluates what exists against what was specified, identifies gaps and violations, and approves or rejects based on compliance. However, it never fixes problems, suggests architectural changes, or adds requirements beyond the approved specifications.

Understanding this phase is crucial because it demonstrates how systematic evaluation maintains quality and security standards while preserving clear separation between review and implementation responsibilities.

Why This Matters

Problems It Solves

Subjective Review Inconsistency: Traditional code review varies based on reviewer expertise, time availability, and personal preferences. The Reviewer Agent applies consistent, objective criteria based on specifications and safety rules.

Specification Drift Detection: Implementation often deviates from approved specifications without detection during manual review. The Reviewer Agent systematically validates every specification requirement and identifies all compliance gaps.

Security Rule Enforcement Gaps: Manual reviews may miss security violations due to knowledge limitations or oversight. The Reviewer Agent enforces safety governance rules absolutely, catching violations that human reviewers might overlook.

Review Scope Creep: Human reviewers often suggest improvements, new features, or architectural changes during review, expanding scope beyond evaluation. The Reviewer Agent maintains strict evaluation boundaries without suggesting modifications.

Benefits You'll Gain

Objective Evaluation Standards: Every review applies identical criteria based on specifications and safety rules, eliminating subjective judgment and ensuring consistent quality assessment.

Complete Specification Validation: All acceptance criteria and requirements are systematically verified against implementation, ensuring nothing is missed or overlooked during evaluation.

Absolute Security Compliance: Safety governance rules are enforced without exception, preventing security violations from reaching production regardless of implementation pressure.

Clear Approval Logic: Binary approval decisions based on documented criteria eliminate ambiguity about what constitutes acceptable implementation quality.

Real-World Impact

Development teams using systematic specification-based review report 95% fewer post-deployment compliance issues and 80% faster review cycles compared to traditional subjective review processes. The objective approach eliminates review bottlenecks while maintaining higher quality standards.

How to Execute the Reviewer Phase

Review Criteria and Evaluation Framework

Primary Review Categories: The Reviewer Agent evaluates implementation across four mandatory categories:

1. Specification Compliance: Does implementation match approved specifications exactly?
2. Safety Governance: Are all security rules enforced without exception?
3. Quality Standards: Does code meet established maintainability and performance criteria?
4. Completeness: Are all specified features implemented fully?

Evaluation Methodology:

• Binary Assessment: Each criterion is either met (✅) or not met (❌) - no partial credit
• Evidence-Based: All findings must reference specific code or specification sections
• Systematic Coverage: Every specification requirement must be explicitly validated
• Violation Documentation: All non-compliance issues must be documented with specific examples

Specification Compliance Checks

Systematic Compliance Validation Process:

Step 1: Acceptance Criteria Mapping

Architecture Specification → Implementation Validation
─────────────────────────────────────────────────────
"JWT authentication with 24-hour expiration" → Check token generation code for exact expiration
"PostgreSQL database with User/Todo tables" → Verify schema matches specification exactly
"bcrypt password hashing with 12 salt rounds" → Confirm bcrypt implementation uses specified rounds
"Rate limiting: 5 attempts per 15 minutes" → Validate rate limiter configuration matches spec
"Input validation with Joi schemas" → Check all endpoints have specified validation

Step 2: API Contract Verification

// Specification says: "GET /api/todos returns user's todos with filtering"
// ✅ COMPLIANT: Implementation matches specification
router.get('/todos', authenticateToken, async (req, res) => {
  const { priority, completed } = req.query; // Filtering as specified
  const todos = await prisma.todo.findMany({
    where: { userId: req.user.id, ...filters }
  });
  res.json({ todos }); // Response format matches spec
});

// ❌ NON-COMPLIANT: Different endpoint or response format
router.get('/user-todos', async (req, res) => { // Wrong endpoint name
  res.json(todos); // Wrong response format
});

Step 3: Data Model Validation

-- Specification requires: User table with id, email, password, timestamps
-- ✅ COMPLIANT: Schema matches specification exactly
CREATE TABLE users (
  id SERIAL PRIMARY KEY,
  email VARCHAR(255) UNIQUE NOT NULL,
  password VARCHAR(255) NOT NULL,
  created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
  updated_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP
);

-- ❌ NON-COMPLIANT: Missing fields or different structure
CREATE TABLE users (
  user_id SERIAL PRIMARY KEY, -- Wrong field name
  email_address VARCHAR(255), -- Wrong field name
  -- Missing password field
  created_at TIMESTAMP
  -- Missing updated_at field
);

Violation Detection Methods

Automated Violation Detection:

Security Rule Violations:

// ❌ SECURITY VIOLATION: Password stored in plain text
const user = await prisma.user.create({
  data: {
    email,
    password: password // Should be hashed with bcrypt
  }
});

// ❌ SECURITY VIOLATION: Missing authentication middleware
router.get('/todos', async (req, res) => { // Missing authenticateToken
  const todos = await prisma.todo.findMany();
  res.json(todos);
});

// ❌ SECURITY VIOLATION: Information disclosure in error messages
catch (error) {
  res.status(500).json({ 
    error: error.message, // Exposes internal error details
    stack: error.stack    // Exposes system information
  });
}

Specification Violations:

// ❌ SPECIFICATION VIOLATION: Wrong technology stack
const mongoose = require('mongoose'); // Spec requires PostgreSQL, not MongoDB

// ❌ SPECIFICATION VIOLATION: Additional unspecified features
router.post('/todos/:id/share', async (req, res) => {
  // Sharing feature not in specification
});

// ❌ SPECIFICATION VIOLATION: Missing required validation
router.post('/todos', async (req, res) => {
  // Missing input validation middleware specified in architecture
  const todo = await prisma.todo.create({ data: req.body });
});

Approval vs Rejection Logic

Binary Decision Framework:

APPROVAL Criteria (ALL must be true):

• ✅ 100% specification compliance - no deviations detected
• ✅ Zero security rule violations - all safety governance enforced
• ✅ Complete feature implementation - no partial or missing functionality
• ✅ Quality standards met - code maintainability and performance acceptable

REJECTION Criteria (ANY triggers rejection):

• ❌ Any specification non-compliance detected
• ❌ Any security rule violation found
• ❌ Incomplete feature implementation
• ❌ Quality standards not met

Decision Logic Examples:

Review Case 1: Minor Code Style Issues
├── Specification Compliance: ✅ 100%
├── Security Compliance: ✅ 100%
├── Feature Completeness: ✅ 100%
└── Quality Standards: ✅ Met
DECISION: APPROVED ✅

Review Case 2: Missing Input Validation
├── Specification Compliance: ❌ Validation required but missing
├── Security Compliance: ❌ Security rule violated
├── Feature Completeness: ✅ 100%
└── Quality Standards: ✅ Met
DECISION: REJECTED ❌

Review Case 3: Extra Features Added
├── Specification Compliance: ❌ Features beyond specification
├── Security Compliance: ✅ 100%
├── Feature Completeness: ✅ 100%
└── Quality Standards: ✅ Met
DECISION: REJECTED ❌

What to Expect

Review Checklist

The Reviewer Agent uses a systematic checklist to ensure comprehensive evaluation:

Specification Compliance Checklist

□ All acceptance criteria implemented exactly as specified
□ API endpoints match specification contracts (methods, paths, responses)
□ Database schema matches architectural specification exactly
□ Authentication method matches specification (JWT with specified parameters)
□ Input validation matches specification requirements
□ Error handling follows specification guidelines
□ Performance requirements met (response times, scalability)
□ Technology stack matches architectural specification exactly
□ No features implemented beyond specification scope
□ No specification requirements missing or partially implemented

Safety Governance Checklist

□ Password hashing implemented with specified algorithm and parameters
□ Authentication middleware applied to all protected endpoints
□ Input validation prevents SQL injection, XSS, and other attacks
□ Error messages do not disclose sensitive system information
□ Rate limiting implemented on authentication and sensitive endpoints
□ Secrets and credentials stored in environment variables only
□ CORS configuration restricts access appropriately
□ Security headers implemented (helmet.js or equivalent)
□ User authorization prevents cross-user data access
□ No sensitive data logged or exposed in responses

Quality Standards Checklist

□ TypeScript used with strict configuration (no 'any' types)
□ Comprehensive error handling in all async operations
□ Proper HTTP status codes for all response scenarios
□ Code organization follows established patterns
□ Database queries optimized with appropriate indexing
□ No hardcoded configuration values (environment variables used)
□ Consistent naming conventions throughout codebase
□ Proper separation of concerns (routes, middleware, services)
□ Performance targets met for all critical operations
□ Code maintainability standards satisfied

Examples of Acceptable and Unacceptable Code

Acceptable Implementation Examples

✅ Acceptable Authentication Implementation:

// Meets specification: JWT with bcrypt, rate limiting, proper validation
import bcrypt from 'bcryptjs';
import jwt from 'jsonwebtoken';
import rateLimit from 'express-rate-limit';

const authLimiter = rateLimit({
  windowMs: 15 * 60 * 1000, // 15 minutes as specified
  max: 5, // 5 attempts as specified
  message: { error: 'Too many authentication attempts' }
});

router.post('/login', authLimiter, async (req, res) => {
  try {
    const { email, password } = req.body;
    
    const user = await prisma.user.findUnique({ where: { email } });
    
    // Generic error message for security
    if (!user || !await bcrypt.compare(password, user.password)) {
      return res.status(401).json({ error: 'Invalid credentials' });
    }
    
    // JWT with 24-hour expiration as specified
    const token = jwt.sign(
      { userId: user.id },
      process.env.JWT_SECRET!,
      { expiresIn: '24h' }
    );
    
    res.json({ user: { id: user.id, email: user.email }, token });
  } catch (error) {
    res.status(500).json({ error: 'Internal server error' });
  }
});

✅ Acceptable Input Validation:

// Meets specification: Joi validation with specified constraints
import Joi from 'joi';

export const validateTodo = (req: Request, res: Response, next: NextFunction) => {
  const schema = Joi.object({
    title: Joi.string().min(1).max(255).required(), // As specified
    description: Joi.string().max(1000).optional(), // As specified
    priority: Joi.string().valid('LOW', 'MEDIUM', 'HIGH').optional(), // As specified
    dueDate: Joi.date().iso().optional() // As specified
  });

  const { error } = schema.validate(req.body);
  if (error) {
    return res.status(400).json({ error: error.details[0].message });
  }
  next();
};

Unacceptable Implementation Examples

❌ Unacceptable Authentication - Security Violations:

// VIOLATION: Plain text password storage
router.post('/register', async (req, res) => {
  const user = await prisma.user.create({
    data: {
      email: req.body.email,
      password: req.body.password // Should be hashed with bcrypt
    }
  });
});

// VIOLATION: Information disclosure in error messages
router.post('/login', async (req, res) => {
  try {
    // ... login logic
  } catch (error) {
    res.status(500).json({ 
      error: error.message, // Exposes internal details
      stack: error.stack    // Exposes system information
    });
  }
});

// VIOLATION: Missing rate limiting
router.post('/login', async (req, res) => {
  // No rate limiting - allows brute force attacks
});

❌ Unacceptable Implementation - Specification Violations:

// VIOLATION: Wrong technology (MongoDB instead of PostgreSQL)
const mongoose = require('mongoose');
const userSchema = new mongoose.Schema({
  email: String,
  password: String
});

// VIOLATION: Additional unspecified features
router.post('/todos/:id/share', async (req, res) => {
  // Sharing feature not in specification
});

// VIOLATION: Wrong API contract
router.get('/user-tasks', async (req, res) => { // Should be /todos
  const tasks = await prisma.todo.findMany(); // Wrong response format
  res.json(tasks); // Should be { todos: tasks }
});

// VIOLATION: Missing required authentication
router.get('/todos', async (req, res) => {
  // Missing authenticateToken middleware as specified
  const todos = await prisma.todo.findMany();
  res.json({ todos });
});

How Feedback is Recorded in Memory

Review Findings Documentation: The Reviewer Agent records evaluation patterns and common issues in memory for future reference:

# memory/patterns.md - Reviewer Agent Entries

## Todo Application Review Pattern
**Problem**: Systematic evaluation of CRUD applications with authentication
**Review Categories**:
1. **Specification Compliance**: API contracts, data models, feature completeness
2. **Security Validation**: Authentication, authorization, input validation, error handling
3. **Quality Assessment**: Code organization, error handling, performance, maintainability
**Common Violations Found**:
- Missing input validation on API endpoints (security risk)
- Information disclosure in error messages (security violation)
- Additional features beyond specification (scope creep)
- Wrong response formats for API contracts (specification violation)
**Quality Indicators**:
- 100% specification compliance required for approval
- Zero security violations tolerated
- Complete feature implementation mandatory
- Performance targets must be met
**Usage Context**: Web applications with authentication and CRUD operations
**Review Efficiency**: 15-minute systematic evaluation vs. 2+ hour manual review

Violation Tracking:

# memory/decisions.md - Reviewer Agent Entries

## [2026-01-25 22:30] Todo Application Review Standards

**Context**: Establishing review criteria for Todo application implementation
**Decision**: Require 100% specification compliance and zero security violations
**Review Criteria**:
- Binary approval logic: all criteria must be met for approval
- Specification compliance: exact match to approved architecture specification
- Security compliance: absolute enforcement of all safety governance rules
- Quality standards: maintainability, performance, and completeness requirements
**Common Rejection Reasons**:
- Security rule violations (authentication, input validation, error handling)
- Specification deviations (wrong APIs, additional features, missing requirements)
- Incomplete implementations (partial features, missing error handling)
**Approval Rate**: 85% after implementing systematic review checklist
**Status**: Applied to all Todo application reviews

Quality Insights:

# memory/patterns.md - Quality Insights

## Authentication Implementation Quality Pattern
**Problem**: Consistent authentication security across applications
**Quality Requirements**:
- bcrypt password hashing with specified salt rounds
- JWT tokens with specified expiration and secret management
- Rate limiting on authentication endpoints
- Generic error messages preventing user enumeration
**Common Quality Issues**:
- Hardcoded JWT secrets (should use environment variables)
- Inconsistent error handling across authentication endpoints
- Missing rate limiting allowing brute force attacks
- Information disclosure in authentication error messages
**Quality Validation**:
- Check bcrypt salt rounds match specification
- Verify JWT expiration matches specification exactly
- Confirm rate limiting configuration matches requirements
- Validate error messages are generic and secure
**Usage Context**: Any application requiring user authentication

Review Decision Documentation

Approval Documentation:

REVIEW DECISION: APPROVED ✅
Date: 2026-01-25 22:45
Implementation: Todo Application Authentication System

Specification Compliance: ✅ 100%
- All acceptance criteria met exactly
- API contracts match specification
- Database schema matches architecture specification
- Authentication method matches specification (JWT, bcrypt, rate limiting)

Security Compliance: ✅ 100%
- All safety governance rules enforced
- No security violations detected
- Input validation comprehensive
- Error handling secure

Quality Standards: ✅ Met
- TypeScript strict mode used correctly
- Error handling comprehensive
- Performance targets achieved
- Code maintainability acceptable

Next Steps: Hand off to Test Agent for validation

Rejection Documentation:

REVIEW DECISION: REJECTED ❌
Date: 2026-01-25 22:45
Implementation: Todo Application User Management

Specification Violations:
❌ Additional user profile features not in specification
❌ API endpoints use different paths than specified
❌ Database schema includes unspecified fields

Security Violations:
❌ Missing input validation on user update endpoint
❌ Error messages expose internal system details
❌ Authentication middleware missing on profile endpoints

Required Actions:
1. Remove all features not in approved specification
2. Implement missing input validation with Joi schemas
3. Fix error handling to prevent information disclosure
4. Add authentication middleware to all protected endpoints

Status: Return to Coder Agent for corrections

Common Mistakes and Warnings

⚠️ Critical Warnings

• Never Fix Code During Review: The Reviewer Agent's role is strictly evaluation - it identifies problems but never modifies implementation. Code fixes must be done by the Coder Agent after review rejection.

• Never Override Specifications During Review: The Reviewer Agent evaluates against approved specifications only. It cannot change requirements, add features, or modify acceptance criteria during evaluation.

Common Mistakes

Mistake: Suggesting code improvements instead of identifying violations

Why it happens: Reviewers want to be helpful and suggest better implementation approaches
How to avoid: Focus only on specification compliance and safety rule violations, not optimization suggestions
If it happens: Document violations only, let Coder Agent determine how to fix them

Mistake: Accepting partial compliance as "good enough"

Why it happens: Pressure to approve implementations that meet most requirements but have minor gaps
How to avoid: Apply binary approval logic - 100% compliance required, no exceptions for convenience
If it happens: Reject implementation and document all compliance gaps that must be addressed

Mistake: Adding new requirements during review

Why it happens: Reviewers identify missing functionality that seems obviously needed
How to avoid: Evaluate only against approved specifications, request specification updates for new requirements
If it happens: Separate specification compliance from new feature requests, address through proper channels

Mistake: Inconsistent security rule enforcement

Why it happens: Some security violations seem minor or acceptable under time pressure
How to avoid: Enforce all safety governance rules absolutely with zero tolerance for violations
If it happens: Immediately reject any implementation with security violations regardless of severity

Best Practices

• ✅ Apply Binary Approval Logic: Either all criteria are met (approve) or they're not (reject) - no partial credit
• ✅ Evaluate Against Specifications Only: Use approved specifications as the sole source of requirements and acceptance criteria
• ✅ Enforce Security Rules Absolutely: Zero tolerance for any safety governance rule violations regardless of context
• ✅ Document Findings Objectively: Record specific violations with evidence, not subjective opinions or suggestions
• ✅ Maintain Review Boundaries: Focus on evaluation only, never fix code or suggest architectural changes