smell

---
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, 代码坏味道, 架构坏味道, 反模式, 找出坏味道, 复杂度分析.
license: MIT
---

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

| 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 |
| **Architecture** | Over-Layered Architecture | Excessive layers/tiers that add pass-through code with no real value |
| **Architecture** | Over-Abstraction | So many indirections/interfaces/generics that you get lost following the code |
| **Architecture** | Futuristic Architecture | Speculative flexibility for requirements that may never come (predicting the future) |
| **Architecture** | Technology-Enthusiast Architecture | Shiny/unproven tech adopted in production because it's new, not because it fits |
| **Architecture** | Overkill Architecture | Heavyweight architecture/tech thrown at a simple problem |
| **Architecture** | Cloud/Visio Architecture | Diagrams disconnected from the actual code and runtime reality |
| **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) |
| **Design** | Switch Statements | Same `switch`/if-else chain on a type code appearing in multiple places; should be polymorphism |
| **Design** | Refused Bequest | Subclass inherits methods/fields it doesn't use or overrides them to throw/no-op |
| **Design** | Alternative Classes w/ Different Interfaces | Two classes do the same thing but have differently-named methods |
| **Design** | Parallel Inheritance Hierarchies | Creating a subclass in one hierarchy forces a matching subclass in another |
| **Design** | Speculative Generality | Unused abstract classes, hooks, params, or generics "for future needs" (YAGNI) |
| **Design** | Incomplete Library Class | Wrapping/patching a third-party class because it lacks needed methods |
| **Cohesion** | Divergent Change | One module changed for many unrelated reasons (opposite of Shotgun Surgery) |
| **Cohesion** | Data Class | Class with only fields + getters/setters, no behavior (anemic data bag) |
| **Cohesion** | Lazy Class | Class/module that does too little to justify its existence |
| **Coupling** | Inappropriate Intimacy | Two classes access each other's private/internal parts too much |
| **Coupling** | Message Chains | Long call chains `a.getB().getC().getD()` (Law of Demeter violation) |
| **Coupling** | Middle Man | Class that only delegates every call to another class |
| **Code** | Temporary Field | Instance field set/used only in certain circumstances, empty otherwise |
| **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 |
| **Readability** | Deep Nesting (Arrow Anti-Pattern) | Loops/conditionals nested > 3 levels deep; rightward-drifting "arrow" shape hard to trace |
| **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

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

| Principle | Confirming evidence | Common false positive / constraint |
|-----------|---------------------|------------------------------------|
| **SOLID** | A design problem spans multiple SOLID lenses or no narrower lens is reliable | Do not duplicate a specific SRP/OCP/DIP finding |
| **DRY** | The same business rule or knowledge must change in multiple places | Similar syntax that is expected to evolve independently |
| **KISS** | Extra layers, indirection, or machinery add cost without observable leverage | A small abstraction that removes real complexity |
| **YAGNI** | Unused extension points, parameters, adapters, or speculative requirements | A tested seam required by an existing boundary or change |
| **SRP** | Multiple independent reasons to change, supported by responsibilities or change history | File size or method count alone |
| **Open/Closed (OCP)** | Adding a known variant repeatedly modifies stable branching logic | One simple, local conditional |
| **Dependency Inversion (DIP)** | High-level policy directly depends on concrete infrastructure, harming replacement or testing | Adding an interface for a single stable implementation |
| **Composition** | Inheritance causes unwanted coupling, refused behavior, or inseparable variation axes | Replacing every valid inheritance relationship mechanically |
| **Separation of Concerns** | Business policy, I/O, presentation, or persistence concerns leak across boundaries | A deliberately thin boundary adapter |
| **Fail Fast** | Invalid input, state, or dependency propagates until a distant operation fails | Intentional aggregation, retry, or deferred validation semantics |
| **Measure First** | A performance, scale, or optimization claim lacks a baseline or representative workload | Static complexity reported as a measured bottleneck |

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:

```markdown
# 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](#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)

#### Parallel Inheritance Hierarchies
Every time you add a subclass to one hierarchy, you must add one to another:
- `Shape`/`ShapeRenderer`, `Employee`/`EmployeePermission` growing in lockstep
- **Remedy:** Merge hierarchies or make one hierarchy reference the other instead of mirroring it

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

#### Deep Nesting (Arrow Anti-Pattern)
Loops and conditionals nested so deeply the code drifts rightward into an "arrow" shape:
- `if { if { for { if { ... } } } }` — hard to trace which conditions hold at any point
- Usually > 3 levels of indentation in one function
- **Remedy:** Guard clauses / early returns, extract nested blocks into methods, invert conditions, replace conditional with polymorphism

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

#### Data Class
A class that is only fields plus getters/setters, with no meaningful behavior:
- A "data bag" other classes reach into and manipulate from outside
- Closely related to Anemic Domain Model at the class level
- **Remedy:** Move the behavior that operates on the data into the class ("Tell, Don't Ask")

#### Lazy Class
A class/module that no longer does enough to justify its existence:
- Left over after refactoring, or an abstraction that never grew
- **Remedy:** Inline it into its caller or collapse the hierarchy

#### Speculative Generality
Abstractions, hooks, parameters, or generics added for hypothetical future needs:
- Unused abstract base classes, unused parameters, "just in case" configuration
- Violates YAGNI
- **Remedy:** Remove unused abstraction; add it when a real second use case appears

#### Temporary Field
An instance field that is only set/used in certain circumstances and empty otherwise:
- Fields populated only during one algorithm, confusing readers the rest of the time
- **Remedy:** Extract the field + the methods that use it into their own class (Extract Class / introduce a Method Object)

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

A static complexity pattern is a candidate, not proof of a bottleneck. Apply **Measure First**:

1. Establish actual input size, call frequency, I/O latency, and whether the path is hot.
2. Prefer a profiler, representative benchmark, query log, trace, or explicit operation count.
3. Promote the candidate to a finding only when the workload and impact justify it.
4. Re-measure the same workload after a fix; without a baseline, do not claim a performance improvement.

For every complexity candidate, state **what to measure**, **when it becomes a finding**, and **when not to flag it**. Keep the Big-O analysis and correctness checks below, but do not infer severity from syntax alone.

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

Use the canonical matrix in Step 3 as the reporting contract. These design checks refine candidate detection; they do not create one finding per principle.

- **SOLID umbrella:** Use only when multiple SOLID concerns share one root cause or a narrower lens is not reliable.
- **SRP:** Confirm independent reasons to change; size alone is insufficient.
- **OCP:** Confirm repeated edits for a real variant; do not replace a simple local conditional with speculative polymorphism.
- **LSP:** Subtypes must preserve the base contract's preconditions, postconditions, and invariants. Report as `SOLID/LSP`.
- **ISP:** Confirm clients depend on methods they do not use. Report as `SOLID/ISP`.
- **DIP:** Confirm high-level policy is coupled to concrete mechanism in a way that harms testing or replacement; a single implementation does not automatically justify an interface.
- **DRY:** Deduplicate shared knowledge, not coincidentally similar syntax.
- **KISS and YAGNI:** Reject abstractions whose future variation or leverage is not demonstrated.
- **Composition:** Prefer it when inheritance exposes unwanted behavior or binds independent variation axes—not as a mechanical rule.
- **Separation of Concerns:** Confirm policy, presentation, persistence, or I/O leakage across an intended boundary.
- **Fail Fast:** Detect invalid state near its source while preserving established error, retry, transaction, and cleanup semantics.
- **Measure First:** Lower unmeasured optimization claims to candidates instead of reporting a second "principle violation."

#### Object-Orientation Abusers (from Fowler / refactoring.guru)

##### Switch Statements (Type-Code Conditionals)
Repeated `switch`/if-else chains that branch on a type code or enum:
- The same conditional structure duplicated in several places
- Adding a new type forces editing every switch (OCP violation)
- **Remedy:** Replace conditional with polymorphism (Strategy/State), or Replace Type Code with Subclasses

##### Refused Bequest
A subclass inherits methods/fields it doesn't need:
- Overrides inherited methods to throw, no-op, or do something unrelated
- Signals the inheritance relationship is wrong
- **Remedy:** Push down unused members, or replace inheritance with delegation

##### Alternative Classes with Different Interfaces
Two classes perform the same role but expose differently-named methods:
- `sort()` vs `arrange()`, `getUser()` vs `fetchUser()` for interchangeable classes
- **Remedy:** Unify the interface (rename methods, extract a common superclass/interface)

##### Incomplete Library Class
A third-party/library class lacks methods you need and can't be modified:
- Scattered helper functions or copy-paste wrappers around the library
- **Remedy:** Introduce a Foreign Method or wrap it in an adapter/local extension class

> Other refactoring.guru smells are documented in their thematic sections above:
> **Divergent Change**, **Data Class**, **Lazy Class**, **Speculative Generality**,
> **Temporary Field**, **Parallel Inheritance Hierarchies**, **Inappropriate Intimacy**,
> **Message Chains**, and **Middle Man**.

---

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