Review Architecture

Use this skill when the task is to review or improve the structure of a Rails application or library.

Core principle: Prioritize boundary problems over style. Prefer simple objects and explicit flow over hidden behavior.

Quick Reference

Review Order

1. Identify the main entry points: controllers, jobs, models, services.
2. Check where domain logic lives.
3. Inspect model responsibilities, callbacks, and associations.
4. Inspect controller size and orchestration.
5. Read every concern, helper, and presenter: does it do one coherent thing, or does it mix auditing + notifications + emails + external API calls? Mixed concerns are High or Medium severity depending on blast radius. Treat any concern used by only one class as a candidate for deletion — inline it instead.
6. Check whether abstractions clarify the design or only move code around.
7. Verify each High-severity finding by reading the actual code — confirm it is a real structural problem, not just a pattern match on file size or line count. If verification reveals the finding is not a genuine structural problem, either downgrade it to Medium/Low with a revised rationale, or remove it entirely. Do not list findings that do not survive code-level confirmation.

Severity Levels

High-Severity Findings

• Business logic hidden in callbacks or broad concerns
• Controllers orchestrating multi-step domain workflows inline
• Models coupled directly to HTTP, jobs, mailers, or external APIs
• Abstractions that add indirection without a clear responsibility
• Cross-layer constant reach that makes code hard to change

Medium-Severity Findings

• Duplicated workflow logic across controllers or jobs
• Scopes or class methods carrying too much query or policy logic
• Helpers or presenters leaking domain behavior
• Service objects wrapping trivial one-liners
• Concerns combining unrelated responsibilities — check EVERY concern in the app

Output Format and Style

Begin with entry points. Open the review by identifying the application's entry points (controllers, jobs, public API surface) before listing findings. Then write findings ordered by review area — boundary problems first, then model/callback issues, then concerns/helpers.

Every finding uses this four-field structure:

**Severity:** High
**Affected file:** app/controllers/orders_controller.rb — OrdersController#create
**Risk:** Controller runs a 5-step domain workflow. Partial state on failure; untestable without HTTP.
**Improvement:** Extract to Orders::CreateOrder.call(params). Controller handles response/redirect only.

For each finding include: severity, affected files or area, why the structure is risky, and the smallest credible improvement. Then list open assumptions and recommended next refactor steps.

High-severity callback example:

# Bad — hidden side effects on every save
module Auditable
  included do
    after_create :log_creation
  end
  def log_creation
    AuditLog.create!(...)
    Slack.notify(...)                          # external API in callback
    UserMailer.admin_alert(...).deliver_later  # mailer in callback
  end
end

Fix: keep only AuditLog.create! in the callback; move Slack/mailer to an explicit service call at the call site.

Pitfalls