Review Architecture

Architecture Review Skill (DDD + Clean Architecture)

Specialized architectural review for modular monoliths using Domain-Driven Design (DDD), Clean/Onion Architecture, and bITdevKit patterns. This skill verifies layer boundaries, domain purity, CQRS patterns, and proper use of Result error handling.

When to Apply This Skill

Use this skill when:

• Conducting architecture reviews of new features or modules
• Reviewing pull requests that add/modify domain models, commands, queries, or endpoints
• Refactoring existing code to align with DDD/Clean Architecture
• Onboarding new developers to the architectural patterns
• Validating that layer boundaries remain intact after changes
• Auditing cross-module dependencies in a modular monolith

Architecture Overview

Layer Structure (Onion/Clean Architecture)

Dependencies flow inward only from outer layers to inner layers:

┌─────────────────────────────────────────┐
│      Presentation (Outermost)           │  Minimal API endpoints, DTOs
│  ┌───────────────────────────────────┐  │
│  │      Infrastructure               │  │  EF Core, Repositories, Jobs
│  │  ┌─────────────────────────────┐  │  │
│  │  │     Application             │  │  │  Commands, Queries, Handlers
│  │  │  ┌───────────────────────┐  │  │  │
│  │  │  │   Domain (Innermost)  │  │  │  │  Aggregates, Entities, Value Objects
│  │  │  │   Pure Business Logic │  │  │  │  Domain Events, Enumerations
│  │  │  │   ZERO Dependencies   │  │  │  │
│  │  │  └───────────────────────┘  │  │  │
│  │  └─────────────────────────────┘  │  │
│  └───────────────────────────────────┘  │
└─────────────────────────────────────────┘

Module Organization

Each module under src/Modules/<ModuleName> follows this structure:

CoreModule/
├── CoreModule.Domain/         # Pure business logic (NO external dependencies)
│   └── Model/
│       ├── CustomerAggregate/
│       │   ├── Customer.cs           (Aggregate Root)
│       │   └── Events/
│       ├── EmailAddress.cs           (Value Object)
│       └── CustomerStatus.cs         (Enumeration)
├── CoreModule.Application/    # Use case orchestration (references Domain only)
│   ├── Commands/
│   │   ├── CustomerCreateCommand.cs
│   │   └── CustomerCreateCommandHandler.cs
│   └── Queries/
├── CoreModule.Infrastructure/ # Technical implementation (references Domain + Application)
│   ├── EntityFramework/
│   │   ├── CoreModuleDbContext.cs
│   │   └── Configurations/
│   └── Repositories/
└── CoreModule.Presentation/   # Endpoints & DTOs (references Application via IRequester)
    └── Web/
        └── Endpoints/
            └── CustomerEndpoints.cs

Review Priorities

🔴 CRITICAL Issues (Must Fix Before Merge)

These violations break architectural boundaries or introduce serious design flaws:

• Layer boundary violations: Dependency flows outward (e.g., Domain → Application, Application → Infrastructure)
• Cross-module direct references: Modules directly referencing each other (breaks modular isolation)
• Domain impurity: Domain layer has external dependencies (EF Core, Application, Infrastructure)
• DbContext in Application/Domain: Direct DbContext usage outside Infrastructure layer
• Public setters on aggregates/entities: Breaks encapsulation; use change methods instead
• Exceptions for business rules: Using exceptions instead of Result for expected failures
• Test independence violations: Tests sharing mutable state or depending on execution order
• Resource leaks: IDisposable not properly disposed (missing using statements)
• Endpoints with business logic: Business rules implemented in Presentation layer

🟡 IMPORTANT Issues (Should Fix Soon)

These issues affect maintainability, consistency, or future extensibility:

• CQRS naming violations: Commands/queries not following [Entity][Action]Command/Query pattern
• Repository pattern violations: Not using IGenericRepository in Application layer
• Strongly-typed ID inconsistency: Missing [TypedEntityId<Guid>] attributes
• Specification pattern ignored: Complex queries inline instead of using specifications
• IRequester not used: Endpoints directly instantiating handlers instead of using IRequester.SendAsync
• N+1 query problems: Missing .Include() causing excessive database round-trips
• Async/await violations: Blocking on async code (.Result, .Wait())
• CancellationToken missing: Async methods not accepting CancellationToken parameters
• Domain event naming: Not in past tense or missing "DomainEvent" suffix

🟢 SUGGESTIONS (Nice to Have)

These improve code quality but are not blocking:

• XML documentation: Missing on public APIs
• Mapster configuration: Ad-hoc mapping instead of MapperRegister
• OpenAPI metadata: Endpoints missing .WithName, .WithSummary, .Produces
• Modern C# features: Opportunities to use pattern matching, expression-bodied members, etc.
• Value object naming: Generic names (Email) instead of descriptive (EmailAddress)

Quick Reference to Detailed Guidance

This skill includes comprehensive checklists, examples, and templates:

Checklists (6 files)

1. Layer Boundaries: Verify dependency flow, detect circular references
2. Domain Patterns: Validate aggregates, entities, value objects, domain events
3. CQRS Patterns: Check commands, queries, handlers, validators, IRequester usage
4. Repository & Data Access: Ensure proper abstraction, specification pattern, N+1 detection
5. Presentation Endpoints: Validate thin adapters, IRequester delegation, OpenAPI docs
6. Result & Error Handling: Enforce Result pattern, HTTP mapping, error clarity

Examples (5 files)

1. Aggregate Patterns: WRONG vs CORRECT aggregate implementations
2. Value Object Patterns: WRONG vs CORRECT value object implementations
3. CQRS Examples: Command/query structure, handler delegation, IRequester usage
4. Result Pattern Examples: Using Result instead of exceptions
5. Layer Violations: Common boundary violations and how to fix them

Documentation (2 files)

1. ADR Quick Reference: One-paragraph summaries of all 20 ADRs with quick lookup
2. bITdevKit Patterns: IRequester/INotifier, repository behaviors, module registration

Templates (2 files)

1. Architecture Review Template: Comment format for violations
2. Review Summary Template: Summary format with ADR references

Example Workflow: Conducting an Architecture Review

Step 1: Identify Scope

Determine which modules and layers are affected:

# Check which files changed in a PR
git diff main...feature-branch --name-only | grep "src/Modules/"

# Example output:
# src/Modules/CoreModule/CoreModule.Domain/Model/CustomerAggregate/Customer.cs
# src/Modules/CoreModule/CoreModule.Application/Commands/CustomerCreateCommand.cs
# src/Modules/CoreModule/CoreModule.Presentation/Web/Endpoints/CustomerEndpoints.cs

Layers affected: Domain, Application, Presentation

Step 2: Check Layer Boundaries (🔴 CRITICAL)

Use: checklists/01-layer-boundaries.md

Verify dependencies flow inward only:

• ✅ Domain: No using statements referencing Application, Infrastructure, or Presentation
• ✅ Application: Only using statements referencing Domain
• ✅ Presentation: Uses IRequester.SendAsync() to call Application layer

Reference: ADR-0001 (Clean/Onion Architecture)

Step 3: Validate Domain Patterns (🔴 CRITICAL)

Use: checklists/02-domain-patterns.md

Check aggregate Customer.cs:

• ✅ Private setters on all properties
• ✅ Factory method Customer.Create() returns Result<Customer>
• ✅ Change methods (ChangeName, ChangeEmail) return Result<Customer>
• ✅ Domain events registered (CustomerCreatedDomainEvent)
• ✅ [TypedEntityId<Guid>] attribute on CustomerId
• ✅ Collection exposed as IReadOnlyCollection<Address>

Reference: ADR-0012 (Domain Logic in Domain Layer), ADR-0008 (Typed Entity IDs)

Step 4: Review CQRS Implementation (🟡 IMPORTANT)

Use: checklists/03-cqrs-patterns.md

Check CustomerCreateCommand.cs:

• ✅ Named CustomerCreateCommand (follows [Entity][Action]Command pattern)
• ✅ Nested Validator class using AbstractValidator<T>
• ✅ Handler uses IGenericRepository<Customer>, NOT DbContext
• ✅ Handler delegates to Customer.Create() (domain), not business logic in handler
• ✅ Returns Result<CustomerId>

Reference: ADR-0011 (Application Logic in Commands/Queries), ADR-0009 (FluentValidation)

Step 5: Validate Repository Usage (🔴 CRITICAL)

Use: checklists/04-repository-data-access.md

Check handler:

• ✅ Injects IGenericRepository<Customer> (abstraction)
• ❌ Injects CoreModuleDbContext directly → WRONG (ADR-0004 violation)

Fix: Replace DbContext with repository abstraction.

Reference: ADR-0004 (Repository Pattern with Decorator Behaviors)

Step 6: Review Endpoints (🟡 IMPORTANT)

Use: checklists/05-presentation-endpoints.md

Check CustomerEndpoints.cs:

• ✅ Derives from EndpointsBase
• ✅ Uses IRequester.SendAsync(command, ct) to delegate to Application
• ✅ Uses .MapHttpCreated() to map Result<CustomerId> to HTTP 201
• ✅ Includes CancellationToken ct parameter
• ⚠️ Missing .WithName("CreateCustomer") → SUGGESTION (🟢)

Reference: ADR-0014 (Minimal API Endpoints), ADR-0005 (Requester/Notifier)

Step 7: Check Result Error Handling (🔴 CRITICAL)

Use: checklists/06-result-error-handling.md

Check error handling:

• ✅ Customer.Create() returns Result<Customer>
• ✅ Validation failures return Result.Failure("error message")
• ❌ throw new ValidationException() in domain method → WRONG (ADR-0002 violation)

Fix: Replace exception with Result<T>.

Reference: ADR-0002 (Result Pattern for Error Handling)

Step 8: Generate Review Summary

Use: templates/review-summary-template.md

Create summary with:

• Issues Found: 🔴 2, 🟡 1, 🟢 1
• Top 3 Priorities: DbContext in Application (ADR-0004), Exception for business rule (ADR-0002), Missing endpoint name (🟢)
• Architecture Compliance: ⚠️ (critical issues present)
• ADRs Referenced: ADR-0001, ADR-0002, ADR-0004, ADR-0005, ADR-0008, ADR-0009, ADR-0011, ADR-0012, ADR-0014

Integration with Architectural Decision Records (ADRs)

This skill references 20 Architectural Decision Records (ADRs) located in docs/ADR/. Each ADR documents a key architectural decision with context, rationale, and consequences.

How to Use ADRs in Reviews

1. Identify the pattern: Determine which architectural pattern is involved (e.g., layer boundaries, CQRS, Result)
2. Find the ADR: Use docs/adr-quick-reference.md to locate the relevant ADR
3. Reference in feedback: Cite the ADR number and title in review comments (e.g., "ADR-0001: Clean/Onion Architecture")
4. Explain the impact: Use the ADR's rationale to explain why the violation matters

Key ADRs by Category

Core Architecture:

• ADR-0001: Clean/Onion Architecture with Strict Layer Boundaries
• ADR-0003: Modular Monolith Architecture

Domain & Data:

• ADR-0012: Domain Logic Encapsulation in Domain Layer
• ADR-0008: Typed Entity IDs using Source Generators
• ADR-0007: Entity Framework Core with Code-First Migrations

Application Layer:

• ADR-0011: Application Logic in Commands & Queries
• ADR-0009: FluentValidation Strategy
• ADR-0010: Mapster for Object Mapping

Error Handling:

• ADR-0002: Result Pattern for Error Handling

Presentation & API:

• ADR-0014: Minimal API Endpoints with DTO Exposure
• ADR-0005: Requester/Notifier (Mediator) Pattern

Infrastructure:

• ADR-0004: Repository Pattern with Decorator Behaviors
• ADR-0019: Specification Pattern for Repository Queries

See docs/adr-quick-reference.md for complete list with one-paragraph summaries.

Common Violation Patterns

🔴 CRITICAL: Domain → Application Dependency

Symptom: Domain layer references Application types (commands, queries, handlers)

// WRONG: Domain references Application
namespace MyApp.Domain.CustomerAggregate;

using MyApp.Application.Commands; // ❌ Domain → Application dependency

public class Customer : AggregateRoot<CustomerId>
{
    public CustomerCreatedCommand ToCommand() // ❌ Domain knows about Application
    {
        return new CustomerCreatedCommand(this.FirstName, this.LastName);
    }
}

Why Critical: Violates ADR-0001 (Clean/Onion Architecture). Domain must be pure business logic with ZERO external dependencies.

Fix: Remove Application reference. Application layer creates commands from domain entities, not vice versa.

Reference: examples/layer-violations.md

🔴 CRITICAL: Application → Infrastructure (DbContext)

Symptom: Application handlers inject DbContext directly

// WRONG: Application uses DbContext
namespace MyApp.Application.Commands;

using MyApp.Infrastructure.EntityFramework; // ❌ Application → Infrastructure dependency

public class CustomerCreateCommandHandler
{
    private readonly CoreModuleDbContext context; // ❌ Direct DbContext usage

    public async Task<Result<CustomerId>> Handle(CustomerCreateCommand request, CancellationToken ct)
    {
        var customer = Customer.Create(...);
        this.context.Customers.Add(customer); // ❌ Application knows about EF Core
        await this.context.SaveChangesAsync(ct);
    }
}

Why Critical: Violates ADR-0001 (layer boundaries) and ADR-0004 (repository pattern). Application layer cannot reference Infrastructure.

Fix: Use IGenericRepository<Customer> abstraction.

Reference: examples/layer-violations.md, checklists/04-repository-data-access.md

🔴 CRITICAL: Exceptions for Business Rules

Symptom: Domain methods throw exceptions for validation failures

// WRONG: Exception for business rule
public static Customer Create(string name, string email)
{
    if (string.IsNullOrWhiteSpace(name))
    {
        throw new ValidationException("Name is required"); // ❌ Exception for expected failure
    }

    return new Customer(name, email);
}

Why Critical: Violates ADR-0002 (Result Pattern). Exceptions should only be used for truly exceptional cases, not expected failures.

Fix: Return Result<Customer> instead.

Reference: examples/result-pattern-examples.md

🟡 IMPORTANT: CQRS Naming Violations

Symptom: Commands/queries not following naming conventions

// WRONG: Poor naming
public sealed record CreateCustomerRequest(...) : IRequest<Result<CustomerId>>; // ❌ "Request" suffix
public sealed record GetCustomer(...) : IRequest<Result<CustomerModel>>; // ❌ Missing "Query" suffix

Why Important: Violates ADR-0011 (CQRS patterns). Inconsistent naming makes codebase harder to navigate.

Fix: Use [Entity][Action]Command and [Entity][Action]Query patterns.

Reference: checklists/03-cqrs-patterns.md

🟡 IMPORTANT: N+1 Query Problem

Symptom: Missing .Include() causes multiple database round-trips

// WRONG: N+1 query problem
var customers = await repository.FindAllAsync(cancellationToken: ct);
foreach (var customer in customers)
{
    // Each iteration triggers a separate query for addresses!
    var addresses = customer.Addresses.ToList();
}

Why Important: Performance issue. Can cause significant slowdowns with large datasets.

Fix: Use eager loading with specifications.

Reference: checklists/04-repository-data-access.md

Success Criteria for Architecture Reviews

Code passes architectural review when:

• ✅ No layer boundary violations: Dependencies flow inward only
• ✅ Domain layer is pure: ZERO external dependencies (only bITdevKit domain abstractions)
• ✅ No circular module references: Modules are self-contained
• ✅ Repository abstractions used: Application uses IGenericRepository<T>, not DbContext
• ✅ Result pattern enforced: Domain methods return Result for failures, not exceptions
• ✅ Aggregates properly encapsulated: Private setters, change methods, factory methods
• ✅ CQRS naming consistent: Commands/queries follow [Entity][Action]Command/Query pattern
• ✅ Endpoints are thin adapters: No business logic in Presentation layer
• ✅ IRequester pattern used: Endpoints delegate to Application via IRequester.SendAsync()
• ✅ No N+1 query problems: Proper eager loading with .Include() or specifications
• ✅ CancellationToken propagated: All async methods accept and pass CancellationToken
• ✅ Test independence: No shared mutable state between tests

Tips for Effective Architecture Reviews

Do

• Start with layer boundaries: Verify dependencies flow inward before checking patterns
• Reference ADRs: Always cite the relevant ADR number and title in feedback
• Provide examples: Show WRONG vs CORRECT code, not just abstract explanations
• Explain impact: Don't just say "this violates ADR-0001", explain why it matters
• Use checklists systematically: Work through checklists sequentially to avoid missing issues
• Distinguish CRITICAL vs IMPORTANT: Use emoji indicators (🔴🟡🟢) to prioritize feedback
• Test architecture rules: Suggest NetArchTest rules to prevent future violations

Don't

• Mix code quality and architecture: Focus on architectural concerns; use review-code skill for code quality
• Overwhelm with minor issues: Prioritize critical and important issues over suggestions
• Accept "it works" as justification: Architecture violations accumulate technical debt
• Skip positive feedback: Acknowledge good patterns when you see them
• Review without ADR context: Always consult ADRs to understand the rationale behind patterns

Related Skills

• review-code: Use for code quality, security, testing, performance, and documentation reviews
• domain-add-aggregate: Use to scaffold new domain aggregates following DDD patterns
• adr-writer: Use to create new ADRs for architectural decisions

References

• AGENTS.md: Project-specific architecture patterns and conventions
• .github/copilot-instructions.md: Detailed coding guidelines
• docs/ADR/: All 20 Architectural Decision Records
• bITdevKit Documentation: Official bITdevKit patterns and features

Version: 1.0 Last Updated: 2026-01-14 Maintainer: bITdevKit Team