# REVIEW.md
## Out of scope
Don't raise findings here: `scripts/` (local tooling), `**/*_generated.*`,
`tests/fixtures/`.
## Layering & dependency direction
Layers are `domain → application → infrastructure`; dependencies point inward only.
- `domain/` never imports `infrastructure/` — no DB clients, SDKs, or framework
types in business logic. It needs one? That's an interface the outer layer implements.
- Framework/vendor types (request objects, ORM entities) stay at the edges; map
them at the boundary.
## Extension points ("add one of many")
Register-and-wire patterns (handlers, event types, providers). A missed wiring
spot degrades **silently**, not with an error.
- Mirror every hit: `grep -rn` an existing sibling and match each — registration,
routing, serialization, metrics, docs.
- The new case honors the same contract as its peers (config, error handling,
output shape).
## Abstractions & interfaces
Program to interfaces; branching on a concrete type rots the moment a new
implementation appears.
- Dispatch on capability, **never** on concrete type (`isinstance` / class-name
switches).
- Advertised ≠ implemented — if an implementation opts out of part of the
interface, callers check support and degrade first.
## Trust & data boundaries
Validate and authorize where *untrusted data enters*, before it reaches core logic.
- Every entry point (API, webhook, queue, imported file) validates shape/range
and checks authorization at the boundary.
- Authorization is about the *resource*, not just the session — an authenticated
caller isn't automatically entitled to this record.
- **Fail closed** — on any error or unknown case, deny.
## Cross-service / cross-repo contracts
Some changes are only correct if a matching change lands where the diff can't reach.
- Shared shapes (API fields, event payloads, schemas) change in lockstep across
every producer and consumer — name the counterpart repos.
- Roll out compatibly: add before you remove, tolerate both shapes during the
transition, then clean up. Shims have a removal plan.
## State, ownership & lifecycle
Invariants that span methods and files, so any single hunk looks fine.
- **Single source of truth** — derived state isn't cached somewhere it can drift.
- Resources (connections, locks, handles) are released on every path, including
errors.
- Idempotency where delivery isn't exactly-once — retries and replays must not
double-apply.
## Reuse before reinvention
Duplication passes review because the original is off-screen.
- Search shared utilities before adding new parsing/formatting/HTTP logic.
- One config key per concept — don't reintroduce a knob under a new name.
- Extend the existing helper instead of forking a near-copy.
