API Review Guidelines

This document captures the main architectural and implementation patterns currently used in apps/api.

It is intended for AI-assisted PR reviews. Treat these rules as "follow by default" guidance extracted from the codebase as it exists today.

If a PR intentionally changes one of these rules, the reviewer should ask for an explicit architectural reason and request that this file be updated in the same change.

Scope

• Primary source: apps/api/src
• Supporting source: packages/schemas/src
• This file reflects observed code patterns, not idealized architecture

How To Review PRs

• hard-fail: patterns that are strongly consistent and should be enforced unless there is an explicit architecture decision
• warn: patterns that are strongly preferred, but already have a few accepted exceptions in the codebase
• known-exception: current deviations that should not automatically fail a PR unless the PR makes them worse

Hard-Fail Rules

1. Keep AppModule as a pure orchestrator

• AppModule should only compose modules and route prefixes
• Do not add business logic, feature-specific providers, or persistence setup directly in AppModule

2. Keep feature modules self-contained

• Each feature module should keep the standard structure: application, domain, infrastructure, presentation
• Each feature module owns its own persistence setup, entities, repositories, and schema-specific TypeORM configuration
• AppModule must not own feature-module entity lists or data sources

3. Only shared and core are cross-cutting modules

• shared contains reusable infrastructure and application-level shared services
• core contains global guards, interceptors, decorators, and request-level cross-cutting behavior
• Feature modules must not become new cross-cutting dependency hubs

4. Cross-feature communication must go through shared integration events

• Do not directly couple feature modules through repositories, services, or internal implementation classes
• Shared contracts for cross-module communication belong in apps/api/src/shared/integration-events
• Publishing and consuming events is the preferred boundary between independent modules
• See Cross-Module Integration Events for the full contract, directory layout, publisher/consumer conventions, and current event inventory

5. Shared contracts use interfaces with I prefix plus Symbol-based DI tokens

• Interfaces should use the I... naming convention
• Repositories, ports, and shared services should expose a ...Key = Symbol('...')
• Depend on interfaces/tokens in handlers and services instead of concrete infrastructure implementations when a boundary exists

6. Register providers through module-level arrays

• Provider collections are usually grouped in index.ts arrays such as controllers, handlers, repositories, adapters, entities, or shared services
• Modules should register those arrays rather than accumulating long ad hoc provider lists inline

7. Request DTOs and schemas belong in packages/schemas

• Endpoint request and response DTO types should come from packages/schemas
• Do not create duplicate API-local DTO classes when the schema package already defines the contract
• Shared schemas are the main source of truth for API input/output shapes

8. Validate request bodies with Zod at the endpoint boundary

• Controller body payloads should be validated with @Body(new ZodValidationPipe(schema))
• Do not rely on local class-validator DTO wrappers for request validation

9. Use Zod at other runtime boundaries when parsing external or framework-provided data

• Validate JWT payloads, auth context, env config, and similar untrusted runtime inputs with Zod before using them

10. Database naming follows SQL snake_case, TypeScript follows camelCase

• Table names are plural snake_case
• Column names are snake_case
• TypeScript properties remain camelCase
• When the database name differs from the TypeScript property name, set the explicit TypeORM name mapping

11. Repositories are the persistence boundary

• Repository interfaces live in the domain layer
• Repository implementations live in infrastructure
• Repositories map between persistence models and domain entities explicitly
• Repositories should be the place where persistence concerns such as TypeORM access and repository metrics are handled
• When repository telemetry exists for the module, wrap operations with the shared Postgres metrics service instead of bypassing instrumentation

12. Auth and throttling rules should be declared with decorators and enforced globally

• Prefer @PublicRoute(), @Auth(), @AuthUser(), @ClientAuth(), and @RateLimit(...)
• Do not reimplement authorization or rate-limit decisions ad hoc inside controller methods

13. Commands that perform multiple related DB writes must use a transaction

• If a command writes to the database more than once and those writes are logically related (i.e., one cannot succeed without the other), they must be wrapped in a single database transaction
• This ensures atomicity: either all writes commit together or all are rolled back on failure, preventing partial state corruption
• Use the TypeORM DataSource.transaction(...) helper or a QueryRunner managed by the repository/handler to wrap these operations
• Commands with a single write, or independent writes that can safely diverge, do not need a transaction

Warn Rules

14. Prefer thin controllers

• Controllers should mainly validate input, gather request context, dispatch commands/queries, unwrap results, and return DTOs
• Business logic should usually stay in command/query handlers and domain methods

15. Prefer CQRS at the presentation boundary

• Controllers should generally call CommandBus and QueryBus
• Command/query handlers should own use-case orchestration

16. Prefer mapping at the endpoint boundary

• When a handler returns domain entities, prefer mapping to DTOs in the controller or other presentation-layer boundary
• It is acceptable to map inside a command/query when the response is complex and doing it there is simpler or already established
• The presentation layer should decide whether the response is public or private and pass mapper options such as publicVersion explicitly
• Keep toDto mappings explicit so public responses cannot leak private fields by accident

17. All toDto mappers should accept options as the last parameter

• The standard pattern is options: IMapperOptions = DefaultMapperOptions
• toDto should validate output with the matching Zod schema by default
• validate: false should remain an explicit opt-out, mainly for internal pipelines or tests

18. Reuse the shared base entity and base persistence patterns

• Domain entities should typically extend BaseEntity
• Persistence entities should typically extend BasePersistenceEntity
• Use Entity.new(...) for creation and Entity.rehydrate(...) when rebuilding from persistence
• Do not redefine inherited base persistence fields such as id, createdAt, or updatedAt unless there is a very strong architectural reason
• Do not overload the base id column with a business identifier; keep business keys as dedicated fields and mark them unique when the relationship is effectively one-to-one

19. Prefer alias imports for cross-boundary API imports

• Use @api/* aliases when importing across module or layer boundaries
• Keep short relative imports for nearby files inside the same local subtree
• Avoid very deep relative import chains when an alias expresses intent better

20. Tests should mirror the app structure and stay focused

• Tests are usually under apps/api/test
• Test files are typically named after the class or behavior under test
• Most current tests are focused unit tests with explicit mocks for ports, repositories, mappers, and collaborators

Known Exceptions

These exist in the current codebase already. Do not fail a PR only for matching one of these patterns unless the PR expands the inconsistency without a good reason.

1. Some commands build response DTOs internally

• Complex auth flows already assemble response DTOs inside command handlers
• Example pattern: auth commands calling SessionMapper.toDto(...) directly inside the handler

2. Not every response currently uses a shared top-level output schema

• Several controllers still return inline { message: string } responses
• Treat this as technical debt, not an immediate hard failure

3. Not every query/command returns Result<T>

• Many handlers use Result<T>, but some queries return primitives directly
• Reviewers should prefer consistency, but not assume Result<T> is universal yet

4. Query-param validation for list endpoints uses the shared query DSL

• All new GET list endpoints with filters / sort / pagination MUST use QueryFilterPipe + listQuerySchemaFactory. See Query Filters & Pagination DSL for the canonical step-by-step guide.
• OAuth callback and redirect parameters that use plain Nest parameter extraction are a known legacy exception; do not mark them as a new violation on unrelated PRs, but do not expand that pattern.

5. Layering is pragmatic, not perfectly pure

• Some application handlers import mappers or publishers directly
• Some presentation-side consumers import infrastructure-level constants
• Review for unnecessary coupling, but do not enforce a rigid "clean architecture" rule that the current codebase does not follow

6. index.ts usage is mixed

• Many index.ts files are used correctly to group providers, handlers, controllers, entities, or adapters for module registration
• Some index.ts files also act as re-export barrels
• For reviews, prefer the module-registration pattern first and avoid expanding barrel usage without a clear reason

7. UUID schema style is not fully uniform yet

• Most newer schemas prefer explicit UUID version validation such as z.uuidv7()
• Some existing schemas still use more generic UUID validation
• Prefer the explicit versioned style in new code

Common Review Findings

These findings come from repeated review comments on real API PRs. Use them as concrete checks when the higher-level rules above feel too abstract.

1. Controller responses must finish with an explicit HTTP contract

• Review signal: a controller dispatches a command, declares Promise<void>, and does not return a DTO, success object, or explicit no-content contract
• Preferred fix: return the final DTO, a shared success payload, or an explicit empty object when that is the established contract for the endpoint
• Preferred fix: keep response mapping in the controller so the HTTP layer, not the handler, decides the final shape

2. Public and private API views must be mapped separately

• Review signal: the same endpoint or DTO shape is used for both admin/internal consumers and public consumers even though the resource has hidden fields or publication state
• Preferred fix: expose separate public/private controller methods or controllers when the audience differs
• Preferred fix: use IMapperOptions.publicVersion at the presentation boundary and keep the public DTO limited to user-visible fields only
• Preferred fix: when a resource has publish state, the public endpoint should return only published data and should not expose internal metadata such as draft status or internal version fields unless the product explicitly needs them

3. Internal mapping can be broad, response mapping must be explicit

• Review signal: toDomain and toPersistence manually copy long property lists that already exist on the source object
• Preferred fix: prefer ...source, mapProperties(...), or another shared helper for internal mapping paths so new fields are less likely to be forgotten
• Review signal: toDto spreads the full entity or persistence object into the response DTO
• Preferred fix: keep toDto field-by-field and whitelist only the properties that are allowed to leave the boundary

4. Persistence models should follow the shared base pattern literally

• Review signal: a new persistence entity redefines id, createdAt, or updatedAt, or uses the base id column to store a business foreign key
• Preferred fix: extend BasePersistenceEntity, inherit the base fields untouched, and model business identifiers as their own columns
• Preferred fix: when the business key is effectively one-to-one, use a dedicated column such as orgId with unique: true instead of replacing the primary identifier semantics

5. Repository methods should return the updated aggregate when the use case needs it

• Review signal: a save/update repository method returns void even though the caller needs the persisted aggregate for the response or later orchestration
• Preferred fix: return the saved domain entity from the repository and let the handler/controller continue from that updated state
• Preferred fix: keep repository metrics inside the repository by wrapping the operation in recordRepositoryOperation(...)

6. CQRS files should stay compact unless reuse justifies the split

• Review signal: a command/query class is split into its own file even though it is only used by a single handler and a single controller flow
• Preferred fix: keep the command/query definition in the same file as its handler when there is no reuse pressure
• Preferred fix: only split the definition into a separate file when multiple handlers, modules, or transports need the same message contract

7. Error contracts must match the actual aggregate being loaded

• Review signal: a handler returns a copied ...NotFound() error from another aggregate such as userNotFound() for an organization legal document query
• Preferred fix: create or reuse an aggregate-specific error factory and return the code/message that matches the actual missing resource

8. Shared schemas should encode real API constraints, not placeholders

• Review signal: a persisted identifier is validated with z.string() or a generic UUID validator even though the API uses UUID v7 everywhere else
• Preferred fix: validate persisted identifiers with z.uuidv7() in new shared schemas
• Review signal: a default schema value introduces a domain state that the current feature does not actually support yet
• Preferred fix: align defaults with the current product behavior, for example defaulting to published when drafts/version history are not implemented yet

Practical PR Heuristics

Fail the PR when you see any of these

• New direct dependency from one feature module into another feature module's private internals
• New DTO duplication inside apps/api when the same contract should live in packages/schemas
• New endpoint body without Zod validation
• New repository logic placed directly in controllers or command/query handlers
• New SQL table or column names introduced in camelCase
• New shared contract without I... interface naming when it is a repository, port, or shared service boundary
• New command that performs multiple related DB writes without wrapping them in a transaction
• New handler returning an unrelated error contract for the wrong aggregate or resource
• New public-facing response leaking draft/internal-only fields when a public/private split is required by the domain
• New persistence entity redefining id, createdAt, or updatedAt instead of extending the shared base model cleanly

Warn on these and ask for alignment

• Controller methods that start accumulating business logic
• Command/query handlers returning raw plain objects without a clear contract type
• New toDto mappers without IMapperOptions
• New cross-boundary imports that should probably use @api/* aliases
• New inline response objects where a shared schema likely belongs in packages/schemas
• Mutation endpoints with no explicit response contract from the controller layer
• Simple one-off command/query definitions split into extra files without a reuse reason
• Manual internal mapping code where mapProperties(...) or a safe spread-based pattern would better preserve new fields
• New shared schema IDs that skip explicit UUID v7 validation
• New schema defaults that describe unsupported business states

Reviewer Default Stance

• Prefer consistency with the existing codebase over theoretical purity
• Enforce the hard rules strictly
• Treat warn rules as preferred direction
• Do not flag known exceptions unless the PR introduces a better opportunity to standardize them