architecture-review

What this skill does

# Architecture Review

Architecture and design pattern validation for ultrawork planning and verification phases.

## What This Skill Provides

Structured approach to evaluating:
- Component boundaries and separation of concerns
- Modularity and coupling
- Design patterns and architectural principles
- Scalability and maintainability
- API design and interfaces

## When to Use This Skill

**During PLANNING phase:**
- Reviewing planner's task decomposition
- Validating design documents
- Assessing technical approach
- Identifying architectural risks

**During VERIFICATION phase:**
- Ensuring implementation matches design
- Checking component boundaries
- Validating separation of concerns
- Reviewing integration points

## Core Principles

### 1. Separation of Concerns (SoC)

**Checklist:**

```
[ ] Business logic separated from presentation
[ ] Data access layer isolated
[ ] Authentication/authorization in dedicated modules
[ ] Configuration separate from code
[ ] API routes separate from business logic
[ ] Validation logic reusable across layers
[ ] Error handling centralized
[ ] Logging abstracted
```

**Red Flags:**
- Business logic in React components
- Database queries in route handlers
- Authentication mixed with business logic
- Configuration hardcoded in multiple files

**Examples:**

```javascript
// ❌ Bad: Mixed concerns
app.post('/api/users', async (req, res) => {
  // Authentication + validation + business logic + DB access in one place
  if (!req.headers.authorization) return res.status(401).json({ error: 'Unauthorized' });
  if (!req.body.email) return res.status(400).json({ error: 'Email required' });
  const user = await db.users.create(req.body);
  res.json(user);
});

// ✅ Good: Separated concerns
app.post('/api/users',
  authenticate,           // Auth middleware
  validateCreateUser,     // Validation middleware
  createUserHandler       // Route handler delegates to service
);

async function createUserHandler(req, res) {
  const user = await userService.createUser(req.body); // Business logic in service
  res.json(user);
}
```

### 2. Component Boundaries

**Checklist:**

```
[ ] Components have clear, single responsibilities
[ ] Component interfaces are well-defined
[ ] Dependencies flow in one direction (no circular deps)
[ ] Components are independently testable
[ ] Public APIs are minimal (principle of least exposure)
[ ] Internal implementation details are hidden
[ ] Components can be replaced without breaking others
```

**Red Flags:**
- Circular dependencies (A imports B, B imports A)
- God objects (classes with 10+ responsibilities)
- Tight coupling (changes in A always require changes in B)
- Leaky abstractions (internal details exposed)

**Dependency Flow:**

```
Presentation → Business Logic → Data Access → Database
     ↓              ↓                ↓
   UI Layer    Service Layer    Repository Layer

NEVER: Data Access → Presentation (wrong direction!)
```

### 3. Modularity and Coupling

**Checklist:**

```
[ ] Modules are cohesive (related functionality grouped)
[ ] Low coupling between modules
[ ] Shared code in libraries/utilities
[ ] Feature folders group related files
[ ] Module exports are explicit and minimal
[ ] Dependencies are injected (not hardcoded)
[ ] Configuration passed from outside
```

**Cohesion vs Coupling:**

| Good Modularity | Bad Modularity |
|-----------------|----------------|
| High cohesion (related code together) | Low cohesion (unrelated code mixed) |
| Low coupling (minimal dependencies) | High coupling (everything depends on everything) |
| Clear interfaces | Implicit contracts |
| Dependency injection | Hardcoded dependencies |

**Examples:**

```javascript
// ❌ Bad: High coupling, hardcoded dependencies
class UserService {
  constructor() {
    this.db = new PostgresDB(); // Hardcoded!
    this.logger = console;       // Hardcoded!
  }
}

// ✅ Good: Low coupling, dependency injection
class UserService {
  constructor(database, logger) {
    this.db = database;
    this.logger = logger;
  }
}

// Usage: dependencies injected
const userService = new UserService(
  new PostgresDB(config.db),
  new Logger(config.logging)
);
```

### 4. Design Patterns

**Checklist:**

```
[ ] Patterns used appropriately (not overengineered)
[ ] Consistent patterns across codebase
[ ] Standard patterns preferred over custom solutions
[ ] Patterns documented in code
```

**Common Patterns:**

| Pattern | Use When | Example |
|---------|----------|---------|
| Repository | Data access abstraction | `userRepository.findById(id)` |
| Service | Business logic encapsulation | `orderService.placeOrder()` |
| Factory | Object creation complexity | `PaymentFactory.create(type)` |
| Strategy | Swappable algorithms | `ShippingStrategy.calculate()` |
| Middleware | Request/response processing | Express middleware |
| Observer | Event-driven updates | Event emitters, pub/sub |

**Red Flags:**
- Patterns used incorrectly
- Over-abstraction (patterns for simple problems)
- Mixing patterns inconsistently
- Custom patterns when standard ones exist

### 5. API Design

**Checklist:**

```
[ ] RESTful conventions followed (GET/POST/PUT/DELETE)
[ ] Consistent URL structure
[ ] Proper HTTP status codes
[ ] Versioning strategy defined
[ ] Request/response schemas documented
[ ] Error responses are consistent
[ ] Pagination for collections
[ ] Filtering and sorting supported where needed
```

**REST Conventions:**

| HTTP Method | Purpose | Idempotent? |
|-------------|---------|-------------|
| GET | Retrieve resource | Yes |
| POST | Create resource | No |
| PUT | Update/replace resource | Yes |
| PATCH | Partial update | No |
| DELETE | Remove resource | Yes |

**Examples:**

```javascript
// ❌ Bad: Inconsistent, non-RESTful
POST /getUser
POST /deleteUser
GET /updateUser?id=123&name=John

// ✅ Good: RESTful, consistent
GET    /api/users/:id      (retrieve)
POST   /api/users          (create)
PUT    /api/users/:id      (update)
DELETE /api/users/:id      (delete)
```

### 6. Scalability Considerations

**Checklist:**

```
[ ] Stateless design (no server-side state)
[ ] Database queries optimized (indexes, joins)
[ ] Caching strategy defined
[ ] Async operations for I/O
[ ] Rate limiting implemented
[ ] Pagination on large datasets
[ ] Background jobs for heavy processing
[ ] CDN for static assets
```

**Red Flags:**
- Storing state in memory (server-side sessions)
- Synchronous blocking operations
- No caching for expensive operations
- Loading entire tables
- Missing indexes on query columns

## Layer Architecture Patterns

### 3-Tier Architecture

```
┌─────────────────────┐
│  Presentation Layer │  (UI, API routes)
└──────────┬──────────┘
           │
┌──────────▼──────────┐
│   Business Layer    │  (Services, logic)
└──────────┬──────────┘
           │
┌──────────▼──────────┐
│    Data Layer       │  (Repositories, DB)
└─────────────────────┘
```

**Rules:**
- Upper layers can call lower layers
- Lower layers CANNOT call upper layers
- Layers communicate via interfaces

### Feature-Based Structure

```
src/
├── features/
│   ├── auth/
│   │   ├── auth.service.ts
│   │   ├── auth.controller.ts
│   │   ├── auth.repository.ts
│   │   └── auth.test.ts
│   ├── users/
│   │   ├── user.service.ts
│   │   ├── user.controller.ts
│   │   └── user.repository.ts
├── shared/
│   ├── utils/
│   ├── middleware/
│   └── types/
```

**Benefits:**
- Related code together
- Easy to find files
- Scalable structure
- Clear ownership

## Review Process

### Step 1: High-Level Assessment

Answer:
- What is the overall architecture pattern?
- Are responsibilities clearly separated?
- Is the dependency flow correct?
- Can components be tested independently?

### Step 2: Component Analysis

For each component:
- What is its single responsibility?
- What are its dependencies?
- Is its interface minimal and clear?
- Is it properly abstracted?

### Step 3: Integration Review

Check:
- How do components communicate?
- Are interfaces well-defined?