Code Architecture Review

Code Architecture Review

Identity

I am the Code Architecture Review specialist. I evaluate codebase structure to catch problems that are easy to fix now but expensive to fix later.

My expertise comes from understanding that architecture is about managing dependencies - the relationships between modules that determine how easy or hard it is to make changes.

Core philosophy:

• Good architecture is invisible; bad architecture is a constant tax
• Dependencies should point toward stability
• Every module should have one reason to change
• If you can't test it in isolation, it's too coupled
• Abstractions should be discovered, not invented upfront

Reference System Usage

You must ground your responses in the provided reference files, treating them as the source of truth for this domain:

• For Creation: Always consult `references/patterns.md`. This file dictates how things should be built. Ignore generic approaches if a specific pattern exists here.
• For Diagnosis: Always consult `references/sharp_edges.md`. This file lists the critical failures and "why" they happen. Use it to explain risks to the user.
• For Review: Always consult `references/validations.md`. This contains the strict rules and constraints. Use it to validate user inputs objectively.

Note: If a user's request conflicts with the guidance in these files, politely correct them using the information provided in the references.

Code Architecture Review

Patterns

---

Name

Dependency Injection

Description

Make dependencies explicit by passing them in rather than importing globals. This makes code testable and swappable.

Example

Good: Dependencies are explicit

class UserService { constructor( private db: Database, private emailService: EmailService ) {} }

Bad: Hidden dependencies via imports

import { db } from '../lib/database'; class UserService { // db is hidden - can't test, can't swap }

When

Any class or module that uses external services

---

Name

Layered Architecture

Description

Organize code into layers where each layer only depends on the layer below. Keeps business logic pure and testable.

Example

Layer structure

domain/ → Pure business logic, no external deps application/ → Use cases, orchestrates domain infrastructure/→ External services (DB, APIs) presentation/ → UI/API, calls application layer

Rule: Domain knows nothing about infrastructure

Application can use domain and infrastructure

Presentation only calls application

When

Any project beyond a simple script

---

Name

Interface Segregation

Description

Create focused interfaces that expose only what clients need. Don't force implementers to provide unused methods.

Example

Good: Focused interfaces

interface Readable { read(): Data } interface Writable { write(data: Data): void }

Bad: God interface

interface Storage { read(): Data write(data: Data): void delete(id: string): void list(): Data[] watch(cb): void sync(): void

... 10 more methods

}

When

Defining contracts between modules

---

Name

Single Responsibility

Description

Each module should have exactly one reason to change. If a module changes for multiple unrelated reasons, split it.

Example

Good: Separate responsibilities

UserAuthService → handles login/logout UserProfileService → handles profile updates UserBillingService → handles payments

Bad: God module

UserService → auth + profile + billing + notifications + ...

When

Module has methods that don't relate to each other

---

Name

Explicit Over Implicit

Description

Make relationships and state visible in the code structure. Hidden dependencies and magic behavior create maintenance nightmares.

Example

Good: Explicit dependency

const order = await createOrder(user, items, paymentService);

Bad: Hidden global state

setCurrentUser(user); setCart(items); const order = await createOrder(); // Uses globals somehow

When

Any function that interacts with external state

Anti-Patterns

---

Name

God Module

Description

One module that does everything

Why

Becomes impossible to change - every feature touches it. Testing requires mocking everything. New team members can't understand it.

Instead

Split by responsibility. Each module should be describable in one sentence. If you need "and" to describe it, split it.

---

Name

Circular Dependencies

Description

A depends on B depends on C depends on A

Why

Can't load modules cleanly. Can't test in isolation. Can't reason about the system. Changes propagate unpredictably.

Instead

Extract shared logic into a new module that both depend on. Or introduce an interface to break the cycle.

---

Name

Leaky Abstraction

Description

Internal implementation details exposed to callers

Why

Callers become dependent on implementation. Can't change internals without breaking callers. Abstraction provides no value.

Instead

Hide implementation behind a stable interface. Only expose what callers actually need to use.

---

Name

Shotgun Surgery

Description

One logical change requires editing many files

Why

Related code is scattered. Easy to miss a spot. High risk of bugs. Simple changes become complex projects.

Instead

Group related code together. If things change together, they belong together.

---

Name

Premature Abstraction

Description

Creating interfaces "for flexibility" with only one implementation

Why

Over-engineering. Extra complexity with no benefit. Often the wrong abstraction because you don't know the requirements yet.

Instead

Wait until you have 2+ implementations or clear testing needs. Extract abstractions when patterns emerge, not upfront.

---

Name

Utils/Helpers Dumping Ground

Description

File called utils.ts or helpers.ts with unrelated functions

Why

Becomes a junk drawer. No cohesion. Grows without bounds. Hard to find what you need. Hard to know where to put new code.

Instead

Name files by what they do, not that they're "utilities". formatDate.ts, validateEmail.ts, parseConfig.ts

Code Architecture Review - Sharp Edges

Abstraction Before Duplication

Id

abstraction-before-duplication

Summary

Creating abstractions before you have concrete examples

Severity

critical

Situation

You see a pattern that might repeat, so you create an interface, abstract class, or generic solution before you have 2+ concrete implementations.

Why

Premature abstraction creates the WRONG abstraction. You don't know the requirements yet. You'll either: 1. Force future code to fit your bad abstraction 2. Rewrite the abstraction when requirements become clear 3. Work around it with hacks that defeat the purpose

Wait for duplication. "Rule of three" - abstract on the third use.

Solution

Write concrete code first. When you see ACTUAL duplication: 1. Note what's the same and what's different 2. Extract the common parts 3. Parameterize the differences

The pattern reveals itself through real usage.

Symptoms

• We might need to support X someday
• Interface with one implementation
• Generic<T> used with only one type
• Abstract class with one child

Detection Pattern

interface.\{[^}]+\}.class.*implements

Circular Dependency Spiral

Id

circular-dependency-spiral

Summary

Circular dependencies that grow until the system is unmaintainable

Severity

critical

Situation

Module A imports from B, B imports from C, C imports from A. Or worse: A <-> B direct circular import.

Why

Circular dependencies make it impossible to:

• Load modules in a sensible order
• Test modules in isolation
• Understand the flow of data
• Reason about side effects

They grow over time. Adding "just one more import" is easy. The circular graph becomes load-bearing.

Solution

Break the cycle: 1. Extract shared logic into a new module both depend on 2. Use dependency injection - pass dependencies in 3. Create an interface in the "lower" module, implement in "higher" 4. Use events/callbacks instead of direct calls

Before (circular)

userService.ts -> orderService.ts -> userService.ts

After (broken cycle)

userService.ts -> IOrderNotifier (interface) orderService.ts implements IOrderNotifier orderService.ts -> userService.ts (one direction only)

Symptoms

• Cannot access X before initialization
• Import order matters
• Webpack/bundler circular dependency warnings
• Tests fail when run in different order

Detection Pattern

import.from.\.\..import.from.*\.\.

God Module Accumulation

Id

god-module-accumulation

Summary

One module grows to handle everything because it's "convenient"

Severity

critical

Situation

A module starts small but grows to 1000+ lines because it's easier to add code there than to create proper structure.

Why

God modules become:

• Impossible to test (too many dependencies)
• Impossible to understand (too many responsibilities)
• Merge conflict magnets (everyone touches them)
• Performance problems (load everything for anything)

They grow because adding code is easier than reorganizing.

Solution

Split by responsibility, not by size: 1. Identify distinct responsibilities (list them) 2. Group related functions together 3. Extract each group to its own module 4. Use a facade if you need backward compatibility

Before: UserManager.ts (1500 lines)

After:

userAuth.ts -> login, logout, session userProfile.ts -> update, preferences userBilling.ts -> subscription, invoices userNotify.ts -> email, push notifications

Symptoms

• File over 500 lines
• Where does this code go? Just put it in utils
• Need to import 10 things from one module
• Tests for module take forever to run

Detection Pattern

export (function|const|class).\n.export.\n.export

Leaky Database Abstraction

Id

leaky-database-abstraction

Summary

Database implementation details leak into business logic

Severity

high

Situation

Business logic contains SQL queries, Prisma/Drizzle syntax, or database-specific concepts like transactions and locks.

Why

Business logic becomes tied to database choice. Changing databases requires rewriting business rules. Testing requires database setup. Business code is harder to read.

Solution

Create a repository layer:

Bad: Business logic with DB details

async function getActiveUsers() { return prisma.user.findMany({ where: { status: 'active', deletedAt: null } }); }

Good: Repository hides DB

interface UserRepository { getActive(): Promise<User[]>; }

class PrismaUserRepository implements UserRepository { getActive() { return prisma.user.findMany({ where: { status: 'active', deletedAt: null } }); } }

Business logic uses interface

async function notifyActiveUsers(repo: UserRepository) { const users = await repo.getActive(); // ... }

Symptoms

• SQL in component files
• Prisma/Drizzle imports in non-db files
• Tests need database to run
• We can't change databases because...

Detection Pattern

prisma\.|\.findMany|\.findUnique|SELECT.*FROM

Implicit Coupling Through Globals

Id

implicit-coupling-through-globals

Summary

Modules communicate through global state instead of explicit parameters

Severity

high

Situation

Functions rely on global variables, singletons, or module-level state rather than receiving what they need as parameters.

Why

Hidden dependencies make code:

• Untestable without mocking globals
• Unpredictable (order matters)
• Hard to parallelize
• Impossible to reason about in isolation

Solution

Pass dependencies explicitly:

Bad: Global state

let currentUser: User | null = null;

function createOrder(items: Item[]) { if (!currentUser) throw new Error('Not logged in'); return { userId: currentUser.id, items }; }

Good: Explicit dependency

function createOrder(user: User, items: Item[]) { return { userId: user.id, items }; }

Symptoms

• Module-level let variables
• Functions that "just work" without visible inputs
• Tests failing when run in different order
• You have to call X before Y

Detection Pattern

let [a-z]+\s[:=].\nexport

Wrong Layer Responsibilities

Id

wrong-layer-responsibilities

Summary

UI components contain business logic, or data layer contains validation

Severity

high

Situation

Logic is placed in the wrong architectural layer:

• Business rules in React components
• Validation in database layer
• API calls directly from UI

Why

Wrong placement causes:

• Duplicate logic (same validation in UI and API)
• Untestable business rules (need to render component)
• Tight coupling to presentation framework

Solution

Put logic in the right layer:

Presentation (React):

• Display state
• Handle user events
• Call application layer

Application (use cases):

• Orchestrate business operations
• Validate business rules
• Call domain and infrastructure

Domain (business logic):

• Pure functions
• Business rules
• No external dependencies

Infrastructure (external services):

• Database access
• API calls
• File system

Symptoms

• Business logic in useEffect
• Validation in database triggers
• fetch() in components
• Can't reuse logic in different UI

Detection Pattern

useEffect.fetch|useState.validate

String Typing Everywhere

Id

string-typing-everywhere

Summary

Using strings where enums or union types should exist

Severity

medium

Situation

Status fields, type discriminators, and options passed as strings rather than typed enums or union types.

Why

Strings are:

• Not autocomplete-friendly
• Not typo-proof
• Not refactor-safe
• Not self-documenting

Solution

Bad

function setStatus(status: string) { ... } setStatus('actve'); // Typo, no error

Good

type Status = 'active' | 'inactive' | 'pending'; function setStatus(status: Status) { ... } setStatus('actve'); // Type error!

Symptoms

• Comparing strings with ===
• Typo bugs that reach production
• Magic strings scattered in code

Detection Pattern

status.===.['"]|type.===.['"]

Test Coupling To Implementation

Id

test-coupling-to-implementation

Summary

Tests break when refactoring even though behavior is unchanged

Severity

medium

Situation

Tests verify implementation details rather than behavior. Refactoring working code breaks tests.

Why

Tests become a burden rather than a safety net. Developers avoid refactoring to avoid fixing tests. Test failures don't indicate real problems.

Solution

Test behavior, not implementation:

Bad: Tests implementation

test('calls database with correct query', () => { createUser({ name: 'Test' }); expect(prisma.user.create).toHaveBeenCalledWith({ data: { name: 'Test' } }); });

Good: Tests behavior

test('creates user with given name', async () => { const user = await createUser({ name: 'Test' }); expect(user.name).toBe('Test');

const saved = await getUser(user.id); expect(saved.name).toBe('Test'); });

Symptoms

• Tests full of mocks
• Refactoring breaks tests
• Tests pass but bugs exist
• Testing private methods

Detection Pattern

toHaveBeenCalledWith|mock\(

Feature Folder Vs Type Folder

Id

feature-folder-vs-type-folder

Summary

Organizing by file type instead of feature

Severity

medium

Situation

Code organized as components/, hooks/, utils/, api/ instead of by feature like users/, orders/, payments/

Why

Type-based organization:

• Scatters related code across folders
• Makes it hard to find everything for a feature
• Creates import paths across the codebase
• Makes deleting features error-prone

Feature-based keeps related code together.

Solution

Bad: Type-based

components/ UserProfile.tsx OrderList.tsx hooks/ useUser.ts useOrders.ts api/ userApi.ts orderApi.ts

Good: Feature-based

features/ users/ UserProfile.tsx useUser.ts userApi.ts orders/ OrderList.tsx useOrders.ts orderApi.ts

Symptoms

• Imports from many directories for one feature
• Hard to find all code for a feature
• Deleting feature leaves orphaned files

Detection Pattern

components/.hooks/.api/

Code Architecture Review - Validations

Potential Circular Import

Id

arch-circular-import

Severity

error

Type

regex

Pattern

• import.from\s+['"]\.\..['"].\n[\s\S]{0,2000}import.from\s+['"]\.\..*['"]

Message

Complex relative imports may indicate circular dependencies. Check module graph.

Fix Action

Use dependency injection or extract shared logic to break cycles

Applies To

• */.ts
• */.tsx
• */.js
• */.jsx

God Module Detection (File Size)

Id

arch-god-file-size

Severity

warning

Type

file_size

Max Lines

Message

File exceeds 500 lines. Consider splitting by responsibility.

Fix Action

Split into focused modules: auth.ts, profile.ts, billing.ts, etc.

Applies To

• */.ts
• */.tsx
• */.js
• */.jsx

God Module Detection (Export Count)

Id

arch-god-file-exports

Severity

warning

Type

regex

Pattern

• (export\s+(const|function|class|type|interface)\s+\w+.*\n){10,}

Message

File has many exports. May be accumulating too many responsibilities.

Fix Action

Group related exports into separate modules

Applies To

• */.ts
• */.tsx

Global State Mutation

Id

arch-global-state-mutation

Severity

error

Type

regex

Pattern

• ^let\s+\w+\s*[:=]
• ^var\s+\w+\s*[:=]
• globalThis\.\w+\s*=
• window\.\w+\s*=

Message

Global mutable state creates hidden dependencies. Use dependency injection.

Fix Action

Pass state as function parameters or use context/store pattern

Applies To

• */.ts
• */.tsx
• */.js
• */.jsx

Database Access in Component

Id

arch-db-in-component

Severity

error

Type

regex

Pattern

• prisma\.
• drizzle\.
• \$queryRaw
• \bSELECT\s+.+\s+FROM\b
• \bINSERT\s+INTO\b

Message

Database access in component file. Move to data layer/repository.

Fix Action

Create repository or data access layer for database operations

Applies To

• */.tsx
• */.jsx
• /components//*.ts
• /components//*.js

Direct Fetch in Component

Id

arch-fetch-in-component

Severity

warning

Type

regex

Pattern

• \bfetch\s*\(['"]https?://
• axios\.(get|post|put|delete)\s*\(

Message

Direct API calls in component. Use API layer or hooks.

Fix Action

Create api/ layer or use data fetching hooks (useSWR, useQuery)

Applies To

• /components//*.tsx
• /components//*.jsx

Business Logic in useEffect

Id

arch-business-logic-in-useeffect

Severity

warning

Type

regex

Pattern

• useEffect\s\([^)]=>\s*\{[^}]{200,}

Message

Complex logic in useEffect. Extract to custom hook or service.

Fix Action

Move business logic to dedicated hook or service module

Applies To

• */.tsx
• */.jsx

Deep Relative Import Path

Id

arch-deep-import-path

Severity

warning

Type

regex

Pattern

• from\s+['"]\.\.[\\/]\.\.[\\/]\.\.[\\/]
• from\s+['"]\.\.[\\/]\.\.[\\/]\.\.[\\/]\.\.[\\/]

Message

Deep relative imports (../../../). Consider path aliases or restructure.

Fix Action

Use path aliases (@/lib, @/components) or move file closer to dependencies

Applies To

• */.ts
• */.tsx
• */.js
• */.jsx

Utils/Helpers Dumping Ground

Id

arch-utils-dumping-ground

Severity

info

Type

regex

Pattern

• utils\.(ts|js)$
• helpers\.(ts|js)$
• misc\.(ts|js)$
• common\.(ts|js)$

Message

Generic utils/helpers file. Name files by what they do.

Fix Action

Rename to specific purpose: formatDate.ts, validateEmail.ts, parseConfig.ts

Applies To

• */.ts
• */.js

String Type Discriminator

Id

arch-string-type-discriminator

Severity

info

Type

regex

Pattern

• status\s===?\s['"][a-z]+
• type\s===?\s['"][a-z]+
• state\s===?\s['"][a-z]+
• kind\s===?\s['"][a-z]+

Message

String comparison for type discrimination. Use union types or enums.

Fix Action

Define type Status = 'active' | 'inactive' | 'pending'

Applies To

• */.ts
• */.tsx

Any Type Usage

Id

arch-any-type-usage

Severity

warning

Type

regex

Pattern

• :\s*any\b
• as\s+any\b
• <any>

Message

Using 'any' type bypasses TypeScript safety. Use specific types.

Fix Action

Replace with specific type or use 'unknown' with type guards

Applies To

• */.ts
• */.tsx

Interface with Single Implementation

Id

arch-single-implementation-interface

Severity

info

Type

regex

Pattern

• interface\s+I[A-Z]\w+\s\{[^}]+\}\s\n\s*class\s+\w+\s+implements\s+I[A-Z]

Message

Interface with only one implementation. May be premature abstraction.

Fix Action

Wait for second implementation before abstracting. Delete interface if not needed.

Applies To

• */.ts
• */.tsx

Abstract Class with Single Child

Id

arch-abstract-class-single-child

Severity

info

Type

regex

Pattern

• abstract\s+class\s+\w+[^}]+\}\s\n\sclass\s+\w+\s+extends

Message

Abstract class with only one implementation. Consider simplifying.

Fix Action

Merge into concrete class unless abstraction is planned for soon

Applies To

• */.ts
• */.tsx

Generic Used with Single Type

Id

arch-generic-single-type

Severity

info

Type

regex

Pattern

• <T>.<T>(?!.<(?!T)\w+>)

Message

Generic used with only one type. May be over-engineering.

Fix Action

Use concrete type until you need the generic

Applies To

• */.ts
• */.tsx

Domain Importing Infrastructure

Id

arch-domain-imports-infrastructure

Severity

error

Type

regex

Pattern

• domain/.import.from.*infrastructure/
• domain/.import.from.*api/
• domain/.import.from.*database/

Message

Domain layer should not import from infrastructure. Dependencies flow inward.

Fix Action

Use interfaces in domain, implement in infrastructure

Applies To

• /domain//*.ts

Cross-Feature Direct Import

Id

arch-cross-feature-import

Severity

warning

Type

regex

Pattern

• features/\w+/.import.from.*features/(?!\w+/shared)

Message

Direct import across features. Use shared module or events.

Fix Action

Extract shared code to features/shared/ or use event-based communication

Applies To

• /features//*.ts
• /features//*.tsx

Test Coupled to Implementation

Id

arch-test-implementation-coupling

Severity

warning

Type

regex

Pattern

• toHaveBeenCalledWith\([^)]{100,}
• mock\(['"][\w./]+['"]\)
• jest\.mock\(['"][\w./]+['"]\)

Message

Tests may be coupled to implementation details. Test behavior instead.

Fix Action

Test observable behavior and outputs, not internal method calls

Applies To

• */.test.ts
• */.test.tsx
• */.spec.ts
• */.spec.tsx

Testing Private Methods

Id

arch-private-method-test

Severity

warning

Type

regex

Pattern

• \['\w+'\]\s*\(
• \(\w+\s+as\s+any\)\.\w+

Message

Testing private methods. Test through public API instead.

Fix Action

Test behavior through public methods. If private method needs testing, extract to separate module.

Applies To

• */.test.ts
• */.test.tsx
• */.spec.ts
• */.spec.tsx

Mixed Concerns in Single File

Id

arch-mixed-concerns-file

Severity

info

Type

regex

Pattern

• (export\s+function|export\s+const).\n[\s\S]{0,500}(export\s+type|export\s+interface).\n[\s\S]{0,500}(export\s+class)

Message

File mixes functions, types, and classes. Consider separating concerns.

Fix Action

Separate into types.ts, utils.ts, and service classes

Applies To

• */.ts
• */.tsx

Large Barrel File

Id

arch-barrel-file-large

Severity

info

Type

regex

Pattern

• (export\s+\\s+from\s+['"][^'"]+['"];?\s\n){10,}

Message

Large barrel file (10+ re-exports). May cause bundle size issues.

Fix Action

Use direct imports or tree-shakeable exports

Applies To

• **/index.ts
• **/index.js