docs

plans/website-architecture-violations.md

@@ -28,6 +28,93 @@ Hard boundaries to enforce:
 - No locale APIs (`Intl.*`, `toLocale*`) in any formatting path (guardrails: [`VIEW_DATA.md`](docs/architecture/website/VIEW_DATA.md:41), [`WEBSITE_GUARDRAILS.md`](docs/architecture/website/WEBSITE_GUARDRAILS.md:13)).
 - All writes enter via Server Actions (contract: [`FORM_SUBMISSION.md`](docs/architecture/website/FORM_SUBMISSION.md:18)).
 
+### 0.1 Naming: stop calling everything “dto” (clarify the model taxonomy)
+
+This repo currently uses `dto` as a generic variable name, which collapses three different concepts.
+
+We will use the contract’s model names and enforce naming conventions to prevent category errors.
+
+Authoritative types:
+
+- **API Transport DTO**: returned by `apps/api` over HTTP. In this repo, often generated from OpenAPI.
+  - Canonical placement: [`apps/website/lib/types/**`](apps/website/lib/types/League.ts:1) per [`WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:41).
+  - Naming rule:
+    - Type names end with `ApiDto` (preferred) or `DTO` (existing generated types).
+    - Variable names: `apiDto`, `apiResponse`.
+
+- **Page DTO**: website-owned server-to-client payload assembled by PageQueries.
+  - Canonical placement: `apps/website/lib/page-queries/**` per [`WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:54).
+  - Naming rule:
+    - Type names end with `PageDto`.
+    - Variable names: `pageDto`.
+
+- **ViewData**: the only allowed input to Templates.
+  - Canonical placement: `apps/website/templates/**` per [`WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:86).
+  - Naming rule:
+    - Type names end with `ViewData`.
+    - Variable names: `viewData`.
+
+Other model names we should keep explicit:
+
+- **ViewModel**: client-only class instance.
+  - Naming rule: `*ViewModel` types/classes, variables `viewModel`.
+
+- **Command DTO** (write intent): the payload sent to Server Actions / API for mutations.
+  - Naming rule: `*CommandDto`, variables `commandDto`.
+
+Non-negotiable rule:
+
+- Never use a bare `dto` variable name. Use `apiDto`, `pageDto`, `viewData`, or `commandDto`.
+
+This is a Clean Architecture guardrail: when names are precise, wrong-layer dependencies become obvious during review.
+
+### 0.2 Enforce the taxonomy with required abstractions (not lint)
+
+You asked for “proper abstractions that must be implemented” (interfaces and/or abstract classes), rather than naming-only or ESLint enforcement.
+
+The goal is that the *compiler* (and the module boundaries) makes it hard to mix:
+
+- API Transport DTO vs Page DTO vs ViewData
+- server-only vs client-only
+- orchestration vs presentation mapping
+
+Proposed required abstractions (website-owned):
+
+1. **API Client contracts** (transport boundary)
+   - Interface per API area, returns **ApiDto/DTO only**.
+   - Example: `LeaguesApiClient` already exists; enforce that it returns `*DTO` types only.
+   - Prohibit returning ViewModels.
+
+2. **Service contracts** (orchestration boundary, server-safe)
+   - Interface per feature: `*Service` returns **ApiDto** or **PageDto** only.
+   - Must be stateless.
+   - Must not import `lib/view-models/**`.
+
+3. **PageQuery contract** (server composition boundary)
+   - A generic interface:
+     - `execute(params) -> PageQueryResult<PageDto>`
+   - PageQueries must only depend on API clients + services (manual wiring).
+   - Must not import `lib/view-models/**`.
+
+4. **Presenter contract** (pure mapping boundary, client-only)
+   - Presenter is a pure mapper:
+     - `PageDto -> ViewData` OR `PageDto -> ViewModel` OR `ViewModel -> ViewData`
+   - No HTTP, no side effects.
+
+5. **ViewModel base** (client-only)
+   - Optional but useful to enforce non-serializability:
+     - e.g. `abstract class ViewModel { abstract toViewData(): ViewData }`
+   - No `toDTO()` method that resembles server DTOs (it tempts server usage).
+
+6. **DisplayObject base** (deterministic formatting)
+   - Abstract base or interface to enforce “no locale APIs” policy by convention and review.
+   - Encourages `new MoneyDisplay(amount).formatted()` style, but only used from Presenters/ViewModels.
+
+Implementation note:
+
+- These abstractions live in `apps/website/lib/contracts/**` so every feature must conform.
+- This is complementary to guardrails: guardrails prevent forbidden imports; contracts ensure the correct “shape” of each layer.
+
 ---
 
 ## 1) Remediation plan (explicit actions)
@@ -58,6 +145,22 @@ Deliverable for this section:
 - A CI-failing rule that prevents future regressions.
 - A grep for `getContainer()` in server modules returns zero hits.
 
+Rationale: why DI is banned on the server side
+
+This is a deliberate safety simplification.
+
+1. Next.js server execution is concurrent; DI containers make it easy to accidentally share state across requests.
+   - The contract explicitly warns about this risk ([`WEBSITE_CONTRACT.md`](docs/architecture/website/WEBSITE_CONTRACT.md:227), [`WEBSITE_DI_RULES.md`](docs/architecture/website/WEBSITE_DI_RULES.md:9)).
+   - The singleton container is explicitly called out as unsafe ([`ContainerManager`](apps/website/lib/di/container.ts:61), [`WEBSITE_DI_RULES.md`](docs/architecture/website/WEBSITE_DI_RULES.md:33)).
+
+2. The codebase already contains stateful “service” patterns (e.g. blockers stored as instance fields), which are harmless in a per-request graph but dangerous in a shared container.
+   - Example: [`LeagueService`](apps/website/lib/services/leagues/LeagueService.ts:95) stores [`submitBlocker`](apps/website/lib/services/leagues/LeagueService.ts:96).
+
+3. Clean Architecture intent: server modules are composition roots; manual wiring makes dependencies explicit and reviewable.
+   - This reduces hidden coupling and prevents accidental imports of client-only types like `lib/view-models/**`.
+
+Note: the strict contract technically allows request-scoped server DI ([`WEBSITE_DI_RULES.md`](docs/architecture/website/WEBSITE_DI_RULES.md:25)), but we choose the stronger rule (manual wiring only) to eliminate an entire class of cross-request bugs and enforcement ambiguity.
+
 ### 1.2 Standardize PageQuery contract (one discriminated union)
 
 **Goal:** all PageQueries return the exact contract described in [`WEBSITE_PAGE_QUERIES.md`](docs/architecture/website/WEBSITE_PAGE_QUERIES.md:17).
@@ -807,6 +910,19 @@ Non-negotiable target:
 - `lib/view-models/**` + Presenters: client-only; pure mapping to ViewData.
 - `templates/**`: dumb renderer; ViewData only.
 
+Naming enforcement (applies across all refactors):
+
+- API response shapes are `ApiDto`/`DTO` and stored/handled as `apiDto`.
+- PageQuery output is always named `pageDto` and typed `*PageDto`.
+- Template props are always `*ViewData` and passed as `viewData`.
+
+Abstractions enforcement (applies across all refactors):
+
+- Every PageQuery implements a common `PageQuery<PageDto>` contract (in `lib/contracts/page-queries/**`).
+- Every service implements a `Service` interface that returns `ApiDto`/`PageDto` only (in `lib/contracts/services/**`).
+- Every Presenter implements `Presenter<Input, Output>` with `Output` being `ViewData` or `ViewModel` (in `lib/contracts/presenters/**`).
+- ViewModels extend a shared base (in `lib/contracts/view-models/**`) to discourage accidental server serialization patterns.
+
 ### 9.1 Guardrails first (prevent regression)
 
 Add CI-failing checks (tests or ESLint rules). These come first to enforce Clean Architecture boundaries while refactors are in flight: