---
name: smell
description: "Detect software architecture bad smells, algorithmic complexity hotspots, and anti-patterns in a codebase. Produces a detailed markdown report identifying violations of architectural principles, design patterns, code quality, and performance complexity. Triggers on: smell, code smell, architecture smell, find anti-patterns, detect bad smells, complexity analysis, 代码坏味道, 架构坏味道, 反模式, 找出坏味道, 复杂度分析."
user-invocable: true
---

# 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.

---

## 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 evidence
3. Identify architectural smells and anti-patterns
4. Generate a detailed markdown report saved to `tasks/smell-report-[timestamp].md`
5. Present a summary of findings 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

| Category | Smell | Detection Heuristic |
|----------|-------|-------------------|
| **Architecture** | Big Ball of Mud | No clear directory structure; everything in root or one flat folder; no separation of concerns |
| **Architecture** | Violated Layer Boundaries | Inner layers importing outer layers; infrastructure code in domain/core layer |
| **Architecture** | Missing Architecture | No `src/`, `lib/`, `core/` separation; SQL inline with UI code; HTTP handlers mixed with business logic |
| **Architecture** | Distributed Monolith | Microservices sharing a database; services that can't deploy independently |
| **Architecture** | Anemic Domain Model | Model/entity classes with only getters/setters and no behavior; all logic in services |
| **Architecture** | CQRS Without Need | Separate read/write models for simple CRUD; unnecessary complexity |
| **Coupling** | Circular Dependencies | Module A imports B, B imports A; detected via import graph analysis |
| **Coupling** | Content Coupling | One module directly accesses another's internal/private members |
| **Coupling** | Common Coupling | Excessive global variables/shared mutable state; singleton abuse |
| **Coupling** | Stamp Coupling | Passing large data structures when only a few fields are needed |
| **Cohesion** | God Object | Single class/module > 500 lines; > 20 public methods; handles unrelated concerns |
| **Cohesion** | Shotgun Surgery | A single change requires touching 5+ files across unrelated modules |
| **Cohesion** | Feature Envy | Method calls foreign class methods more than its own class methods |
| **Cohesion** | Data Clumps | Same group of 3+ parameters appearing together in multiple method signatures |
| **Design** | Leaky Abstractions | Implementation details (DB queries, HTTP calls) exposed through interfaces |
| **Design** | Static Cling | Excessive use of static methods; static state that prevents testability |
| **Design** | Service Locator Abuse | DI container passed around instead of proper constructor injection |
| **Design** | Violated SOLID | SRP violations, OCP violations (switch/if-else chains on types), ISP violations (fat interfaces) |
| **Code** | Duplicated Code | Identical/similar logic appearing in 3+ places; copy-paste patterns |
| **Code** | Long Method | Methods > 50 lines; deep nesting (> 3 levels) |
| **Code** | Long Parameter List | Methods with > 4 parameters |
| **Code** | Primitive Obsession | Using strings/ints instead of domain types (e.g., `string email` instead of `Email` type) |
| **Code** | Magic Numbers/Strings | Hardcoded literals without named constants |
| **Code** | Comments as Deodorant | Excessive comments explaining bad code instead of refactoring |
| **Code** | Dead Code | Unused imports, unreachable code, commented-out blocks |
| **Testing** | No Tests | Modules with zero test coverage |
| **Testing** | Test-Implementation Coupling | Tests that assert internal implementation details instead of behavior |
| **Testing** | Slow Tests | Tests doing real I/O, database calls, network requests without mocking |
| **Naming** | Vague Names | `Manager`, `Handler`, `Processor`, `Helper`, `Util`, `Service`, `Data`, `Info` used excessively without context |
| **Naming** | Inconsistent Naming | Snake_case and camelCase mixed; different patterns for same concept |
| **Complexity** | Nested Loops (O(n^2)+) | Loop inside loop; forEach inside for; map inside map; nested iteration suggesting polynomial complexity |
| **Complexity** | Repeated Linear Scan | `includes()`/`indexOf()`/`.find()` inside a loop; O(n*m) membership check on list instead of Set/Map |
| **Complexity** | Sort-in-Loop | `.sort()` or `sorted()` called inside iterative code; repeated O(n log n) when sort-once suffices |
| **Complexity** | N+1 Query Pattern | Database/API/HTTP call inside a loop; `fetch`/`query`/`execute`/`findMany` per iteration instead of batch |
| **Complexity** | Render-Path Recompute | `.filter().map().sort()` chains in component render body; expensive transforms without memoization |
| **Complexity** | Pairwise Comparison | Nested iteration comparing every element with every other; O(n^2) when sort+two-pointer would be O(n log n) |
| **Complexity** | Unnecessary Recompute | Same expensive computation repeated without caching; missing `useMemo`/`memo`/lazy eval |
| **Complexity** | Wrong Data Structure | Array used where Set/Map would give O(1) lookup; List where Queue/Heap/Stack is natural fit |

---

## Step 3: Report Generation

Generate the report in this structure:

```markdown
# Architecture Smell Report

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

---

## Executive Summary

[2-3 paragraph summary: architectural style detected, overall health assessment, and top 3-5 critical issues]

---

## 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]

---

## 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]
- **Principle Violated:** [SOLID/DRY/KISS/etc.]
- **Description:** [What was found and why it's a problem]
- **Evidence:** [Code snippet or structure description]
- **Recommendation:** [Specific fix, with refactoring approach]

---

## 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

| 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

### 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

### 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

### Code-Level Smells

#### Long Method
- Methods > 50 lines (or whatever suits the language)
- Deep nesting > 3 levels
- Multiple levels of abstraction mixed
- **Remedy:** Extract methods at same abstraction level, compose

#### Long Parameter List
- Methods with > 4 parameters
- Boolean flags controlling behavior
- **Remedy:** Introduce parameter object, split method, remove flag arguments

#### Duplicated Code
- Identical or near-identical logic in 3+ places
- Copy-paste with slight variations
- **Remedy:** Extract shared method, apply Template Method or Strategy pattern

#### Primitive Obsession
Using primitives instead of domain types:
- `string` for Email, PhoneNumber, URL
- `int` for Money, Age, Quantity
- `decimal` without Currency context
- **Remedy:** Create value objects with validation and behavior

#### Magic Numbers/Strings
- Hardcoded literals without explanation
- `if (status == 3)` instead of `if (status == Status.COMPLETED)`
- **Remedy:** Extract named constants or enums

#### Comments as Deodorant
- Comments that explain what code does (code should be self-documenting)
- Commented-out code blocks
- "TODO" comments accumulating without resolution
- **Remedy:** Refactor to make code clear, delete dead code, track TODOs as issues

#### Dead Code
- Unused imports, variables, functions
- Unreachable branches
- Commented-out code in version control
- **Remedy:** Delete it (git history preserves it if needed)

### Testing Smells

#### No Tests
- Modules with zero test coverage
- Business logic without unit tests
- **Remedy:** Write characterization tests first, then add behavior tests

#### Test-Implementation Coupling
- Tests asserting internal method calls, private state, or implementation details
- Tests breaking on refactoring without behavior changes
- **Remedy:** Test through public APIs, assert behavior not implementation

#### Test Environment Dependency
- Tests depending on file system, network, database, system clock without mocking
- Non-deterministic tests (flaky tests)
- **Remedy:** Use test doubles, control environment, use DI

### Complexity Smells (Algorithmic Anti-Patterns)

Complexity smells indicate code whose runtime grows inefficiently with input size. These are not mere "micro-optimizations" — they are algorithmic choices that cause real performance degradation at scale.

#### Nested Loops (O(n^2) and Worse)
Two or more loops nested inside each other, producing polynomial complexity.
- **Detection:** `for`/`while` inside another `for`/`while`; `forEach`/`map` inside `forEach`/`map`; loop containing another loop (any depth)
- **Impact:** O(n^2) for double-nested, O(n^3) for triple; explodes with moderate data sizes
- **Remedy:**
  - Build a Map/Set index for the inner collection → O(n+m)
  - Sort + two-pointer approach → O(n log n)
  - Group/bucket data before iterating
  - Sweep-line for interval/range problems
- **Correctness checks:** Does order matter? Are there duplicate keys? Is the original picking first/last/all matches?

#### N+1 Query Pattern
A database query, API call, or I/O operation inside a loop body.
- **Detection:** `fetch()`/`axios()`/`query()`/`execute()`/`findMany()`/`findOne()`/`findUnique()`/`select()`/`where()` inside any loop construct
- **Impact:** 1 + N round-trips instead of 1; network latency multiplied by item count
- **Remedy:**
  - Batch fetch by IDs: `SELECT * FROM x WHERE id IN (...)` then join in memory
  - Use ORM eager-loading / `include` / `preload` / DataLoader
  - Bulk API endpoints accepting arrays
  - Preserve: auth filters, tenancy isolation, ordering, pagination, error semantics
- **Correctness checks:** Don't fetch records the original per-item logic wouldn't authorize; preserve missing-record behavior

#### Repeated Linear Scan (Missing Index)
Linear search (`includes`, `indexOf`, `.find`, `in_array`) inside a loop, where a Set/Map would give O(1) lookup.
- **Detection:** `.includes()` / `.indexOf()` / `.find()` / `.findIndex()` / `in_array()` / `contains()` inside a loop body
- **Impact:** O(n*m) instead of O(n+m) — each iteration scans the entire collection
- **Remedy:** Build a `Set` (for membership) or `Map` (for key→value lookup) once before the loop
- **Correctness checks:** Does equality semantics change after Set conversion? JavaScript object identity vs. value equality; Python hashability

#### Sort-in-Loop
Sorting inside a loop body, repeating O(n log n) work unnecessarily.
- **Detection:** `.sort()` / `sorted()` / `sort()` inside any iterative block
- **Impact:** O(k * n log n) instead of O(n log n) — sort repeated k times
- **Remedy:**
  - Sort once outside the loop
  - Maintain a heap (PriorityQueue) if incremental top-K is needed
  - Use binary search/insertion into sorted collection
- **Correctness checks:** Is each intermediate sorted state externally observable? Does comparator depend on loop-local state?

#### Render-Path Recompute (UI Complexity)
Expensive data transformation (filter→map→sort chains) inside UI component render bodies, recomputed on every render.
- **Detection:** `.filter().map().sort().reduce()` chains inside React/Vue/Svelte component function bodies; inside `function Component()` or `const Component = () =>` in JSX/TSX
- **Impact:** Re-derivation on every state change even if inputs unchanged; jank with large collections
- **Remedy:**
  - `useMemo` / `computed` / `derived` with correct dependency arrays
  - Move derivation to selectors, loaders, or server-side
  - Virtualize long lists (windowing)
  - Stabilize callbacks and object props only when child renders are affected
- **Correctness checks:** Dependency arrays must include every semantic input; memoization must not hide mutations of mutable inputs

#### Pairwise Comparison
Comparing every element with every other element using double-nested iteration.
- **Detection:** Two nested loops iterating the same or similar collections, comparing pairs
- **Impact:** O(n^2) for pair matching, overlap detection, conflict checking, nearest-neighbor
- **Remedy:**
  - Sort + two-pointer for pair/range matching
  - Sweep-line for interval overlaps
  - Spatial hashing or grid bucketing for proximity
  - Union-find for connectivity
- **Correctness checks:** Order stability; tie-breaking in equality cases

#### Unnecessary Recompute (Missing Memoization)
Same pure computation repeated with same inputs without caching.
- **Detection:** Identical function calls with same arguments in hot paths; repeated expensive transforms; recursive calls without memoization
- **Impact:** Linear/polynomial wasted work; especially bad with recursive Fibonacci-style patterns (O(2^n) → O(n) with memo)
- **Remedy:** Add memoization/caching with proper invalidation; use `lru_cache`/`memoize`/`useMemo` as appropriate

#### Wrong Data Structure
Using a suboptimal data structure for the access pattern.
- **Detection:**
  - Array/List used for frequent membership tests → should be Set
  - Array/List used for key-value lookups → should be Map/Object
  - Array used as queue with `shift()`/`pop(0)` (O(n) per dequeue) → should use proper Queue
  - Sorted insertion into array (O(n) per insert) → should use Heap
- **Remedy:** Replace with the data structure whose complexity matches the access pattern:
  - Set → O(1) has/add/delete
  - Map → O(1) get/set
  - Heap → O(log n) push/pop for priority
  - Queue/Deque → O(1) enqueue/dequeue

#### What NOT to Flag
- **Cold paths:** Complexity that only runs on startup, config loading, or tiny N (< 100) is rarely worth fixing
- **Intentional tradeoffs:** Clear, readable O(n) code where O(n log n) would add complexity with no measurable gain
- **Already optimized:** Map/Set already in use; batch loading already implemented; memoization already present

### Design Principle Violations

#### SOLID Violations Checklist
- **S (SRP):** Class/module has multiple reasons to change → God Object smell
- **O (OCP):** switch/if-else chains on type codes → Strategy/Polymorphism needed
- **L (LSP):** Subclass changes behavior of base class unexpectedly → Check pre/post conditions
- **I (ISP):** Fat interfaces with methods clients don't use → Split interfaces
- **D (DIP):** High-level modules depending on low-level details → Introduce abstractions

#### Other Principle Violations
- **DRY Violation:** Same knowledge repeated in multiple places
- **KISS Violation:** Over-engineered solutions; premature abstractions
- **YAGNI Violation:** Code for hypothetical future requirements; unused abstractions

---

## Edge Cases & Fallback

| Scenario | Handling |
|----------|----------|
| User doesn't specify scope | Default to recent changes (`git diff`) for repos > 200 files, full analysis otherwise |
| Project has no clear architecture | Report "Big Ball of Mud" with evidence, recommend incremental refactoring |
| Empty/monorepo project | Report that architecture analysis requires code; ask user to specify module |
| Language not supported | Report general structural observations; note language-specific checks are limited |
| Report file path conflicts | Append `-2`, `-3`, etc. to filename |
| User wants a quick check | Run only Critical-level scans, skip Code and Naming categories |
| User wants only one category | Focus analysis on that category, skip others |

---

## Report Output Example

```
🔍 Architecture Smell Analysis Complete

Project: goal-workflow
Style: Modular Monolith (with some layering violations)
Files Analyzed: 47
Health: 🟡 Fair

Critical: 3  |  Warnings: 6  |  Suggestions: 9

🔴 Critical Issues:
  1. Anemic Domain Model — `models/` classes have only getters/setters,
     all logic in `services/`. Violates DDD Rich Domain Model principle.
  2. N+1 Query Pattern — `services/order.ts:142` fetches user per order in loop;
     should batch-load users by IDs (O(n*m) → O(n+m)).
  3. Static Cling — `util/ApiClient.ts` uses all static methods,
     making consumer code untestable.

🟡 Warnings:
  1. God Object — `services/workflow.ts` at 847 lines handles too many concerns
  2. Nested Loop O(n^2) — `analytics.ts:89` pairwise comparison of events;
     sort+two-pointer would be O(n log n)
  3. Leaky Abstraction — `repositories/user.ts` exposes MongoDB query syntax
  4. Duplicated Code — validation logic duplicated across 4 controllers
  5. Circular Dependency — `auth` ↔ `user` modules depend on each other
  6. Magic Numbers — ~23 hardcoded values without named constants

Full report: tasks/smell-report-2026-05-27-1530.md
```