cratis-code-review

Cratis code review

A review is worth the reader's time only when it separates what must change from what could. Everything below is a criterion an analyzer does not already enforce — if the build is clean and this list is clean, the change is sound on the axes a reviewer can judge.

Review the change, not the file. A pre-existing violation in a line the change did not touch is a note, never a blocker.

Verified product sources

Package	Version	Purpose
Cratis.Arc.Core	22.10.4	Command, query, validation and analyzer surface (ARC0001–ARC0015)
Cratis.Chronicle	16.45.2	Event, projection, read-model and constraint surface
Cratis.Fundamentals	7.18.2	ConceptAs<T>, IInstancesOf<T>, the DI conventions

Reverify against the owning product repository before asserting a framework contract this file does not already state.

Route near misses

• A focused authentication, authorization, data-exposure, or event-sourcing security audit: use cratis-security-review.
• A focused Chronicle, database, .NET or React scalability analysis: use cratis-performance-review. The performance items below are the ones a general reviewer should catch in passing; do not duplicate the specialist's findings when both have run.
• Deciding whether the behavior is right at all: that is modeling, not review.

Step 1 — Run the gates first

A review that reports what the build already says is noise. Confirm the change builds clean in Debug and Release, its specifications pass, and lint and the TypeScript build are clean. Report a gate failure as the finding and stop — there is nothing to review under a red build.

Step 2 — Architecture

• Each slice is its own folder, <Module>/<Feature>/<Slice>/<Slice>.cs, with the backend artifacts together. No top-level Features/ wrapper.
• Namespace mirrors the folder path under the source root.
• Commands are record types with Handle() on the record. No separate handler class.
• Business rejection returns a ValidationResult or Result<TEvent, ValidationResult>, or comes from a validator. Never thrown from Provide() or Handle() — a throw is HTTP 500, not a validation error.
• Fetched or computed handler data is in Provide(), not inline in Handle().
• Events are record types: past tense, no mutable and no nullable properties, never carrying the event-source id, and each has an XML <summary>.
• Identity concepts derive from EventSourceId<T>, not ConceptAs<Guid>.
• Domain values are concepts, not raw Guid, string, or int.
• Projections consume events, never read models. AutoMap is on by default — .AutoMap() appears only inside a scope disabled with .NoAutoMap().
• A [Projection] id, once given explicitly and deployed, is permanent. The argument is optional; adding one to an existing projection after the fact changes its identity.
• Model-bound query custom paths use [Path("...")], never ASP.NET [Route].
• No service locator: IServiceProvider is not injected. Implementation sets come from IInstancesOf<T>, never IEnumerable<T>.
• No explicit singleton registration where [Singleton] suffices.
• Logging lives in a *Logging.cs partial with [LoggerMessage], not inline in domain code.
• No shared mutable state between commands.