From 36dc2c8aa8b7ce477072a28c24a856f0a4bdc0e9 Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 00:16:14 +0200 Subject: [PATCH 1/4] Add convention: no local functions inside methods Extract a normal method instead; a trivial helper of up to ~3 lines is the only exception. Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 2 ++ docs/technical-requirements.md | 2 ++ 2 files changed, 4 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 0311e12..37050da 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -93,6 +93,8 @@ restart the AppHost. - UTC instants end in `Utc` (`CreatedAtUtc`). Identifiers are spelled out — no abbreviations. - Private fields are camelCase without an underscore prefix. +- No local functions inside methods — extract a normal method. The only + exception is a trivial helper of up to ~3 lines. - Comments only for a non-obvious *why*, one sentence; when in doubt, none. - Correctness analyzer rules are build errors (see [.editorconfig](.editorconfig)); style rules are suggestions. `TreatWarningsAsErrors` is on. diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index fb36da8..d14f4ec 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -298,6 +298,8 @@ rating bounds) — it is the demo's headline claim. - **Naming:** UTC instants end in `Utc` (`CreatedAtUtc`); identifiers are spelled out in full — no abbreviations; private fields are camelCase without an underscore prefix. +- **No local functions inside methods** — extract a normal method instead; + the only exception is a trivial helper of up to ~3 lines. - **Comments** only where they capture a non-obvious *why*; when in doubt, none. - **Exhaustive switches:** enum switch expressions never have a `default`/`_` From 25f00c070dc07bcfb324fcd677bd77bdf989effe Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 11:32:53 +0200 Subject: [PATCH 2/4] Add convention: tests never reshape production code Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 10 ++++++++-- docs/technical-requirements.md | 7 +++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 37050da..8d2e51d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -79,12 +79,18 @@ restart the AppHost. assert raw JSON from anonymous payloads; property tests generate strong-typed values (`Kalicz.StrongTypes.FsCheck`). E2E signs in through the real Zitadel form. -11. **The OpenAPI document is the frontend contract.** Frontend code only +11. **Tests never reshape production code.** No signature, visibility, setter, + or constructor on a domain entity or business operation ever changes to + accommodate a test — no test-only overloads, no `InternalsVisibleTo`. + Tests reach the state they need through the public domain API or + dependency injection; when neither suffices, the test manipulates state + itself (seed through EF, reflection as a last resort). +12. **The OpenAPI document is the frontend contract.** Frontend code only calls the API through the generated `openapi-fetch` client. After changing any DTO/route: run the stack, `npm --prefix frontend run refresh:api`, commit `openapi.json` + `schema.d.ts` together with the API change — the E2E contract test fails otherwise. -12. **Nothing blanket-global.** `[Authorize]` per action (no +13. **Nothing blanket-global.** `[Authorize]` per action (no `RequireAuthorization()` on the route table), ServiceDefaults in its own namespace, no global route middleware in the frontend. diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index d14f4ec..2f87255 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -293,6 +293,13 @@ per test; no mocking anywhere (ADR-0007). The OpenAPI document itself is asserted in integration tests (email format, `minLength`, `exclusiveMinimum`, rating bounds) — it is the demo's headline claim. +**Tests never reshape production code.** No signature, visibility, setter, or +constructor on a domain entity or business operation ever changes to +accommodate a test — no test-only overloads, no `InternalsVisibleTo`. Tests +reach the state they need through the public domain API or dependency +injection; when neither suffices, the test manipulates state itself (seeding +through EF, reflection as a last resort). + ## 12. Coding conventions - **Naming:** UTC instants end in `Utc` (`CreatedAtUtc`); identifiers are From 11f2c00a7cc837f79439c5676255f4ffd8d99891 Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 12:26:28 +0200 Subject: [PATCH 3/4] Add convention: EF navigation set with FK; DDD framing for test rule Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 34 +++++++++++++++++++++------------- docs/technical-requirements.md | 19 +++++++++++++------ 2 files changed, 34 insertions(+), 19 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 8d2e51d..0c92d4e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,27 +70,35 @@ restart the AppHost. constructors demand the loaded data (`ProductWithReviews`, `ReviewWithVotes` via `CompleteQueries`) — never pass entities around and hope the navigation was included. -8. **Denormalized aggregates are recomputed, never incremented** — always from +8. **EF: set the navigation property together with the FK.** When assigning a + relationship by id (e.g. `ProductId`), also assign the navigation property + (`Product`) — in constructors, factory methods, and setters alike — so the + navigation is usable and non-null right after the call. The exception is + performance-sensitive batch work (inserting thousands of rows), where it's + fine to set only the id rather than load entities nothing will read. +9. **Denormalized aggregates are recomputed, never incremented** — always from source rows, in the same `SaveChanges`. -9. **Seeding happens at startup** (idempotent, through domain entities), never - in migrations. Migrations are schema-only and tool-generated — never edit - them by hand. -10. **Tests use real dependencies.** No mocking libraries. Integration tests +10. **Seeding happens at startup** (idempotent, through domain entities), never + in migrations. Migrations are schema-only and tool-generated — never edit + them by hand. +11. **Tests use real dependencies.** No mocking libraries. Integration tests assert raw JSON from anonymous payloads; property tests generate strong-typed values (`Kalicz.StrongTypes.FsCheck`). E2E signs in through the real Zitadel form. -11. **Tests never reshape production code.** No signature, visibility, setter, - or constructor on a domain entity or business operation ever changes to - accommodate a test — no test-only overloads, no `InternalsVisibleTo`. - Tests reach the state they need through the public domain API or - dependency injection; when neither suffices, the test manipulates state - itself (seed through EF, reflection as a last resort). -12. **The OpenAPI document is the frontend contract.** Frontend code only +12. **Tests never reshape production code.** The domain model is designed for + the domain, not for testability. No signature, visibility, setter, or + constructor on a domain entity or business operation ever changes to + accommodate a test — no test-only overloads, no `InternalsVisibleTo`. If a + test needs some capability, the test figures out how to achieve it: + through the public domain API or dependency injection; when neither + suffices, by manipulating state itself (seed through EF, reflection as a + last resort). +13. **The OpenAPI document is the frontend contract.** Frontend code only calls the API through the generated `openapi-fetch` client. After changing any DTO/route: run the stack, `npm --prefix frontend run refresh:api`, commit `openapi.json` + `schema.d.ts` together with the API change — the E2E contract test fails otherwise. -13. **Nothing blanket-global.** `[Authorize]` per action (no +14. **Nothing blanket-global.** `[Authorize]` per action (no `RequireAuthorization()` on the route table), ServiceDefaults in its own namespace, no global route middleware in the frontend. diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index 2f87255..5d1a0fc 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -293,12 +293,13 @@ per test; no mocking anywhere (ADR-0007). The OpenAPI document itself is asserted in integration tests (email format, `minLength`, `exclusiveMinimum`, rating bounds) — it is the demo's headline claim. -**Tests never reshape production code.** No signature, visibility, setter, or -constructor on a domain entity or business operation ever changes to -accommodate a test — no test-only overloads, no `InternalsVisibleTo`. Tests -reach the state they need through the public domain API or dependency -injection; when neither suffices, the test manipulates state itself (seeding -through EF, reflection as a last resort). +**Tests never reshape production code.** The domain model is designed for the +domain, not for testability. No signature, visibility, setter, or constructor +on a domain entity or business operation ever changes to accommodate a test — +no test-only overloads, no `InternalsVisibleTo`. If a test needs some +capability, the test figures out how to achieve it: through the public domain +API or dependency injection; when neither suffices, by manipulating state +itself (seeding through EF, reflection as a last resort). ## 12. Coding conventions @@ -307,6 +308,12 @@ through EF, reflection as a last resort). an underscore prefix. - **No local functions inside methods** — extract a normal method instead; the only exception is a trivial helper of up to ~3 lines. +- **Entity Framework: set the navigation property together with the FK** — + when assigning a relationship by id (e.g. `ProductId`), also assign the + navigation property, in constructors, factory methods, and setters alike, + so the navigation is usable and non-null right after the call; for + performance-sensitive batch inserts it's fine to set only the id rather + than load entities nothing will read. - **Comments** only where they capture a non-obvious *why*; when in doubt, none. - **Exhaustive switches:** enum switch expressions never have a `default`/`_` From a4ebe46a600fecbab07136c2875e302a081a6bfa Mon Sep 17 00:00:00 2001 From: KaliCZ Date: Thu, 16 Jul 2026 12:31:25 +0200 Subject: [PATCH 4/4] Use the global-rules wording for the three conventions Co-Authored-By: Claude Fable 5 --- CLAUDE.md | 33 +++++++++++++++++---------------- docs/technical-requirements.md | 32 +++++++++++++++++--------------- 2 files changed, 34 insertions(+), 31 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 0c92d4e..99221dd 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -70,12 +70,13 @@ restart the AppHost. constructors demand the loaded data (`ProductWithReviews`, `ReviewWithVotes` via `CompleteQueries`) — never pass entities around and hope the navigation was included. -8. **EF: set the navigation property together with the FK.** When assigning a - relationship by id (e.g. `ProductId`), also assign the navigation property - (`Product`) — in constructors, factory methods, and setters alike — so the - navigation is usable and non-null right after the call. The exception is - performance-sensitive batch work (inserting thousands of rows), where it's - fine to set only the id rather than load entities nothing will read. +8. **Entity Framework: set the navigation property together with the FK.** + When assigning a relationship by ID (e.g. `ProductId`), also assign the + navigation property (`Product`) — in constructors, factory methods, and + setters alike — so that right after the call, `something.Product` is + usable and non-null. The exception is performance-sensitive work like + inserting thousands of rows in a batch: there it's fine to set only the ID + rather than load entities into memory that nothing will read. 9. **Denormalized aggregates are recomputed, never incremented** — always from source rows, in the same `SaveChanges`. 10. **Seeding happens at startup** (idempotent, through domain entities), never @@ -85,14 +86,12 @@ restart the AppHost. assert raw JSON from anonymous payloads; property tests generate strong-typed values (`Kalicz.StrongTypes.FsCheck`). E2E signs in through the real Zitadel form. -12. **Tests never reshape production code.** The domain model is designed for - the domain, not for testability. No signature, visibility, setter, or - constructor on a domain entity or business operation ever changes to - accommodate a test — no test-only overloads, no `InternalsVisibleTo`. If a - test needs some capability, the test figures out how to achieve it: - through the public domain API or dependency injection; when neither - suffices, by manipulating state itself (seed through EF, reflection as a - last resort). +12. **Tests never reshape production code.** We use DDD: the domain model is + designed for the domain, not for testability. If a test needs some + capability, the test figures out how to achieve it — never add methods, + parameters, setters, or hooks to production code just so a test can reach + something. Dependency injection should be enough; otherwise the test + manipulates state itself (seed through EF, reflection as a last resort). 13. **The OpenAPI document is the frontend contract.** Frontend code only calls the API through the generated `openapi-fetch` client. After changing any DTO/route: run the stack, `npm --prefix frontend run refresh:api`, @@ -107,8 +106,10 @@ restart the AppHost. - UTC instants end in `Utc` (`CreatedAtUtc`). Identifiers are spelled out — no abbreviations. - Private fields are camelCase without an underscore prefix. -- No local functions inside methods — extract a normal method. The only - exception is a trivial helper of up to ~3 lines. +- No nested local functions — write a normal method. Don't reach for local + functions; extract a private method instead. A one-liner local function can + occasionally be acceptable, but that's an edge case, not something to aim + for. - Comments only for a non-obvious *why*, one sentence; when in doubt, none. - Correctness analyzer rules are build errors (see [.editorconfig](.editorconfig)); style rules are suggestions. `TreatWarningsAsErrors` is on. diff --git a/docs/technical-requirements.md b/docs/technical-requirements.md index 5d1a0fc..2794089 100644 --- a/docs/technical-requirements.md +++ b/docs/technical-requirements.md @@ -293,27 +293,29 @@ per test; no mocking anywhere (ADR-0007). The OpenAPI document itself is asserted in integration tests (email format, `minLength`, `exclusiveMinimum`, rating bounds) — it is the demo's headline claim. -**Tests never reshape production code.** The domain model is designed for the -domain, not for testability. No signature, visibility, setter, or constructor -on a domain entity or business operation ever changes to accommodate a test — -no test-only overloads, no `InternalsVisibleTo`. If a test needs some -capability, the test figures out how to achieve it: through the public domain -API or dependency injection; when neither suffices, by manipulating state -itself (seeding through EF, reflection as a last resort). +**Tests never reshape production code.** We use DDD: the domain model is +designed for the domain, not for testability. If a test needs some +capability, the test figures out how to achieve it — never add methods, +parameters, setters, or hooks to production code just so a test can reach +something. Dependency injection should be enough; otherwise the test +manipulates state itself (seeding through EF, reflection as a last resort). ## 12. Coding conventions - **Naming:** UTC instants end in `Utc` (`CreatedAtUtc`); identifiers are spelled out in full — no abbreviations; private fields are camelCase without an underscore prefix. -- **No local functions inside methods** — extract a normal method instead; - the only exception is a trivial helper of up to ~3 lines. -- **Entity Framework: set the navigation property together with the FK** — - when assigning a relationship by id (e.g. `ProductId`), also assign the - navigation property, in constructors, factory methods, and setters alike, - so the navigation is usable and non-null right after the call; for - performance-sensitive batch inserts it's fine to set only the id rather - than load entities nothing will read. +- **No nested local functions — write a normal method.** Don't reach for + local functions; extract a private method instead. A one-liner local + function can occasionally be acceptable, but that's an edge case, not + something to aim for. +- **Entity Framework: set the navigation property together with the FK.** + When assigning a relationship by ID (e.g. `ProductId`), also assign the + navigation property (`Product`) — in constructors, factory methods, and + setters alike — so that right after the call, `something.Product` is usable + and non-null. The exception is performance-sensitive work like inserting + thousands of rows in a batch: there it's fine to set only the ID rather + than load entities into memory that nothing will read. - **Comments** only where they capture a non-obvious *why*; when in doubt, none. - **Exhaustive switches:** enum switch expressions never have a `default`/`_`