5 Code Review Mistakes That Let Architecture Debt Slip Through

Code reviews are the last line of defense before changes hit your main branch. They catch bugs, enforce style, and spread knowledge across the team. But most code reviews have a blind spot the size of a building: they focus on the trees and miss the forest.

Developers review individual lines of code with precision. They spot typos, suggest better variable names, flag missing error handling. All valuable. But while the team obsesses over whether a function should use map or reduce, structural problems walk right through the gate.

Architecture debt does not arrive in a single dramatic PR. It accumulates across hundreds of small, seemingly harmless changes. An import here, a shortcut there. Each one passes code review because each one looks fine in isolation. It is only months later, when the codebase becomes a tangled mess, that the team realizes the reviews were checking the wrong things.

Here are the five most common review mistakes that let architecture debt slip through — and what to do instead.

Mistake 1: Only Reviewing Line-Level Code, Not Module-Level Impact

This is the most widespread problem. A reviewer opens the diff, reads the changed lines, checks the logic, and approves. The review is thorough at the micro level but completely blind at the macro level.

The Problem

Consider a PR that adds a new utility function to a shared module:

// src/utils/format.ts
import { getUserLocale } from '../services/user-service';

export function formatCurrency(amount: number): string {
  const locale = getUserLocale();
  return new Intl.NumberFormat(locale, {
    style: 'currency',
    currency: 'USD',
  }).format(amount);
}

A line-level review might approve this without a second thought. The code is clean, it handles localization, it uses Intl.NumberFormat. Everything looks correct.

But zoom out. A utility module (utils/format.ts) now depends on a service module (services/user-service.ts). Utility modules should be pure — they take inputs and return outputs. They should not reach into service layers to fetch data. This single import creates a dependency that violates the architecture, and it will be copied by every developer who sees it as a pattern to follow.

The Fix

Before approving any PR, mentally step back and ask: "What module-level dependency does this change create?" If a file in utils/ is importing from services/, or a file in components/ is importing directly from database/, that is a red flag regardless of how clean the individual lines look.

Some teams formalize this by requiring a brief architecture impact note in PRs that cross module boundaries. Even a single sentence — "This change adds a dependency from the utils layer to the services layer" — forces the author and reviewer to acknowledge the structural implication.

Mistake 2: Ignoring Dependency Direction

Every well-designed application has a dependency direction. In a typical layered architecture:

Controllers → Services → Repositories → Database
     ↓            ↓           ↓
   DTOs        Models      Schemas

Dependencies flow downward and inward. Higher-level modules depend on lower-level modules. A controller can import from a service, but a service should never import from a controller.

The Problem

Here is a subtle violation that passes most reviews:

// src/services/notification-service.ts
import { OrderController } from '../controllers/order-controller';

export class NotificationService {
  async sendOrderConfirmation(orderId: string) {
    // Reusing the controller's formatting logic
    const controller = new OrderController();
    const formattedOrder = controller.formatOrderForDisplay(orderId);
    await this.send(formattedOrder);
  }
}

The developer needed the formatting logic that happened to live in the controller. Instead of extracting it into a shared module, they imported the controller directly into the service. The code works. The tests pass. The review focuses on the notification logic and misses the upward dependency.

Now the service layer depends on the controller layer. This creates a cycle: controllers depend on services (normal), and now a service depends on a controller (violation). Build order becomes unpredictable. Testing the service in isolation requires instantiating a controller. Refactoring the controller might break the notification service.

The Fix

Reviewers should check the import section of every changed file — not just the code below it. When you see an import, ask: "Is this dependency direction correct?"

A quick mental model:

• controllers/ can import from services/, utils/, types/
• services/ can import from repositories/, utils/, types/
• repositories/ can import from database/, utils/, types/
• utils/ should import only from other utils/ or external packages

If the import goes "upward" in this hierarchy, it needs to be called out. The fix is usually to extract the shared logic into a lower-level module that both layers can depend on.

Mistake 3: Missing New Circular Dependencies

Circular dependencies are one of the most destructive forms of architecture debt. They make code untestable, break build tools, cause runtime errors, and resist refactoring. They are also surprisingly easy to introduce and surprisingly hard to spot in a review.

The Problem

A circular dependency rarely appears as a direct A-imports-B and B-imports-A in a single PR. More often, it completes a chain that was already partially there:

Before the PR:
  auth-service.ts → user-service.ts → email-service.ts

The PR adds:
  email-service.ts → auth-service.ts  (to check permissions before sending)

Now you have a three-node cycle: auth → user → email → auth. The reviewer sees only the last link being added. The PR diff shows email-service.ts importing from auth-service.ts, which looks perfectly reasonable in isolation. Nobody realizes it completes a cycle.

Here is what the problematic change might look like:

// src/services/email-service.ts
import { checkPermission } from './auth-service';

export async function sendEmail(userId: string, template: string) {
  const canSend = await checkPermission(userId, 'email:send');
  if (!canSend) throw new Error('Unauthorized');
  // ... send email
}

Nothing in this diff screams "danger." The import looks legitimate. The permission check is good practice. But the circular dependency it creates will eventually cause problems.

The Fix

Catching circular dependencies manually requires knowing the full dependency graph of the project — which is unrealistic for any codebase of meaningful size.

The pragmatic solution is automation. Linting tools like eslint-plugin-import with the no-cycle rule can detect circular dependencies at build time. Architecture analysis tools like ReposLens can detect them visually on every PR, showing the reviewer the full dependency path rather than just the new import.

If your team does not have automated cycle detection, at minimum establish this review habit: when you see a new import between two service-level files, trace the chain manually. Does Service A already depend on this service through other files? If you are not sure, that is a signal that you need better tooling.

Mistake 4: Not Checking If the Change Increases Coupling

Coupling is a measure of how much one module knows about another. Some coupling is necessary — modules need to communicate. But unnecessary coupling makes the system rigid and fragile.

The Problem

A common pattern that increases coupling without anyone noticing:

// PR: "Add order total to dashboard"

// src/components/Dashboard.tsx
import { calculateOrderTotal } from '../../services/order-service';
import { getShippingCost } from '../../services/shipping-service';
import { getTaxRate } from '../../services/tax-service';
import { getDiscount } from '../../services/discount-service';
import { formatCurrency } from '../../utils/format';

export function DashboardOrderSummary({ orderId }: { orderId: string }) {
  const total = calculateOrderTotal(orderId);
  const shipping = getShippingCost(orderId);
  const tax = getTaxRate(orderId);
  const discount = getDiscount(orderId);

  return (
    <div>
      <p>Total: {formatCurrency(total + shipping + tax - discount)}</p>
    </div>
  );
}

The reviewer sees a clean component that displays an order summary. The code is readable, each function is well-named, and the calculation looks correct.

But the Dashboard component now depends on four separate services. It knows about orders, shipping, tax, and discounts. Any change to any of these services might break the dashboard. The component cannot be tested without mocking four services. And the business logic (how to calculate a final total) is embedded in a UI component instead of living in the service layer.

The Fix

Count the imports. Seriously. When a file's import section is growing, that is a coupling red flag. A practical rule of thumb:

• 1-3 imports from your own codebase: normal
• 4-6 imports: worth questioning — can some be consolidated?
• 7+ imports: almost certainly a design problem

In the example above, the fix is a single import:

import { getOrderSummary } from '../../services/order-service';

export function DashboardOrderSummary({ orderId }: { orderId: string }) {
  const summary = getOrderSummary(orderId);
  return <div><p>Total: {summary.formattedTotal}</p></div>;
}

The getOrderSummary function encapsulates the business logic. The component depends on one service, not four. Coupling is reduced dramatically.

During reviews, when you see a file importing from multiple services or modules, ask: "Should there be an intermediary that aggregates this?"

Mistake 5: Reviewing in Isolation Without Seeing the Bigger Picture

The final and most fundamental mistake is treating each PR as a self-contained unit. PRs are not self-contained. They are changes to a living system, and their impact depends on context that the diff alone cannot provide.

The Problem

Over six months, a team makes these individually reasonable changes:

• PR #201: Add helper function to shared/utils.ts (approved)
• PR #215: Import shared utils in the payments module (approved)
• PR #228: Add payment types to shared/types.ts (approved)
• PR #241: Import shared types in the auth module (approved)
• PR #256: Add auth helpers to shared/utils.ts (approved)
• PR #270: Import shared utils in the notification module (approved)

Each PR is small, clean, and correct. But after six months, shared/ has become a dumping ground for unrelated functionality. Every module depends on it. It cannot be modified without risking breakage across the entire application. It has become a "god module" — and no single review created it.

The Fix

This is the hardest problem to solve with manual reviews alone because it requires seeing patterns across multiple PRs over time. Some strategies:

Periodic architecture reviews. Once a month or once a quarter, the team reviews the full dependency graph of the project. This catches the gradual accumulation of debt that individual PR reviews miss.

Module ownership. Assign modules to specific people or teams. When a PR modifies a module boundary — adding a dependency to or from the module — the owner is added as a required reviewer.

Automated architecture checks. Tools that analyze the dependency graph on every PR can flag when a module exceeds a coupling threshold, when new cross-module dependencies are added, or when the overall architecture complexity increases. ReposLens provides this kind of analysis automatically, showing reviewers not just what changed in the code, but how the change affects the system structure.

Architecture Decision Records (ADRs). Document the intended architecture boundaries. When a PR violates those boundaries, the ADR provides an objective basis for pushing back, instead of relying on individual reviewers' memory of what the architecture should look like.

Building a Better Review Culture

Fixing these five mistakes does not require a complete overhaul of your review process. It requires adding one question to every review: "How does this change affect the architecture?"

In practice, this means:

1. Read the imports first. Before reading the code, read the imports. They tell you which modules this file touches and whether those dependencies make sense.

2. Zoom out before approving. After reviewing the code, take 10 seconds to think about the module-level impact. Does this change add a dependency that did not exist before? Is that dependency appropriate?

3. Automate what humans are bad at. Humans are excellent at judging code quality and logic. They are terrible at tracking dependency graphs across hundreds of files. Use tools for the latter.

4. Track architecture metrics over time. Number of circular dependencies, average coupling per module, count of cross-boundary imports. These metrics make architecture debt visible before it becomes a crisis.

5. Make architecture a review criterion. Add it to your review checklist alongside correctness, tests, and style. If your team uses labels or categories for review feedback, add "architecture" as an option.

The cost of catching an architecture issue in code review is a 5-minute conversation. The cost of fixing it six months later is a multi-sprint refactoring effort. The math is straightforward.