Smell

Smell — Architecture Bad Smell Detector

Analyze a codebase to find violations of software architecture principles, anti-patterns, code "bad smells," and algorithmic complexity hotspots. Produce a comprehensive, actionable markdown report.

Knowledge base: This skill encodes architectural patterns, anti-patterns, code smells, and algorithmic complexity heuristics drawn from industry research and practice, including the classic code smells catalog by Martin Fowler / Kent Beck (as organized on refactoring.guru: Bloaters, Object-Orientation Abusers, Change Preventers, Dispensables, Couplers).

The Job

1. Understand the scope — ask what part of the project to analyze (full project, specific module, or recent changes)
2. Scan the codebase using find, grep, and Agent (Explore subagent) to gather candidate signals and evidence
3. Validate candidates against context, callers, history, workload, and measurements before confirming findings
4. Generate a detailed markdown report saved to tasks/smell-report-[timestamp].md
5. Present a summary of confirmed findings and separate candidates to the user

Step 1: Scope Clarification

Ask the user:

What scope should I analyze?
  A. Entire project (thorough, may take time)
  B. Specific module/directory: [please specify]
  C. Only recently changed files (git diff)
  D. Only architectural-level issues (skip low-level code smells)

If the user doesn't specify, default to option A for small projects (< 100 files) or C for large projects.

Step 2: Evidence Gathering

Use the Explore subagent (Agent with subagent_type: "Explore") to scan the codebase for architectural patterns and anti-patterns. Run multiple parallel explorations:

Exploration Commands

Run these in parallel to gather evidence efficiently:

1. Project Structure Scan: Map the directory tree, identify the architectural style (layered, modular monolith, microservices, etc.)
2. Dependency Analysis: Find import/include patterns, check for circular dependencies, identify coupling hotspots
3. Module/Component Scan: Identify God Objects (files > 500 lines), check cohesion, check single responsibility violations
4. Pattern Detection: Look for known anti-pattern signatures (static cling, service locator abuse, leaky abstractions)
5. Testing Scan: Check test coverage patterns, test file locations, test-to-code ratios
6. Naming & Clarity Scan: Flag misleading names, overly generic names (Manager, Helper, Util), inconsistent naming conventions
7. Complexity Scan: Detect algorithmic complexity hotspots — nested loops, N+1 queries, repeated scans, sort-in-loop, expensive recomputation in render paths

Key Heuristics

Heuristics are candidate signals, not findings. A line-count, nesting, naming, or Big-O match must be validated against the code's responsibility, callers, change history, workload, and intentional constraints. Do not assign severity from a threshold alone.

Step 3: Report Generation

Finding Identity and Evidence

Process every candidate in three stages:

1. Candidate detection: static patterns, file metrics, and dependency scans produce candidates only.
2. Context validation: read the implementation and relevant callers; check change frequency, input size, runtime frequency, framework constraints, generated/vendor status, and existing mitigations.
3. Finding confirmation: merge candidates with the same root cause, affected path, failure/change scenario, and remediation direction into one finding.

Count findings by independent root cause, never by the number of principles they implicate. Use one Primary principle and optional Related principles. SOLID is an umbrella label; use SRP, OCP, or DIP as the primary label when the evidence supports a specific lens, without also creating a separate SOLID finding.

Canonical 11-Principle Matrix

For each confirmed finding, record evidence strength (Measured, Observed, or Inferred) separately from confidence (High, Medium, or Low/Candidate). Evidence strength does not imply severity.

Severity Rubric

• Critical: correctness, reliability, security, data consistency, or measured system-level impact; normally requires high confidence.
• Warning: clear reach or repeated change/runtime cost with a concrete maintenance or runtime consequence.
• Suggestion: local, low-frequency, or limited-impact improvement with evidence.
• Candidate requiring measurement: static signal with unknown impact; exclude it from severity totals and put it in a separate report section.

Generate the report in this structure:

# Architecture Smell Report

**Project:** [project-name]
**Scope:** [scope description]
**Date:** [date]
**Analyzer:** smell skill (Ducc)

---

## Executive Summary

[2-3 paragraph summary of confirmed findings only: architectural style detected, overall health assessment, and top 3-5 critical issues. Mention candidates separately.]

---

## Architectural Style Detected

[Identify the architectural style: Layered, Modular Monolith, Microservices, Hexagonal, Clean Architecture, or Big Ball of Mud]

### Style Expectations vs. Reality

| Expectation | Reality | Status |
|-------------|---------|--------|
| [e.g., Clear layer separation] | [what was found] | ✅/⚠️/🔴 |

---

## Findings by Category

### 🔴 Critical Issues (Must Fix)

[Issues that fundamentally undermine architecture]

### 🟡 Warnings (Should Fix)

[Issues that degrade maintainability but don't block function]

### 🔵 Suggestions (Nice to Fix)

[Minor improvements that would increase quality]

### Candidates Requiring Measurement

[Static candidates whose runtime impact, change frequency, or workload is not yet established. These do not count toward severity totals.]

---

## Detailed Findings

### Finding #1: [Title]

- **Category:** [Architecture/Coupling/Cohesion/Design/Code/Testing/Naming/Complexity]
- **Severity:** 🔴 Critical / 🟡 Warning / 🔵 Suggestion
- **Anti-Pattern:** [Name of anti-pattern]
- **Location:** [file:line references and relevant callers]
- **Confidence:** [High/Medium]
- **Evidence strength:** [Measured/Observed/Inferred]
- **Failure or change scenario:** [Concrete scenario]
- **Primary principle:** [Most specific principle]
- **Related principles:** [Explanatory only; do not count separately]
- **Description:** [What was found and why it's a problem]
- **Evidence:** [Code, dependency, history, or measurement]
- **Measured/observed impact:** [Reach, frequency, consequence, or baseline]
- **Recommendation:** [Smallest justified refactoring]
- **Verification:** [How to prove behavior and impact]

---

## Dependency Graph Analysis

[Summary of module dependencies, circular dependencies found, coupling hotspots]

---

## Module Health Scorecard

| Module | Lines | God Object Risk | Coupling | Cohesion | Test Coverage | Health |
|--------|-------|----------------|----------|----------|---------------|--------|
| [name] | [N] | [Low/Med/High] | [Low/Med/High] | [Low/Med/High] | [% or N/A] | 🟢/🟡/🔴 |

---

## Smell Distribution

Count only deduplicated confirmed findings by their primary category. Related principles and candidates do not affect these totals.

| Category | Count | Critical | Warning | Suggestion |
|----------|-------|----------|---------|------------|
| Architecture | [N] | [N] | [N] | [N] |
| Coupling | [N] | [N] | [N] | [N] |
| Cohesion | [N] | [N] | [N] | [N] |
| Design | [N] | [N] | [N] | [N] |
| Code | [N] | [N] | [N] | [N] |
| Testing | [N] | [N] | [N] | [N] |
| Naming | [N] | [N] | [N] | [N] |
| Complexity | [N] | [N] | [N] | [N] |

---

## Refactoring Roadmap

Order work by impact, confidence, dependency sequence, and verification cost—not by principle count or smell name. Put only high-confidence, verifiable findings in Immediate Actions; for candidates, recommend the next measurement instead of a rewrite.

### Immediate Actions (This Sprint)
1. [Actionable fix 1]
2. [Actionable fix 2]

### Short-Term (1-3 Months)
1. [Structural improvement 1]
2. [Structural improvement 2]

### Long-Term (3-12 Months)
1. [Architectural transformation 1]
2. [Architectural transformation 2]

---

## Appendix: Anti-Pattern Reference

[A condensed reference of anti-patterns checked, with brief descriptions]

Step 4: Save and Present

Save the report to tasks/smell-report-[YYYY-MM-DD-HHmm].md and present a brief summary to the user.

Anti-Pattern Knowledge Base

This section documents the architectural anti-patterns and bad smells the skill knows about.

Architectural Anti-Patterns

Big Ball of Mud

The most common de-facto architecture. A haphazardly structured, sprawling system with no perceivable architecture. Characterized by:

• Promiscuous sharing of information between distant elements
• Global or duplicated important state
• Structure eroded beyond recognition or never defined
• Repeated expedient repair ("duct tape and bailing wire")
• Forces: Time pressure, cost, inexperience, complexity, change, scale
• Remedy: Define architecture boundaries, refactor incrementally, apply SHEARING LAYERS, KEEP IT WORKING

Distributed Monolith

Microservices that must be deployed together. Symptoms:

• Services share a database
• Synchronous chains of service calls
• Changes require coordinated deployments
• Remedy: Decouple data stores, introduce async messaging, enforce bounded contexts

Anemic Domain Model

Domain objects with only getters/setters (data bags), all logic in services. Violates:

• "Tell, Don't Ask" principle
• Rich Domain Model pattern from DDD
• Remedy: Move behavior into domain objects, use domain services only for cross-aggregate operations

God Object

A class that knows too much or does too much. Characteristics:

• 500 lines or > 20 public methods

• Handles unrelated concerns
• Difficult to test in isolation
• Single Responsibility Principle violation
• Remedy: Extract cohesive groups of methods into dedicated classes

Leaky Abstractions

Abstractions that expose implementation details. Signs:

• Interface methods named after implementation (e.g., SaveToPostgres, FetchFromRedis)
• Consumers catching implementation-specific exceptions
• Configuration details exposed through abstractions
• Remedy: Design interfaces from the consumer's perspective, hide implementation details

Static Cling

Excessive use of static methods/state. Problems:

• Untestable (can't mock static calls)
• Hidden dependencies
• Thread-safety issues with static state
• Remedy: Use dependency injection, convert stateless statics to instance methods

Service Locator Abuse

Using a service locator instead of dependency injection. Issues:

• Hidden dependencies (dependencies not visible in constructor)
• Runtime errors instead of compile-time errors
• Testing difficulty
• Remedy: Use constructor injection, register dependencies at composition root

Violated Layer Boundaries (Clean/Onion/Hexagonal Architecture)

In layered architectures:

• Clean Architecture: Outer layers (frameworks) leaking into inner layers (use cases, entities)
• Onion Architecture: Infrastructure concerns in domain core
• Hexagonal Architecture: Business logic coupled to specific adapters instead of ports
• Remedy: Apply dependency inversion, define clear port interfaces

CQRS Overuse

Applying CQRS to simple CRUD. Signs:

• Separate read/write models for trivial data access
• Event sourcing when events don't add business value
• Unnecessary complexity
• Remedy: Use CQRS only when read/write models genuinely differ or have different scaling needs

Vertical Slice Contamination

In Vertical Slice Architecture:

• Cross-slice coupling (one feature directly calling another)
• Shared service classes undermining slice independence
• Remedy: Use events/messages for cross-slice communication, duplicate simple logic if needed

Top Ten Software Architecture Mistakes

A set of architecture-level anti-patterns describing over- and under-engineering. The common thread: architecture disconnected from real needs and reality. The opposite extreme (too little architecture) is equally a smell.

Over-Layered / Multitier Architecture

"Layers on layers on layers." Adding tiers beyond what the problem needs:

• Each layer just forwards calls to the next with no transformation or value
• Simple read requires touching 6+ classes across 4 layers
• Remedy: Collapse pass-through layers; keep only layers that carry real responsibility

Over-Abstraction

Abstraction piled on until the code is impossible to follow:

• Excessive interfaces, generics, factories, and indirection for single implementations
• You can't tell what actually runs without stepping through many hops
• Remedy: Inline single-implementation abstractions; abstract only at real variation points (rule of three)

Futuristic Architecture

Solution built for imagined future requirements that no one can actually predict:

• Extensibility points, plugin systems, config knobs nothing uses
• Most speculative flexibility is wasted effort — closely related to Speculative Generality and YAGNI
• Remedy: Build for today's known requirements; add flexibility when a real second case arrives

Technology-Enthusiast Architecture

New/shiny technology put into production because the architect liked it:

• Unproven tech adopted without validating it fits the problem or scales
• Chasing trends over stability
• Remedy: Evaluate tech against actual requirements; prefer proven tools; prototype before committing

Overkill Architecture

A simple problem solved with a disproportionate amount of architecture and technology:

• Microservices, event sourcing, k8s for a CRUD app with a handful of users
• Remedy: Match architecture weight to problem size (KISS); start simple, evolve when justified

Cloud / Visio Architecture

"Architecture" that exists only in nice diagrams, disconnected from the code and runtime reality:

• Diagrams don't match what's actually deployed; boxes and arrows with no code correspondence
• Remedy: Keep architecture docs grounded in and verified against the real system

Note on the opposite extreme: total lack of architecture (no boundaries, no structure) is equally a smell — see Big Ball of Mud and Missing Architecture. Both under- and over-engineering are failures.

Coupling & Cohesion Smells

Circular Dependencies

Module A → Module B → Module A. Detected via:

• Import graph analysis
• "Cannot access before initialization" errors
• Remedy: Extract shared interface/common module, apply dependency inversion

Content Coupling

One module directly modifying another's internal state. Signs:

• Direct field access across module boundaries
• friend/package-private abuse
• Remedy: Use public APIs, encapsulate internal state

Common Coupling (Global State)

Multiple modules depending on shared global mutable state:

• Global variables, singletons with mutable state
• Ambient context (e.g., CurrentUser static property)
• Remedy: Parameterize, use dependency injection, make state explicit

Stamp Coupling

Passing entire data structures when only a few fields needed:

• Functions receiving large DTOs but using one field
• Remedy: Create focused parameters or smaller interfaces (ISP)

Shotgun Surgery

A single change requires modifications across many files:

• Adding a field touches 5+ files in different modules
• Remedy: Consolidate related behavior, apply Single Responsibility

Feature Envy

A method that uses another class's methods more than its own:

• Method calls other.foo(), other.bar(), other.baz() with few self-calls
• Remedy: Move the method to the class it envies

Data Clumps

Same group of fields appearing together in multiple places:

• (street, city, zip) appearing in 5 method signatures
• Remedy: Extract into a value object

Divergent Change

One module/class is repeatedly changed for many unrelated reasons (the opposite of Shotgun Surgery):

• "I always change these three methods for DB changes, and those two for UI changes" in the same class
• Remedy: Split the class along its axes of change (Single Responsibility)

Inappropriate Intimacy

Two classes are too entangled with each other's internals:

• Reaching into another class's private fields, tight bidirectional references
• Remedy: Move methods/fields to the class they belong to, extract a shared class, or replace with delegation

Message Chains

Long navigation chains like a.getB().getC().getD().doThing():

• Client coupled to the whole object graph; violates the Law of Demeter
• Remedy: Hide delegation — add a method on the first object that returns what the client needs

Middle Man

A class that delegates almost all of its work to another class:

• Most methods just forward calls; adds indirection without value
• Remedy: Remove the middle man and let clients talk to the real object (inline the delegation)