# Architecture Review

> Use when you need to evaluate architecture, coupling, duplication, integration fit, and whether to keep, adapt, or replace existing structures.

- Skill: `fluxonlab/architecture-review` (Agent Skill)
- Install (CLI): `npx skillmds@latest add fluxonlab/architecture-review`
- Raw SKILL.md: https://api.skillmd.com/api/skills/fluxonlab/architecture-review/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Integrations & APIs
- Author: FluxonLab (https://skillmd.com/u/fluxonlab)
- Updated: 2026-09-17
- Page: https://skillmd.com/skills/fluxonlab/architecture-review

---


# Architecture Review

## Purpose

Evaluate architecture, coupling, duplication, integration fit, and whether to keep, adapt, or replace existing structures. Every Keep, Adapt, or Replace decision must be justified with file-level evidence — churn, blast radius, coupling — not intuition. The default bias is toward the smallest change that fits the existing structure; a rewrite is recommended only when the explicit rule below is met, and never by standing up a parallel app or repo.

## When to use

- A new feature or refactor touches multiple modules and you need to confirm it follows the existing dependency direction before committing to a plan.
- Circular dependencies, god modules, or competing implementations of the same concern are suspected.
- A pull request or design proposal introduces a second way of solving something already solved (two HTTP clients, two state stores, conflicting conventions).
- Before a larger refactor, to establish which components to Keep, Adapt, or Replace and prove the blast radius is bounded.

## When not to use

- The task is unrelated to core operations work.
- The work would require production deploys, destructive data actions, or secret disclosure.
- You first need a structural map or a runnability baseline — use `codebase-cartography` or `repo-diagnostics`, then return here.
- A narrower skill or existing project instruction already covers the need.

## Procedure

1. **Build a module map.** List the top-level directories, each one's responsibility, and the dependency direction between them. Locate the boundary between UI, domain or business logic, data access, and infrastructure.
2. **Detect cycles and illegal dependencies.** Find inversions where lower layers depend on higher ones (UI importing the DB client, domain importing a framework, a shared `core` importing a `routes` module).
3. **Find duplication.** Identify parallel implementations of one concern (two HTTP clients, two state stores, two date utils), copy-pasted logic, and competing patterns for the same job.
4. **Assess integration fit for the change.** Does the proposed work follow existing data-fetching, error-handling, auth, and state conventions, or introduce a second way of doing the same thing?
5. **Decide Keep, Adapt, or Replace** for each questioned component using the rule below, justified with evidence (churn, blast radius, coupling).
6. **Surface the highest-risk files** (high fan-in plus high churn) and state the smallest viable change that fits the existing structure.

## Concrete checks

Dependency direction:
- Nothing in `domain` or `core` imports from `ui`, `routes`, or a framework package.
- Lower layers do not import higher ones; the dependency graph points one way.
- No business rule is embedded directly in a UI component or route handler.

Duplication:
- No two files export the same function name for the same purpose.
- There is one HTTP layer, not an `axios` wrapper plus bare `fetch` calls.
- There is one state solution for a given slice of data, not Redux plus Zustand plus Context.

Coupling and fit:
- Changing one feature does not force edits across five or more unrelated files.
- A new module follows the existing data-fetching, error-handling, and config conventions.
- No god module is imported by twenty or more files.
- The proposed change reuses an existing seam rather than introducing a parallel one.

Decision readiness:
- Each questioned component has churn and fan-in evidence attached.
- Any Replace verdict carries a concrete migration path and a rollback.
- The smallest change that fits the structure is identified, not just the cleanest.

## Keep / Adapt / Replace rule

- **Keep:** the pattern is consistent, tested, and the change fits without contortion.
- **Adapt:** the structure is mostly right but needs a seam — extract an interface, add an adapter, narrow a type. This is the lowest-risk default.
- **Replace:** only when the existing structure blocks the goal, is duplicated, or is unmaintained, AND a migration path plus rollback are defined. Never replace by standing up a parallel app or repo.

## Commands

```bash
# --- cycles / orphans ---
# circular deps and orphans (JS/TS)
npx madge --circular --extensions ts,tsx,js,jsx src
npx madge --orphans --extensions ts,tsx src

# --- dependency direction ---
# does domain/core import from ui/routes? (inversion)
rg -n "from ['\"].*(ui|routes|components|pages)" src/domain src/core 2>/dev/null

# boundary leaks: DB calls inside components/handlers
rg -n 'prisma\.|db\.query|sql`' src/components src/app 2>/dev/null

# --- duplication ---
# duplicate exported symbols (parallel implementations)
rg -oN 'export (?:function|const|class) (\w+)' -r '$1' src | sort | uniq -d

# competing HTTP or state libraries in the same codebase
rg -n "from ['\"](axios|node-fetch|redux|zustand|jotai|recoil)" src | sort | uniq -c

# --- god modules / fan-in ---
# most-imported local modules (god-module candidates)
rg -oN "from ['\"](\.[^'\"]+)['\"]" -r '$1' src | sort | uniq -c | sort -rn | head -20

# --- churn / risk ---
# high-change files are refactor risk
git log --since=6.months --name-only --pretty=format: | sort | uniq -c | sort -rn | head -20

# --- convention drift ---
# multiple error-handling shapes across the codebase
rg -n 'try\s*\{|\.catch\(|Result<|Either<|throw new' . | head

# multiple data-fetching patterns for the same concern
rg -n 'useQuery|useSWR|getServerSideProps|loader\(|fetch\(' . | head

# --- layer boundaries ---
# framework types leaking into the domain layer
rg -n "import .*(react|express|next|@nestjs)" src/domain src/core 2>/dev/null

# --- duplication depth ---
# copy-pasted blocks via repeated function signatures
rg -oN 'function (\w+)\(' -r '$1' src | sort | uniq -c | sort -rn | head
```

## Common issues & anti-patterns

- **Dependency inversion:** a `domain` module imports a React component or an Express type, so the business logic can no longer be reused or tested in isolation.
- **Parallel HTTP layers:** half the app uses an `axios` instance with interceptors and the other half uses bare `fetch`, so retries, auth headers, and error handling diverge.
- **Competing state stores:** the same server data lives in both a Redux slice and a Zustand store, and they drift out of sync.
- **God util:** a `helpers.ts` imported by thirty files; any change risks an unrelated regression and the file resists testing.
- **Replace-by-parallel-app:** instead of adapting the existing structure, a second app or package is stood up "to do it cleanly", doubling the maintenance surface and the drift.
- **Silent convention fork:** a new feature introduces its own error-handling shape, so error responses are now inconsistent across endpoints.
- **Unbounded blast radius:** a change scoped to one feature quietly edits eight shared files — a signal the seam is in the wrong place.
- **Circular dependency between layers:** `service` imports `controller` and `controller` imports `service`, creating load-order fragility and untestable units.
- **Premature abstraction:** a generic base class or plugin system built for one caller, adding indirection that obscures the single real path through the code.
- **Adapter avoided:** a new third-party client wired directly into twelve call sites instead of behind one adapter, so swapping or mocking it later means touching all twelve.
- **Replace without rollback:** a proposal to swap a core module with no migration sequence or fallback, so a mid-migration failure leaves the app in an unshippable state.
- **Leaky repository:** a data-access layer that returns raw ORM entities up to the UI, so the database schema and the view are coupled and a column rename breaks the frontend.
- **Convention by copy-paste:** each new endpoint copies the previous one's structure including its quirks, so a single bad pattern propagates instead of a shared helper.
- **Hidden temporal coupling:** two calls that must happen in a specific order with nothing in the types or signatures enforcing it, so a reordering silently breaks behavior.
- **Shotgun surgery:** adding one field requires edits in the model, the DTO, the mapper, the validator, and three components, signaling responsibilities are smeared across layers.
- **Anemic seam:** an interface exists but every implementation is identical and there is only one, so the abstraction adds indirection without buying flexibility.
- **Framework lock in the core:** business rules written directly against a framework's request or ORM objects, so the logic cannot be unit-tested without booting the framework.
- **Duplicated date/money logic:** two or three hand-rolled helpers for the same formatting or arithmetic, which drift apart and produce inconsistent values across screens.
- **Conditional sprawl:** a single function that branches on a type flag everywhere instead of polymorphism, so each new variant edits many call sites rather than adding one implementation.

## When a rewrite is genuinely warranted

Recommend Replace only when all three hold: the existing structure provably blocks the goal (not merely inconvenient), the blast radius of adapting it exceeds the cost of replacing it, and a staged migration with a rollback at each step is defined. Even then, replace the component in place behind its existing seam — never by standing up a second app, repo, or framework alongside the first.

## Required output

Return: a module and dependency map; coupling and duplication findings with `file:line` evidence and severity; a Keep, Adapt, or Replace verdict per questioned component with rationale; the high-risk file list (high fan-in plus high churn); and the smallest change that fits the existing architecture. Do not recommend a rewrite unless the Keep/Adapt/Replace rule is met, and when Replace is chosen include the migration path and rollback.

## Safety

- Review-only by default; do not begin large refactors without an approved plan and a rollback path.
- Do not introduce a parallel framework, second app, or duplicate repo "to do it cleanly".
- Base every claim on file evidence, not assumption.
- Flag — do not silently make — cross-cutting changes that exceed the task scope.
- Redact any secret-like values surfaced while reading config.

## Completion criteria

Done means the architecture is mapped from evidence; coupling and duplication findings are concrete with `file:line`; each questioned component has a Keep, Adapt, or Replace decision with rationale (and any Replace carries a migration plus rollback path); the high-risk files are listed; and the recommended path is the smallest one that fits the existing structure.