docs: add 5 architecture decision records#14
Conversation
Adds initial ADR set documenting Compendium's structural choices: - 0001 Result pattern over exceptions for business errors - 0002 Hexagonal architecture (strict) - 0003 Zero-dependency Core - 0004 Multi-tenancy strategy - 0005 Event sourcing over state-stored persistence Plus index (README.md) and MADR template (0000-template.md).
There was a problem hiding this comment.
Pull request overview
Adds a new docs/adr/ documentation area to capture Compendium’s key architectural decisions using the MADR format, intended to serve as a durable reference for contributors and consumers.
Changes:
- Introduces an ADR index (
docs/adr/README.md) and a reusable MADR template (docs/adr/0000-template.md). - Adds five “Accepted” ADRs documenting major architectural decisions (Result pattern, strict hexagonal architecture, zero-dependency Core, multi-tenancy, event sourcing).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/adr/README.md | ADR landing page and index table for quick navigation. |
| docs/adr/0000-template.md | MADR template for consistent future ADR submissions. |
| docs/adr/0001-result-pattern.md | Documents the decision to prefer Result<T> for business errors. |
| docs/adr/0002-hexagonal-architecture.md | Documents strict ports/adapters boundaries and dependency direction. |
| docs/adr/0003-zero-dep-core.md | Documents “Core has zero NuGet dependencies” constraint. |
| docs/adr/0004-multi-tenancy-strategy.md | Documents tenant resolution/consistency/isolation approach. |
| docs/adr/0005-event-sourcing-vs-state.md | Documents event sourcing as the source-of-truth persistence model. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| - Aggregates (`AggregateRoot<TId>` in `Compendium.Core`) raise domain events; the event stream is the source of truth. | ||
| - The PostgreSQL adapter (`Compendium.Adapters.PostgreSQL`) implements the event store. State is rebuilt by replaying events. | ||
| - Read models are **projections**: derived, eventually consistent, replayable from the event log. Live in `Compendium.Infrastructure` (generic projection plumbing) plus per-consumer projection handlers. | ||
| - Outbox pattern for cross-boundary publishing of events to external systems, ensuring at-least-once delivery without distributed transactions. | ||
| - Snapshots are an optimisation, not a substitute for the log; an aggregate's history is always reconstructible from events alone. |
There was a problem hiding this comment.
The ADR states an Outbox pattern is part of the decision, but there doesn't appear to be any Outbox implementation/types in the repository (no Outbox* classes or infrastructure). If Outbox is not implemented yet, consider rewording this as a planned follow-up (or changing status), or link to the actual component name if it exists under a different term.
| - `Compendium.Core.Results.Result<T>` and `Error` live in the zero-dependency Core (see [ADR 0003](0003-zero-dep-core.md)). | ||
| - Command and query handlers, domain factories, and adapter operations all return `Result<T>`. | ||
| - Exceptions are reserved for: programmer bugs (assertion failures, never-meant-to-happen branches), infrastructure that genuinely cannot be a `Result` (cancellation, OOM), and a thin translation layer at the HTTP edge. | ||
| - `Result.Failure(...)` and `Result.Success(...)` are the only sanctioned construction paths. No `null` for "absent value" — use `Result.Failure(Error.NotFound(...))`. |
There was a problem hiding this comment.
This claims Result.Success(...) / Result.Failure(...) are the only sanctioned construction paths, but the current Result API also provides implicit conversions (e.g. TValue -> Result<TValue> and Error -> Result<T>). Either document these implicit conversions as sanctioned, or tighten the API if you want to enforce only factory usage.
| - `Result.Failure(...)` and `Result.Success(...)` are the only sanctioned construction paths. No `null` for "absent value" — use `Result.Failure(Error.NotFound(...))`. | |
| - Prefer `Result.Failure(...)` and `Result.Success(...)` as the explicit construction paths. Where the API provides implicit conversions (for example `TValue -> Result<TValue>` and `Error -> Result<T>`), those are also sanctioned convenience forms. No `null` for "absent value" — use `Result.Failure(Error.NotFound(...))`. |
| | # | Title | Status | Date | | ||
| |---|---|---|---| | ||
| | 0001 | [Result pattern over exceptions](0001-result-pattern.md) | Accepted | 2026-Q2 | | ||
| | 0002 | [Hexagonal architecture (strict)](0002-hexagonal-architecture.md) | Accepted | 2026-Q2 | | ||
| | 0003 | [Zero-dependency Core](0003-zero-dep-core.md) | Accepted | 2026-Q2 | |
There was a problem hiding this comment.
The index uses quarter-style dates (e.g. 2026-Q2) while the ADRs themselves and the template use YYYY-MM-DD. This inconsistency makes it harder to sort/filter ADRs and can confuse readers; consider using the same ISO date format in the README (or explicitly document why the index uses quarters).
|
|
||
| The framework provides `Compendium.Multitenancy` with these moving parts: | ||
|
|
||
| - **`TenantContext`** — a small immutable record carrying `TenantId` plus its provenance (which source produced it). Registered as a scoped service; one per request. |
There was a problem hiding this comment.
This describes TenantContext as an immutable record with provenance and as a scoped "one per request" service. In the current codebase, TenantContext is a mutable class backed by AsyncLocal, and DI registers tenant context/accessor as singletons (per-async-flow isolation rather than per-request scope). Please update the ADR to match the actual types/lifetimes (or adjust the implementation to match the ADR).
| ### Positive | ||
| - Cross-tenant access is impossible by default: any code path that forgets to scope a query reads from `TenantContext`, and `TenantContext` is request-scoped and validated. | ||
| - Multi-source consistency catches misconfigured gateways (e.g. JWT for tenant A but `X-Tenant-ID` for tenant B) and trivial token-replay attempts. |
There was a problem hiding this comment.
These bullets imply the tenant-scoping guarantees come from TenantContext being request-scoped. Given TenantContextAccessor/ITenantContext are registered as singletons and rely on AsyncLocal, the guarantee is different (async-context scoping). The ADR should describe the actual isolation mechanism so readers don't assume ASP.NET Core DI scoping is the enforcement point.
## Summary PR #23 incorrectly bumped CHANGELOG to `[1.0.0-preview.2] - 2026-04-26` with the quality-sweep entries — but tag `v1.0.0-preview.2` was already cut on **2026-04-25** from a different commit set (PRs #1-7) and **already published to nuget.org** (`Compendium.Core 1.0.0-preview.2` is live). Reusing that version was a mistake. This PR reconciles the CHANGELOG with the published reality and rolls today's work into a new **preview.3**: ### `[1.0.0-preview.2] - 2026-04-25` — rewritten retroactively Now matches the auto-generated GitHub release notes for `v1.0.0-preview.2`: - **Added** — `Compendium.Adapters.Shared` (PII masking utilities, introduced in #3). - **Changed** — Dependabot bumps #4-7, OSS governance scaffolding. - **Security** — workflow `permissions:` block (#1), tenant log sanitization (#2), email removal from adapter logs / GDPR (#3). ### `[1.0.0-preview.3] - 2026-04-26` — new Everything since `v1.0.0-preview.2`: - **Added** — DocFX site (#17), 5 ADRs (#14), public ROADMAP (#15), getting-started guide (#20), 4 concept pages (#21), 8 adapter how-to guides (#22). - **Changed** — CodeQL Default Setup → `extended` query suite. - **Security** — `softprops/action-gh-release` pinned to commit SHA (#16, alert #28 closed). ## Test plan - [ ] CI green on this PR. - [ ] After merge, tag `v1.0.0-preview.3` triggers Release workflow successfully. - [ ] `Compendium.* @ 1.0.0-preview.3` published on nuget.org. - [ ] GitHub Release `v1.0.0-preview.3` created with auto-generated notes. VK: POM-186 (Code Quality sweep parent). Co-authored-by: sacha <sacha@scojhconsult.com>
Summary
docs/adr/directory with MADR-format ADRs documenting Compendium's structural choicesREADME.md) and reusable template (0000-template.md)ADRs added
Each ADR is < 60 lines and follows MADR sections: Status, Date, Context, Decision, Consequences (Positive / Negative-Trade-offs), Alternatives considered.
Notes
docs/toc.ymlintegration intentionally skipped — POM-180 (docs scaffolding) lands separately and will wire ADRs into the TOC there.Test plan