chore(claude): encode 5 architectural rules + reconcile README to VSA#39
Merged
Conversation
…nfig Adds the "factory pattern earns its keep at 2+ impls" rule to the three canonical encoding sites + a worked sketch at the reference site: - CLAUDE.md: new bullet right after the consumer-substitution rule. Codifies that .NET keyed services (AddKeyedScoped + [FromKeyedServices]) is the canonical multi-impl factory; don't hand-roll IPortFactory; don't pre-build the factory while there's only one impl. NotificationService cited as the canonical "ready for the factory, not yet wearing it" case. - .claude/agents/architecture-reviewer.md: new section keyed to Infrastructure/DependencyInjection.cs files. Two flagged patterns: premature factory (Must-fix) and missing factory at 2+ impls (Should-consider). Explicit guidance that IServiceProvider's keyed- services API IS the canonical factory. - .coderabbit.yaml: matching path_instructions block under **/Infrastructure/DependencyInjection.cs, encoding the same two patterns so CodeRabbit catches future violations at PR-review time. - NotificationService/Infrastructure/DependencyInjection.cs: composition- root XML doc now sketches the future keyed-services rewrite that the current single-AddScoped will become when SendGrid/Twilio actually ship — so the rule lands with a concrete reference shape in code, not just in config. Why now: PR #30 + #31 collapsed the I*Repository wrappers and tightened the consumer-substitution rule. The factory-pattern shape is the natural next codification — same lesson (layers without capability are speculative coupling), one tier up in abstraction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
PR #31 collapsed CatalogService from Clean Architecture to VSA, but the README still referenced CatalogService as a "deliberate Clean Architecture carve-out" and described the design as "mixed per-service architecture." Both claims contradicted the (correct) Project Structure section that said VSA across all five services. Changes: - Collapsed the four-bullet "About this repo" preamble into two bullets: Monorepo + single architectural shape (was three contradictory bullets plus a long MySQL-translation aside), and the two-database-engines bullet retained. - Updated the How It Works doc description: "Clean Architecture" -> "VSA layout" (matches docs/how-it-works.md which was already updated in the collapse). - Dropped the seven `Source: file.excalidraw` subtitle lines under the hero diagram + each of the six reference diagrams. The SVG is already embedded and clickable to full-size; the editable .excalidraw sources are now noted once in the section intro. Net: -20 lines, -1 contradiction. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Two threads in one commit: REVIEW FIXES (3) from the architecture-reviewer pass on the prior commits: 1. Reconciled categorization drift on "MISSING FACTORY at 2+ impls" between architecture-reviewer.md (was "Must-fix on absence when per-call selection is required") and .coderabbit.yaml (was "Should-consider", no qualifier). Both now agree: Must-fix when per-call routing is intended (latent bug — plain AddScoped registrations collide silently, DI returns last-registered impl every time, dropping routing intent); Should-consider when impls are interchangeable (no live bug, but tighten for explicitness). 2. Split architecture-reviewer.md's dual-category bullet into two cleaner bullets — one for the per-call-selection Must-fix case, one for the interchangeable Should-consider case. 3. Narrowed the Infrastructure DI section heading: the glob now stands alone (`**/Infrastructure/DependencyInjection.cs`), and the broader port-adapter scope is lifted into the body so the glob and the prose claim match. NEW RULE — long-running work / 202 Accepted shape. Encoded across CLAUDE.md "Performance Rules" + .coderabbit.yaml Endpoints path + architecture-reviewer Endpoints section. Rule: if a write handler can take more than ~1 second, reshape as 202 Accepted (validate + persist intent + publish Wolverine message + return job ID with Location header). Background handler does the work; client polls or gets pushed. NextAurora already has all the machinery (Wolverine + Service Bus + outbox + saga) — the rule is "use it when a handler would otherwise block on minutes-scale work." Reference shape: POST /api/v1/orders (place → publish OrderPlaced → return OrderId; downstream PaymentService + ShippingService handle saga via async events). Same rule applies to Wolverine handlers themselves — if the handler body runs for minutes, factor the work into a follow-up message handler. Why now: the rule isn't actively violated in the codebase today (no minutes-scale synchronous endpoints exist), but the encoding is what guarantees future endpoints don't introduce one. Same logic as the factory pattern rule landed in the prior commit — encode the shape before the violation, not after. Verified no existing "See CLAUDE.md" markers paraphrase a contradictory version of the new rule (the two nearest markers reference money calculations and consumer substitution, both unaffected). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
… follow-up
Three new rules + one rule tightening + one open-issue acknowledgement,
all sourced from two Milan Jovanović / Kerim Kara articles on scaling
patterns. PR scope continues to be "encode the rules before the next
instance hits review."
Rules added:
1. NON-SARGABLE PREDICATES DEFEAT INDEXES — fix at write time.
A Where(...) that wraps a column in a function (u.Email.ToLower() == x,
o.CreatedAt.Date == today, EF.Functions.ILike with leading wildcard)
defeats any B-tree index on that column. Fix at write time: normalize
on insert/update + Where against the normalized column, or use
case-insensitive column collation. Leading-wildcard substring search
isn't B-tree-indexable in any database — escalate to Postgres tsvector
full-text or a dedicated search engine when load justifies it.
CatalogService/Features/SearchProducts.cs already documents the
leading-wildcard trade-off explicitly; updated its cross-reference to
point at the new rule by name.
2. FAN-OUT BELONGS ON THE MESSAGE BUS, NOT IN A SYNCHRONOUS HANDLER LOOP.
A handler that iterates a recipient list inline and awaits a sender
per recipient holds the request open for N × per-recipient-latency
and concentrates work on one process. Right shape: publish one
Wolverine message per recipient (or per batch of K), return immediately,
let per-recipient handlers run in parallel under
MaxDegreeOfParallelism throttle. Preventative — not retroactively
violated today (NotificationService receives one event = one outbound
notification).
3. PARALLELIZE INDEPENDENT AWAITS WITH TASK.WHENALL.
Sequential await of independent I/O calls serializes latency for free.
When a handler makes N independent I/O calls (N gRPC to different
services, N HTTP to different external APIs, N queries against
DIFFERENT DbContexts), Task.WhenAll pays the max latency, not the sum.
Reference: OrderService/Features/PlaceOrder.cs:93 (gRPC fan-out over
per-line GetProductAsync with the DbContext-safety caveat documented
inline). Explicit caveats encoded: don't WhenAll dependent operations,
operations sharing the same DbContext scope (not thread-safe), or
operations whose per-call failure observability matters (WhenAll
surfaces only the first exception).
Rule tightening (from prior architecture-reviewer feedback):
4. 202 ACCEPTED RULE — two definitional clarifications.
(a) "Tracking row" can be the aggregate itself when its ID is the
polling key (POST /api/v1/orders pattern), not necessarily a
separate jobs table.
(b) "Commit atomically" branches by dispatch path: bus.InvokeAsync
gets AutoApplyTransactions for free; inline persist+publish needs
the explicit BeginTransactionAsync → SaveChangesAsync → CommitAsync
wrap from the existing Outbox-outside-handler rule. Cross-reference
added.
STATUS.md follow-up:
5. POST /api/v1/payments/process noted as a partial Should-consider
under the new long-running-work rule. Endpoint returns 202 but the
handler awaits Stripe synchronously; typical sub-second but tail
latency on degraded states is seconds-to-30s. Deferred (current
shape works for the demo; rule is encoded preventatively).
Note on the second Kerim Kara article ("Every Senior .NET Developer Has
Defended This Architecture"): most of its lessons are already encoded in
NextAurora — the I*Repository deletion (PR #30), the CatalogService
Clean→VSA collapse (PR #31), the "Interfaces earn their keep through
consumer substitution" rule, the integration-tests-over-mocked-repository
discipline, the DbContext-direct data-access shape. The article reads
as a victory lap for the simplicity refactor. The one net-new takeaway
was the sequential-awaits trap, encoded here as rule #3.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
WalkthroughThis PR updates architecture review guidance across configuration and docs, adding rules for non-sargable query predicates, async/parallel handler patterns, long-running write work via message-bus (202 + job tracking), and correct DI/keyed-service registration shapes; it also updates README, STATUS, and a few inline comments. ChangesArchitecture Review Rules and Performance Guidance
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
docs/STATUS.md (1)
5-5:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate
Last updatedto today’s date.This file changed in this PR, so the header should be updated to
2026-05-27.As per coding guidelines, when
docs/STATUS.mdchanges, the “Last updated:” date should be today.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@docs/STATUS.md` at line 5, Update the "**Last updated:**" line in docs/STATUS.md from "2026-05-26" to "2026-05-27" so the header reflects today’s date; locate the line beginning with "**Last updated:**" (the exact string shown in the diff) and replace the date only, leaving the rest of the line unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@CatalogService/Features/SearchProducts.cs`:
- Around line 16-17: Update the inline rule-summary comment inside the
SearchProducts class (in CatalogService/Features/SearchProducts.cs) so its final
sentence ends exactly with the phrase "See CLAUDE.md." — replace the current
trailing text "see CLAUDE.md " or similar with the canonical grep marker "See
CLAUDE.md." to match project guidelines.
In `@CLAUDE.md`:
- Around line 19-20: Update the reviewer checklist to include running the
paraphrase-drift audit whenever CLAUDE.md changes: when editing the rule text
(e.g., the factory/repository guidance in CLAUDE.md) ensure reviewers add the
note “Run /check-rules locally to audit paraphrases against this diff” (also
applied to the same change range at lines 193-197); run the local command
`/check-rules` to perform the paraphrase audit and record or surface any
paraphrase mismatches as part of the PR review.
---
Outside diff comments:
In `@docs/STATUS.md`:
- Line 5: Update the "**Last updated:**" line in docs/STATUS.md from
"2026-05-26" to "2026-05-27" so the header reflects today’s date; locate the
line beginning with "**Last updated:**" (the exact string shown in the diff) and
replace the date only, leaving the rest of the line unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 1d95edbe-c5b7-4026-9fb5-95a3f2aedfab
📒 Files selected for processing (7)
.claude/agents/architecture-reviewer.md.coderabbit.yamlCLAUDE.mdCatalogService/Features/SearchProducts.csNotificationService/Infrastructure/DependencyInjection.csREADME.mddocs/STATUS.md
Surfaced by /check-rules audit: `OrderService/Features/PlaceOrder.cs:138`
had a `See CLAUDE.md.` marker with no specific section named, paraphrasing
a principle ("never trust client-submitted prices for money calculations")
that wasn't a named rule in CLAUDE.md. Option 2 (per the audit prompt):
add the named rule, tighten the paraphrase to point at it.
Changes:
- CLAUDE.md "Security Requirements" gets a new named bullet:
"Server-controlled fields are computed server-side, never trusted from
the client" — covers money fields (Price/Currency/Tax), authorization
identifiers (BuyerId/SellerId), state-machine columns (Status), and
security flags (IsAdmin/IsDeleted). Specifies the failure mode (price
tampering — client submits Price=0.01 for a $999 product) and the
canonical pattern (fetch authoritative value from its source; treat
the request DTO as untrusted input). References PlaceOrder.cs as the
reference example. Cross-links to the existing "Mass assignment" check
in the architecture-reviewer.
- OrderService/Features/PlaceOrder.cs:138 cross-reference tightened from
generic `See CLAUDE.md.` to the named rule.
No other files paraphrase this principle today (verified via the prior
/check-rules audit run that found this single drift).
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@OrderService/Features/PlaceOrder.cs`:
- Around line 138-139: Update the inline comment in PlaceOrder.cs so it ends
exactly with "See CLAUDE.md." for grep-findability; replace the current trailing
section path ("Security Requirements → Server-controlled fields are computed
server-side, never trusted from the client") with the single phrase "See
CLAUDE.md." so the full comment reads as a short summary followed by that exact
reference.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f184eb68-fb55-4c1a-8701-e5390ebab319
📒 Files selected for processing (2)
CLAUDE.mdOrderService/Features/PlaceOrder.cs
4 tasks
emeraldleaf
added a commit
that referenced
this pull request
May 27, 2026
…solved #39 landed the "Long-running work belongs on the message bus" rule along with a STATUS.md "Open issues" entry flagging POST /api/v1/payments/process as a partial Should-consider under that rule. The Acceptor + Gateway split in this PR's preceding commit (c7d293b) is the resolution. Strike-through + "Resolved 2026-05-26" preserves the deferred-then-fixed trail per the STATUS.md convention used by the prior read/write data-access split resolution entry. The separate "Payment recovery: gateway-side idempotency keys not yet used" open issue (above) is unchanged — that's a different concern (Stripe-side idempotency for the redelivery double-charge race) that this refactor doesn't address and doesn't make worse. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
emeraldleaf
added a commit
that referenced
this pull request
May 27, 2026
* refactor(payments): split ProcessPayment into Acceptor + Gateway handlers First application of the long-running-work / 202 Accepted rule landed in PR #39: the Stripe gateway call is the slow part (sub-second in the happy path, seconds-to-30s on degraded states); doing it inline in the handler held an HTTP request open (and a Wolverine handler slot + DbContext + SB message lease on the saga path) for the full duration. The refactored shape: Entry points (unchanged): POST /api/v1/payments/process ─┐ ├─► ProcessPaymentCommand OrderPlacedHandler (saga) ─┘ ProcessPaymentHandler (Acceptor — fast): - idempotency check on OrderId - re-publish terminal event for completed/failed re-entry - persist Payment(Pending) + publish PaymentProcessingRequested - SaveChanges (atomic: Payment row + staged envelope) - return PaymentId No gateway call here. Sub-second. PaymentProcessingRequestedHandler (Gateway — slow, runs on Wolverine worker): - re-fetch Payment, status-guard for at-least-once redelivery - call Stripe - mark Completed/Failed, publish PaymentCompletedEvent/PaymentFailedEvent - SaveChanges (atomic: state transition + staged event) PaymentProcessingRequested is an internal Wolverine message — lives in PaymentService/Features/ (not NextAurora.Contracts) because no other service is supposed to subscribe to it. No explicit routing config means Wolverine defaults to a local in-memory queue, exactly as intended. What's preserved: - All existing idempotency semantics (OrderId existence check, terminal-state re-publish, unique-index race recovery). - Outbox atomicity: both handler paths use the publish-before-save pattern; AutoApplyTransactions + UseEntityFrameworkCoreTransactions commit entity writes + staged envelopes in one DB transaction. - The PaymentRecoveryJob sweeper still handles stuck Pendings (no behavior regression in the failure-mode recovery path). - OrderPlacedHandler cascade pattern (still returns ProcessPaymentCommand). - The HTTP endpoint still returns 202 Accepted; what changes is that the response now reflects work actually moved to the bus, not just "the downstream saga happens to be async." What's NOT fixed by this PR (separate STATUS.md item): - Gateway-side idempotency keys. The "Stripe charged but process crashed before MarkAsCompleted → redelivery double-charges" race still exists; fixing it requires plumbing payment.Id.ToString() into the gateway as an Idempotency-Key header. Tracked in STATUS.md as a separate open issue. Testing: - All 6 unit test projects pass (112 tests). PaymentService.Tests.Unit (19) confirms no regressions in validator + OrderPlacedHandler tests. - The right tier for verifying this refactor at the EF + Wolverine outbox level is PaymentService.Tests.Integration — a project that doesn't exist yet (paired STATUS.md gap with ShippingService.Tests.Integration). - Per CLAUDE.md "Testing", mocking DbContext for unit-level handler tests is explicitly out — integration tests with Testcontainers are the right axis. Note on merge order: this PR is the first application of PR #39's rule. PR #39 should land first so the rule reference in the new endpoint comment ("See CLAUDE.md Performance Rules → Long-running work...") points at a canonical rule that exists in main. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * docs(status): mark PaymentService process-endpoint Should-consider resolved #39 landed the "Long-running work belongs on the message bus" rule along with a STATUS.md "Open issues" entry flagging POST /api/v1/payments/process as a partial Should-consider under that rule. The Acceptor + Gateway split in this PR's preceding commit (c7d293b) is the resolution. Strike-through + "Resolved 2026-05-26" preserves the deferred-then-fixed trail per the STATUS.md convention used by the prior read/write data-access split resolution entry. The separate "Payment recovery: gateway-side idempotency keys not yet used" open issue (above) is unchanged — that's a different concern (Stripe-side idempotency for the redelivery double-charge race) that this refactor doesn't address and doesn't make worse. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
5 tasks
emeraldleaf
added a commit
that referenced
this pull request
May 28, 2026
* chore(claude): encode distributed rate-limit rule + annotate registration sites ASP.NET Core's built-in RequireRateLimiting + AddFixedWindowLimiter / AddSlidingWindowLimiter stores counters in-process — correct for single-instance deployment, but the limit silently multiplies by N when scaled to N instances. Each instance enforces its own counter; a client hitting any instance gets a fresh allowance. NextAurora is single-instance everywhere today (CatalogService deployed on one Fly Machine; the other four services local), so the in-memory limiter is correct *for now*. This PR encodes the rule preventatively + marks the two registration sites with the future-swap trigger. Encoded across the three canonical sites: - CLAUDE.md "Security Requirements → Rate Limiting": extended bullet covering the in-memory weakening + Redis-backed swap + Lua-EVAL for atomic increment+TTL. - .coderabbit.yaml `**/Endpoints/**/*.cs` path: RATE-LIMIT SHAPE block flagging missing justification comments + deployment-config scale-out without paired limiter swap. - .claude/agents/architecture-reviewer.md Endpoints pattern checklist: Rate-limiter shape bullet (Should-consider on single-instance, Must-fix when scaled to 2+ instances). Reference-comment annotation at both registration sites (the "AddFixedWindowLimiter" calls in CatalogService/Program.cs and PaymentService/Program.cs) — documents WHY in-memory is correct today and WHAT triggers the swap. Mirrors the worked-sketch pattern from PR #39 (NotificationService DependencyInjection.cs). Connection to docs/full-saga-deployment-plan.md (PR #42): Phase 3 deliverable already lists the audit + Redis-backed swap when scale-out happens. The rule encoded here is the rule that audit applies. Source: a Milan Jovanović article on distributed rate limiting with Redis (the "Scale Rate Limiting with Redis" piece) — same pattern as other rule-encoding PRs in this session: read article → encode the discrete lesson → apply where it'll be findable. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> * chore(claude): address CodeRabbit — inline rule comments end with 'See CLAUDE.md.' CodeRabbit flagged the two rate-limiter annotation comments for not ending with the canonical grep-auditable marker. The repo convention (CLAUDE.md "Debugging Discipline") is that any inline comment summarizing a CLAUDE.md rule ends with `See CLAUDE.md.` so it's findable via grep. Reshaped both comments: the section reference now reads as a preceding "Rule: Security Requirements → Rate Limiting." sentence, and the comment ends with the literal `See CLAUDE.md.` marker. Keeps the section pointer for human readers while satisfying the grep convention exactly. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.7 <noreply@anthropic.com>
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Encodes 5 new architectural rules across
CLAUDE.md,.coderabbit.yaml, and.claude/agents/architecture-reviewer.md— the three canonical sites per CLAUDE.md "Continuous Rule Encoding." Also reconcilesREADME.mdpost-VSA-collapse, adds a worked sketch inNotificationService, and logs one open issue inSTATUS.md. No production C# behavior changed; one comment cross-reference updated.The 5 rules encoded
Factory pattern /
[FromKeyedServices]earns its keep at 2+ impls — not before. Single-impl ports stay asAddScoped<IPort, OnlyImpl>(); the keyed-services factory shape is for the day a second adapter actually ships.NotificationService/Infrastructure/DependencyInjection.csdocuments the future shape in XML doc as the canonical "ready for the factory, not yet wearing it" reference.Long-running work belongs on the message bus, not the synchronous HTTP handler (202 Accepted). Handlers that block >1s reshape as
202 Accepted(validate + persist tracking row + publish Wolverine message + return job ID withLocationheader). Background handler does the work. Same rule applies to Wolverine handlers themselves (minutes-scale handler body → factor into a follow-up message). Reference:POST /api/v1/orders.Non-sargable predicates defeat indexes — fix at write time.
Where(...)that wraps a column in a function (u.Email.ToLower() == x,EF.Functions.ILike(p.Name, "%text%")with leading wildcard) can't use a B-tree index. Fix at write time: normalize on insert/update +Whereagainst the normalized column, or use case-insensitive column collation. Leading-wildcard substring search escalates totsvectoror a dedicated search engine. Reference:CatalogService/Features/SearchProducts.cs.Fan-out belongs on the message bus, not in a synchronous handler loop. Iterating a recipient list inline (
foreach (var follower in followers) await _sender.SendAsync(...)) serializes N × per-recipient latency on one process and creates traffic spikes. Right shape: publish one Wolverine message per recipient (or per batch of K), return immediately, throttle viaMaxDegreeOfParallelism. Preventative; not retroactively violated.Parallelize independent awaits with
Task.WhenAll— sequential awaits serialize latency for free. N independent I/O calls (gRPC to different services, HTTP to different external APIs, queries against differentDbContexts) shouldTask.WhenAll, not sequentialawait. Reference:OrderService/Features/PlaceOrder.cs:93(gRPC fan-out over per-lineGetProductAsyncwith the DbContext-safety caveat documented inline). Explicit anti-patterns flagged: don'tWhenAlldependent operations, same-DbContextqueries (not thread-safe), or operations whose per-call failure observability matters.Sources
Rules 2, 3, 4, 5 were sourced from two recent Milan Jovanović / Kerim Kara articles on scaling patterns. Rule 1 was sourced from an article on the factory pattern with .NET keyed services. A second Kerim Kara article ("Every Senior .NET Developer Has Defended This Architecture") was reviewed and found to be largely a victory lap for NextAurora's existing encoded rules — the only net-new takeaway (sequential-awaits trap) became rule 5.
Plus
Source: file.excalidrawsubtitle lines (SVGs are already embedded + clickable). Fixed the How It Works doc description..coderabbit.yamlandarchitecture-reviewer.mdon the "2+ impls missing factory" case reconciled. Dual-category bullet split. Infrastructure DI section heading narrowed.POST /api/v1/orderspattern), and added an explicit branch forbus.InvokeAsyncvs inline-publish atomicity wraps.POST /api/v1/payments/processnoted as a partial Should-consider under the new long-running-work rule (handler awaits Stripe synchronously; tail latency on degraded states is seconds-to-30s). Deferred.Test plan
dotnet buildclean (0 warnings)See CLAUDE.mdmarkers point at stale rule paraphrasespath_instructionsblocks on next PR🤖 Generated with Claude Code
Summary by CodeRabbit
Documentation
Chores
Style