sdlc-architectural-reviewer

# sdlc-architectural-reviewer

Produces a structural assessment of an existing codebase. Evaluates against recognized design principles and identifies specific anti-patterns with concrete evidence. This skill produces findings; the Architectural Review Agent synthesizes them into the full report with recommendations and diagrams.

## Responsibility

Evaluate the structural design of a codebase. Produce findings per principle/pattern area. Do not redesign. Do not write code. Produce structured evidence.

## Input

```json
{
  "repository_path": "absolute path to repo",
  "reverse_engineering_output": "path to docs/codebase-analysis/ if available",
  "primary_goals": ["scalability", "testability", "team_velocity", "cost_reduction"],
  "planned_changes": "brief description of upcoming features or scale requirements"
}
```

## Analysis Dimensions

### Separation of Concerns

Check for:
- Business logic placed inside controllers, route handlers, or HTTP layer code
- Database/ORM annotations or queries mixed into domain model objects
- Presentation formatting logic embedded in service or repository classes
- Cross-cutting logic (logging, auth, validation) duplicated across layers instead of delegated to middleware or interceptors

For each violation:
- `file`, `class_or_function`, `line`, `description`, `layer_affected`, `recommended_move`

### SOLID Violations

**Single Responsibility**
- Classes or modules with more than one distinct reason to change
- Indication: large class (200+ lines non-blank), many unrelated public methods, multiple import clusters serving different concerns

**Open/Closed**
- Code that requires modifying existing switch/if-else chains when adding new behavior
- Missing use of polymorphism or strategy pattern where extension is common

**Liskov Substitution**
- Subclasses that override parent methods and throw exceptions or return different types
- Inheritance hierarchies where the subtype narrows behavior expected by callers of the parent

**Interface Segregation**
- Interfaces with many unrelated methods
- Implementations that stub or throw on methods they do not use

**Dependency Inversion**
- Concrete classes instantiated directly inside higher-level modules (new ConcreteService() inside a business class)
- Service locator patterns hiding hard dependencies

### Coupling

- High afferent coupling: components that many other components depend on (single point of failure)
- High efferent coupling: components that depend on many others (fragile, hard to test)
- Circular dependencies between modules or packages
- For each: list the components involved, direction of dependency, severity (high/medium/low)

### Layering Violations

- Does the codebase have a recognizable layer structure (e.g., presentation/service/repository/domain)?
- Are dependencies pointing in the wrong direction (e.g., domain layer importing from infrastructure)?
- List any cases where a lower layer imports a higher layer

### Scalability Concerns

- Shared