Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions .coderabbit.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ reviews:
HandleAsync (NOT Handle). Private instance fields prefixed with _; constants and
static readonly use PascalCase. Sync-over-async patterns (.Result, .Wait(),
.GetAwaiter().GetResult()) are banned at build time — flag any sneak-through.

MODEL NAMING (intent-based, by role). Flag a generic `*Dto` suffix on a
REQUEST model — name inbound CQRS messages `*Command` (writes) / `*Query`
(reads), gRPC contracts `*Request`/`*Response`. `*Dto` is reserved for
read-side projections (the shape a query handler projects to, e.g.
OrderSummaryDto) and is acceptable ONLY there. So: a new `CreateXDto` or
`XRequestDto` on a command/query is a Should-consider (rename to
`*Command`/`*Query`); a `*Dto` on a query-result projection is fine. Don't
recommend renaming existing `*Dto` read projections to `*Response` —
they're query contracts, not HTTP-response models. See CLAUDE.md
"Coding Standards → Model names are intent-based, by role".
Interfaces earn their keep through consumer substitution (test mock or 2+ impls
today, OR concrete near-term roadmap) — flag speculative interfaces. Repository
wrappers around EF Core (IFooRepository + FooRepository) are explicitly NOT
Expand Down
6 changes: 6 additions & 0 deletions CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,11 @@ project's lesson, not an inconsistency to clean up.
- Private *instance* fields prefixed with `_` (camelCase). Constants and `static readonly` fields use PascalCase per .NET convention — do NOT prefix with `_` (e.g. `OrdersPlaced`, `Carriers`, `TraceIdKey`). The `.editorconfig` enforces this split via separate naming rules
- Async methods suffixed with `Async`
- Interfaces prefixed with `I`
- **Model names are intent-based, by role — prefer a specific suffix over a generic `Dto` on a request model.** A generic suffix like `CreateOrderDto` only says "data transfer object"; it doesn't say what the type *does* or where it belongs. Name by role instead:
- **CQRS messages:** `*Command` for writes (`PlaceOrderCommand`, `ProcessPaymentCommand`), `*Query` for reads (`GetOrderByIdQuery`, `SearchProductsQuery`). These are *more* intent-revealing than a generic `*Request` — they say write-vs-read *and* that the type flows the Wolverine message pipeline. This is the canonical inbound shape; do NOT introduce a `*Dto`/`*Request` request model where a Command/Query fits.
- **gRPC contracts:** `*Request` / `*Response` pairs (`GetProductRequest` / `ProductResponse`) — the gRPC idiom, already used in CatalogService.
- **Read projections:** `*Dto` (`OrderSummaryDto`, `ProductDto`) — the query-result shape a query handler projects to. `Dto` is acceptable *here only*, where it correctly signals "read-side projection / transfer shape" in a CQRS context; it is NOT coupled to HTTP, so don't rename it to `*Response`.
- The rule in one line: **generic `*Dto` on a request model hides intent — name it by what it does; reserve `*Dto` for read projections.**
- Use `var` when type is apparent
- TreatWarningsAsErrors is enabled - zero warnings allowed
- Static analyzers: Meziantou, SonarAnalyzer, Roslynator
Expand Down Expand Up @@ -208,6 +213,7 @@ These are always-on. Deeper guidance (modern EF features, transactions, caching
- **Cache invalidation in the write path**: if a handler mutates a cached entity, it must invalidate or update the cache in the same handler — not "later" or "via TTL".
- **Migrations are immutable once applied**: never edit a migration that has run anywhere (dev included). Destructive changes (drop column/table, rename, NOT NULL on existing column) need a multi-step plan, not a single migration.
- **Measure before optimizing**: don't add caching, compiled queries, `ValueTask`, or `AsSplitQuery()` on intuition. Use BenchmarkDotNet for code paths, `dotnet-counters`/k6 for system behavior, `ToQueryString()` for EF.
- **`AsSpan` over `Substring` for zero-allocation slicing — but only on profiled hot paths, and mind the async constraint.** `Substring(...)` allocates a new `string` per call; `AsSpan(...)` returns a `ReadOnlySpan<char>` view over the original with no allocation, and `string.Concat`, `int.Parse`, etc. have span overloads. This is a real win in *synchronous, hot, string-heavy loops* — parsers, formatters, tokenizers, bulk ID/field manipulation. **Two guardrails make it a narrow tool, not a default:** (1) it's a micro-optimization governed by "Measure before optimizing" above — apply it where profiling shows string-allocation pressure, not reflexively; (2) **`Span<T>`/`ReadOnlySpan<T>` is a `ref struct` — stack-only, can't cross an `await` boundary or be captured in a lambda/field**, so it's rarely usable inside NextAurora's async-everywhere request handlers (the compiler will stop you). NextAurora has no such hot path today — its string work is incidental (log templates, IDs), and the bottleneck is always I/O (EF, gRPC, HTTP), never `Substring`. The rule is preventative: *if* a synchronous string-crunching hot path appears and profiling justifies it, reach for `AsSpan`; until then, `Substring` is fine and clearer.
- **Dapper is the sanctioned escape hatch from EF**, not a peer abstraction. Reach for it only when (a) the SQL is provider-specific and doesn't translate cleanly, (b) profiling proves EF is the bottleneck on a hot path, or (c) the query is a SQL aggregation where LINQ obscures intent. Always use `ctx.Database.GetDbConnection()` so Dapper shares the EF connection and any ambient transaction — never open a separate `NpgsqlConnection`/`SqlConnection`. Writes always go through aggregates + EF (Dapper bypasses concurrency tokens, domain validation, and the outbox). Full rationale: [docs/performance-and-data-correctness.md "Decision: when to reach past EF Core (Dapper escape hatch)"](docs/performance-and-data-correctness.md#decision-when-to-reach-past-ef-core-dapper-escape-hatch).

## Testing
Expand Down
Loading