From 75fdb052059420d666461e5115a05b59c41b5312 Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Mon, 6 Jul 2026 14:31:03 -0500 Subject: [PATCH 1/5] docs(todo): close TRIM-004 (PR #68, CI gate green); draft TRIM-001 in full Co-Authored-By: Claude Fable 5 --- ...ositional-record-signature-preservation.md | 104 +++++++++++++++++- .../plans/004-trimming-harness-ci-gate.md | 8 +- .../todo.md | 2 +- 3 files changed, 105 insertions(+), 9 deletions(-) diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md index 8e061601..79765413 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md @@ -1,11 +1,107 @@ # TRIM-001 — Positional-record preservation in factory signatures **Plan #:** 001 -**Status:** Draft -**Plan-review opt-in:** TBD at draft -**Code-review opt-in:** TBD at draft +**Date:** 2026-07-06 **Related Todo:** [../todo.md](../todo.md) +**Status:** Draft +**Last Updated:** 2026-07-06 +**Plan-review opt-in:** Yes (changes the generator's emission contract for every consumer's registrar; touches documented behavior in docs/trimming.md and CLAUDE-DESIGN.md; incremental-pipeline equality semantics involved) +**Code-review opt-in:** Yes (behavior-changing generator work) + +--- ## Scope -Make the factory-signature DTO walk preserve positional records (types with only parameterized ctors) instead of silently dropping them. Today `DtoTypeWalker.WalkFactoryReturn` requires `HasParameterlessCtor`, so a record like zTreatment's `StartVisitResultV2` — returned from a `[Remote, Execute]` command — gets no preservation and the trimmed client throws `DeserializeNoConstructor`. The fix shape already exists in the codebase's own history: bucket-sort discovered types the way the (now-dead) `WalkEventRoot` did — parameterless ctor → `DtoConstructorRegistry.Register(() => new T())`, parameterized/record → `DtoConstructorRegistry.PreserveType()` (deserialization then flows through the existing `RecordBypassConverterFactory`). Applies uniformly to return types, non-service parameters, and nested properties of discovered DTOs. Includes disposing of the dead `WalkEventRoot` helper (delete or refit as the shared bucket-sort walk), a publish-trimmed test in `RemoteFactory.TrimmingTests` covering record-as-return, record-as-parameter, and record-nested-in-DTO, and the `docs/trimming.md` "What Qualifies as a DTO" correction. Does NOT touch `[Factory]` entity property descent (TRIM-002) or event preservation (TRIM-003). +Make the factory-signature DTO walk preserve positional records (types with only parameterized public ctors) instead of silently dropping them. Today `DtoTypeWalker.WalkFactoryReturn` requires `HasParameterlessCtor`, so a record like zTreatment's `StartVisitResultV2` — returned from a `[Remote, Execute]` command — gets no preservation and the trimmed client throws `DeserializeNoConstructor`. The fix shape already exists in the codebase's own history: bucket-sort discovered types the way the (now-dead) `WalkEventRoot` did — parameterless ctor → `DtoConstructorRegistry.Register(() => new T())`, parameterized/record → `DtoConstructorRegistry.PreserveType()` (deserialization then flows through the existing `RecordBypassConverterFactory`). Applies uniformly to return types, non-service parameters, and nested properties of discovered DTOs — with property descent into both buckets. Includes retiring the dead `WalkEventRoot` in favor of the refit shared walk, trimmed-harness repro checks (record-as-return, record-as-parameter, record-nested-in-DTO), and the docs/Design corrections for "What Qualifies as a DTO". Does NOT touch `[Factory]` entity property descent (TRIM-002), event preservation (TRIM-003), or the over-retention question (TRIM-005). + +--- + +## Intent + +- A consumer returning or accepting a positional record through any factory method gets a trimming-safe client with zero manual preservation work — the exact shape that broke zTreatment's cut-over. +- The Design projects' already-documented promise (`ExampleRecordResult(int Id, string Name)` as an interface-factory return type, `AllPatterns.cs`) becomes true under `PublishTrimmed=true`, not just in untrimmed test runs. +- The generator's two-bucket emission (Register vs PreserveType) becomes the documented, tested contract for DTO preservation going forward — TRIM-002 reuses it for entity property descent. + +--- + +## Framework & Architectural Alignment + +- Emission lands in the generated `FactoryServiceRegistrar` alongside the existing `Register` calls — same pattern, second bucket (`PreserveType` already exists at `DtoConstructorRegistry` with `[DynamicallyAccessedMembers(All)]` rooting; it is currently emitted nowhere). +- Deserialization path unchanged: parameterized-ctor types are claimed by `RecordBypassConverterFactory` (existing detection rule matches the bucket rule); parameterless DTOs keep flowing through `NeatooJsonTypeInfoResolver.CreateObject`. +- Roslyn incremental-generator discipline: model types stay value-equatable (`EquatableArray`) so pipeline caching doesn't regress. +- Unit-test pattern: `DiagnosticTestHelper.RunGenerator` + assertions over generated trees (the `NestedDtoDiscoveryTests` shape). +- Trimmed-repro pattern: named bool checks in the `RemoteFactory.TrimmingTests` harness under the TRIM-004 exit-code contract. + +--- + +## Constraints & Invariants + +- Existing `Register` emission for parameterless DTOs is unchanged — same call shape, same idempotent `TryAdd` semantics in consumer registrars. +- `IsDtoStructureCandidate` exclusions stay intact: `[Factory]` types (direct or via interface), `System.*`, primitives, abstract/interface types get neither bucket. +- A type never lands in both buckets, and the visited-set dedupe holds across return/parameter/nested discovery within a method and across methods within a type. +- The existing DtoDiscovery unit tests and the full suite stay green on net9.0 + net10.0. +- The TrimmingTests harness stays green in CI (TRIM-004 gate) — new checks added, existing checks untouched. + +--- + +## Steps + +1. Replace the walker's parameterless-ctor gate with bucket classification — parameterless → Register bucket, parameterized-public-ctor → PreserveType bucket — shared across return types, non-service parameters, and nested property descent (descending into both buckets' types). Retire the dead `WalkEventRoot` in favor of this shared walk and fix the stale file-header comment that still names the removed relay-handler caller. +2. Thread the ctor-shape distinction through the discovery pipeline: method-level discovery → per-type aggregation/dedupe → model builder → all three factory models, preserving value equality for incremental caching. +3. Emit `PreserveType()` alongside `Register()` in all three registrar renderers (class, interface, static). +4. Extend the DtoDiscovery unit tests to pin bucket assignment: records as return type, as parameter, nested in a class DTO, class DTO nested in a record; existing exclusions still emit nothing. +5. Add trimmed-harness repro checks mirroring the consumer failure: a positional record returned from a `[Remote, Execute]` command, taken as a parameter, and nested as a property of a discovered DTO — each serializer round-tripped on the trimmed client (the `EventRelaySmokeTest` shape). +6. Update the documentation to the shipped behavior: `docs/trimming.md` "What Qualifies as a DTO" (records are preserved via `PreserveType`, not merely "handled separately by `RecordBypassConverterFactory`" — that sentence conflates deserialization mechanics with preservation), CLAUDE-DESIGN.md's DTO-registry section and FAQ row, and the `AllPatterns.cs` comments around `ExampleRecordResult`. + +--- + +## Acceptance + +Tier note: `[trimmed-harness]` is a project-local tier — a named check in `RemoteFactory.TrimmingTests` executed under `PublishTrimmed=true` by the CI trimming gate (TRIM-004). + +- [ ] The generator emits `PreserveType()` for a positional record appearing as a factory-method return type, as a non-service parameter, and as a property of a discovered DTO; `Register()` emission for parameterless DTOs is unchanged. `[unit]` +- [ ] A positional record returned from a `[Remote, Execute]` command round-trips through the Neatoo serializer on a publish-trimmed client (the zTreatment `StartVisitResultV2` shape). `[trimmed-harness]` +- [ ] Record-as-parameter and record-nested-in-a-discovered-DTO shapes round-trip on the publish-trimmed client. `[trimmed-harness]` +- [ ] `[Factory]` types, `System.*`/primitive, and abstract/interface types still produce no preservation emission of either kind. `[unit]` +- [ ] Full solution build/test green on net9.0 + net10.0; CI trimming gate green. `[explicit-skip: build/test/CI gates]` +- [ ] `docs/trimming.md`, CLAUDE-DESIGN.md, and `AllPatterns.cs` comments describe the two-bucket emission as shipped. `[explicit-skip: doc delta, reviewed at code review]` + +--- + +## Current State (Pre-Flight) + +Walked 2026-07-06 on `TRIM` (post PR #68 merge, a566538): + +- Gate: `DtoTypeWalker.WalkFactoryReturn` rejects at `src/Generator/DtoTypeWalker.cs:145` (`!IsDtoStructureCandidate || !HasParameterlessCtor`); property descent only happens for accepted types. +- Dead code: `WalkEventRoot` (`DtoTypeWalker.cs:173-231`) has no callers; it already implements the bucket shape (root → parameterized bucket always; nested → bucket by ctor). File-header comment (`DtoTypeWalker.cs:3-4`) still claims a `FactoryGenerator.RelayHandler` caller — stale. +- Discovery: `MethodInfo.DiscoverDtoTypes` (`FactoryGenerator.Types.cs:741-772`) walks return type (`unwrapTask: true`) and non-service, non-CancellationToken parameters; collects into a flat `List` + shared visited set. +- Aggregation: per-type dedupe via `HashSet` at `FactoryGenerator.Types.cs:236-245` → `TypeInfo.DtoReturnTypes` (`EquatableArray`, lines 318/731). +- Model flow: `FactoryModelBuilder.cs:92/144/278` pass `typeInfo.DtoReturnTypes.ToList()` → `ClassFactoryModel.cs:48`, `InterfaceFactoryModel.cs:30`, `StaticFactoryModel.cs:32` (`IReadOnlyList`). +- Emission sites: `ClassFactoryRenderer.cs:1541`, `InterfaceFactoryRenderer.cs:480`, `StaticFactoryRenderer.cs:117` — identical `Register<{dtoType}>(() => new {dtoType}())` loops. +- Runtime: `DtoConstructorRegistry.PreserveType` exists (`DtoConstructorRegistry.cs:43`, DAM All); `RecordBypassConverterFactory.CanConvert` claims exactly the PreserveType bucket shape (no public parameterless ctor + ≥1 public parameterized ctor). +- Unit-test home: `RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/` — `NestedDtoDiscoveryTests` (regex helper `GetRegisteredDtoTypes` extracts `Register<...>` matches; needs a PreserveType twin), `NestedDtoFailureTest`. +- Harness: TRIM-004 contract — named bool checks aggregating into `failedChecks`, keyed `HttpClient` with `NoOpHttpHandler` registered, CI publishes linux-x64 and runs. New checks slot in before the summary block in `Program.cs`. +- Docs to correct: `docs/trimming.md` "What Qualifies as a DTO" (~line 261) documents the record exclusion as handled; CLAUDE-DESIGN.md FAQ row (~295) and "DTO Constructor Registry for Trimming" (~768); `Design.Domain/FactoryPatterns/AllPatterns.cs` `ExampleRecordResult` comments (~453-463) promise record returns work. +- Edge shapes to settle at the keyboard: `record struct` (structs report an implicit parameterless ctor → Register bucket; verify `new T()` renders), records with both parameterless and parameterized ctors (Register bucket — matches `RecordBypassConverterFactory.CanConvert` declining them), private-ctor-only types. + +--- + +## Test Evidence + +Filled after implementation, before the Step 5 gate. + +| Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | +|---|---|---|---| +| — | — | — | — | + +--- + +## Plan Amendments + +(None yet.) + +--- + +## Notes + +- TRIM-002 will reuse the bucket walk for `[Factory]` entity property descent — keep the classification a single shared code path, not per-call-site logic. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md index 055ead82..ab180972 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/004-trimming-harness-ci-gate.md @@ -3,8 +3,8 @@ **Plan #:** 004 **Date:** 2026-07-06 **Related Todo:** [../todo.md](../todo.md) -**Status:** In Progress -**Last Updated:** 2026-07-06 +**Status:** Done +**Last Updated:** 2026-07-06 (closed: PR #68 merged, CI trimming step green on linux-x64) **Plan-review opt-in:** No (test-infrastructure/CI wiring only; no public API, schema, or documented business-rule surface) **Code-review opt-in:** No (no library behavior change; harness and workflow only) @@ -54,7 +54,7 @@ Give the publish-trimmed harness enforceable pass/fail semantics and make CI run ## Acceptance - [x] A deliberately-injected smoke-check failure makes the published trimmed exe exit non-zero; the all-green run exits 0. `[explicit-skip: harness-gate semantics — verified by one-off failure injection at the keyboard; the harness itself is the test]` -- [ ] CI publishes and runs the trimmed harness on every push/PR build, and the job fails when the harness fails. `[explicit-skip: CI wiring — verified by this plan's own workflow run]` *(pending first workflow run — triggers on PR to main or workflow_dispatch; user controls push)* +- [x] CI publishes and runs the trimmed harness on every push/PR build, and the job fails when the harness fails. `[explicit-skip: CI wiring — verified by this plan's own workflow run]` *(verified: PR #68 run 28817388916 — trimming step published linux-x64, marker grep passed, harness "All checks passed")* - [x] Server-only marker absence in the published assembly is asserted by CI, not just documented in the README. `[explicit-skip: binary-inspection gate — workflow grep step]` *(grep logic verified locally against the win-x64 publish; CI asserts the linux-x64 artifact)* - [x] `dotnet build` and `dotnet test` of `Neatoo.RemoteFactory.sln` remain green (net9.0 + net10.0). `[explicit-skip: build gate]` @@ -80,7 +80,7 @@ Filled after implementation, before the Step 5 gate. All four Acceptance bullets | Acceptance bullet (short) | Tier declared | Test method / evidence | Tier confirmed | |---|---|---|---| | Injected failure → non-zero exit; all-green → 0 | `[explicit-skip]` | Keyboard verification 2026-07-06: injected `failedChecks.Add(...)` → `dotnet run` exit 1; removed → exit 0; trimmed publish all-green → exit 0 | ✓ | -| CI publishes and runs the trimmed harness | `[explicit-skip]` | `build.yml` "Trimming verification" step; **pending first workflow run** (PR to main or workflow_dispatch) | ✗ pending | +| CI publishes and runs the trimmed harness | `[explicit-skip]` | `build.yml` "Trimming verification" step; verified by PR #68 workflow run 28817388916 (linux-x64, all checks passed) | ✓ | | Marker absence asserted by CI | `[explicit-skip]` | `build.yml` grep step; logic verified locally against win-x64 publish (implementations absent, interface retention → TRIM-005) | ✓ | | Solution build/test green | `[explicit-skip]` | `reviews/004-build.log` (0 errors), `reviews/004-test.log` (2254 passed, 0 failed, net9.0+net10.0) | ✓ | diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 5d91feab..0a746939 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -40,7 +40,7 @@ A third suspected gap turned out to be already fixed: event records derive `Fact | # | Status | Plan | Source | |-----|--------|------|--------| -| 004 | Draft | [Trimming harness pass/fail semantics + CI gate](./plans/004-trimming-harness-ci-gate.md) | 2026-07-06 recon: TrimmingTests outside .sln/CI, exits 0 on failure — 001–003's trimmed acceptance signals need this gate first | +| 004 | Done | [Trimming harness pass/fail semantics + CI gate](./plans/004-trimming-harness-ci-gate.md) | 2026-07-06 recon: TrimmingTests outside .sln/CI, exits 0 on failure — 001–003's trimmed acceptance signals need this gate first | | 001 | Draft | [Positional-record preservation in factory signatures](./plans/001-positional-record-signature-preservation.md) | `DtoTypeWalker.WalkFactoryReturn` `HasParameterlessCtor` gate; zTreatment cut-over `StartVisitResultV2` hotfix | | 002 | Draft | [`[Factory]` entity property-graph DTO discovery](./plans/002-factory-entity-property-dto-discovery.md) | `WalkFactoryReturn` bails on `[Factory]` roots without descending; zTreatment `TreatmentBanner` / `DashboardContactResult` hotfixes | | 003 | Draft | [Verify event-record preservation needs no consumer entries](./plans/003-verify-event-record-preservation.md) | `FactoryEventBase` DAM annotation shipped v1.4.0; consumer entries predate it, never re-tested | From 3752405f78b516a6fb4d616e3b162f032fc5a4c4 Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Mon, 6 Jul 2026 14:41:15 -0500 Subject: [PATCH 2/5] docs(todo): TRIM-001 plan review (APPROVED, 5 callouts folded into draft) Co-Authored-By: Claude Fable 5 --- ...ositional-record-signature-preservation.md | 11 +++--- .../reviews/001-plan-review.md | 39 +++++++++++++++++++ 2 files changed, 45 insertions(+), 5 deletions(-) create mode 100644 docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-plan-review.md diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md index 79765413..cea0d228 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md @@ -12,7 +12,7 @@ ## Scope -Make the factory-signature DTO walk preserve positional records (types with only parameterized public ctors) instead of silently dropping them. Today `DtoTypeWalker.WalkFactoryReturn` requires `HasParameterlessCtor`, so a record like zTreatment's `StartVisitResultV2` — returned from a `[Remote, Execute]` command — gets no preservation and the trimmed client throws `DeserializeNoConstructor`. The fix shape already exists in the codebase's own history: bucket-sort discovered types the way the (now-dead) `WalkEventRoot` did — parameterless ctor → `DtoConstructorRegistry.Register(() => new T())`, parameterized/record → `DtoConstructorRegistry.PreserveType()` (deserialization then flows through the existing `RecordBypassConverterFactory`). Applies uniformly to return types, non-service parameters, and nested properties of discovered DTOs — with property descent into both buckets. Includes retiring the dead `WalkEventRoot` in favor of the refit shared walk, trimmed-harness repro checks (record-as-return, record-as-parameter, record-nested-in-DTO), and the docs/Design corrections for "What Qualifies as a DTO". Does NOT touch `[Factory]` entity property descent (TRIM-002), event preservation (TRIM-003), or the over-retention question (TRIM-005). +Make the factory-signature DTO walk preserve positional records (types with only parameterized public ctors) instead of silently dropping them. Today `DtoTypeWalker.WalkFactoryReturn` requires `HasParameterlessCtor`, so a record like zTreatment's `StartVisitResultV2` — returned from a `[Remote, Execute]` command — gets no preservation and the trimmed client throws `DeserializeNoConstructor`. The fix shape already exists in the codebase's own history: bucket-sort discovered types the way the (now-dead) `WalkEventRoot` did for *nested* types — parameterless ctor → `DtoConstructorRegistry.Register(() => new T())`, parameterized/record → `DtoConstructorRegistry.PreserveType()` (deserialization then flows through the existing `RecordBypassConverterFactory`). Roots bucket by ctor shape too — `WalkEventRoot`'s root-always-PreserveType rule is event-specific and must not be ported (plan review B2). Applies uniformly to return types, non-service parameters, and nested properties of discovered DTOs — with property descent into both buckets. Includes retiring the dead `WalkEventRoot` in favor of the refit shared walk, trimmed-harness repro checks (record-as-return, record-as-parameter, record-nested-in-DTO), and the docs/Design corrections for "What Qualifies as a DTO". Does NOT touch `[Factory]` entity property descent (TRIM-002), event preservation (TRIM-003), or the over-retention question (TRIM-005). --- @@ -27,8 +27,8 @@ Make the factory-signature DTO walk preserve positional records (types with only ## Framework & Architectural Alignment - Emission lands in the generated `FactoryServiceRegistrar` alongside the existing `Register` calls — same pattern, second bucket (`PreserveType` already exists at `DtoConstructorRegistry` with `[DynamicallyAccessedMembers(All)]` rooting; it is currently emitted nowhere). -- Deserialization path unchanged: parameterized-ctor types are claimed by `RecordBypassConverterFactory` (existing detection rule matches the bucket rule); parameterless DTOs keep flowing through `NeatooJsonTypeInfoResolver.CreateObject`. -- Roslyn incremental-generator discipline: model types stay value-equatable (`EquatableArray`) so pipeline caching doesn't regress. +- Deserialization path unchanged: parameterized-ctor types are claimed by `RecordBypassConverterFactory` (detection rule matches the bucket rule for reference types; `record struct` diverges benignly — Roslyn reports the synthesized parameterless ctor → Register bucket, reflection omits it → bypass converter claims it; both sides round-trip, see plan review B3); parameterless DTOs keep flowing through `NeatooJsonTypeInfoResolver.CreateObject`. +- Roslyn incremental-generator discipline: the pipeline cache boundary is the transform-output records (`TypeInfo` / `TypeFactoryMethodInfo` / `MethodInfo`) — the second bucket must be an `EquatableArray` there or incremental caching silently regresses with no failing test (plan review B1). The factory models run inside `RegisterSourceOutput` and may keep `IReadOnlyList`. - Unit-test pattern: `DiagnosticTestHelper.RunGenerator` + assertions over generated trees (the `NestedDtoDiscoveryTests` shape). - Trimmed-repro pattern: named bool checks in the `RemoteFactory.TrimmingTests` harness under the TRIM-004 exit-code contract. @@ -46,8 +46,8 @@ Make the factory-signature DTO walk preserve positional records (types with only ## Steps -1. Replace the walker's parameterless-ctor gate with bucket classification — parameterless → Register bucket, parameterized-public-ctor → PreserveType bucket — shared across return types, non-service parameters, and nested property descent (descending into both buckets' types). Retire the dead `WalkEventRoot` in favor of this shared walk and fix the stale file-header comment that still names the removed relay-handler caller. -2. Thread the ctor-shape distinction through the discovery pipeline: method-level discovery → per-type aggregation/dedupe → model builder → all three factory models, preserving value equality for incremental caching. +1. Replace the walker's parameterless-ctor gate with bucket classification — parameterless → Register bucket, parameterized-public-ctor → PreserveType bucket — applied by ctor shape at every level (roots and nested alike; `WalkEventRoot`'s root-always-PreserveType rule is event-specific and not ported), shared across return types, non-service parameters, and nested property descent (descending into both buckets' types). Retire the dead `WalkEventRoot` in favor of this shared walk and fix the stale file-header comment that still names the removed relay-handler caller. +2. Thread the ctor-shape distinction through the discovery pipeline: method-level discovery → per-type aggregation/dedupe → model builder → all three factory models, keeping the bucket `EquatableArray`-backed on the transform-output records (`TypeInfo`/`MethodInfo`/`TypeFactoryMethodInfo`) where incremental caching keys live. 3. Emit `PreserveType()` alongside `Register()` in all three registrar renderers (class, interface, static). 4. Extend the DtoDiscovery unit tests to pin bucket assignment: records as return type, as parameter, nested in a class DTO, class DTO nested in a record; existing exclusions still emit nothing. 5. Add trimmed-harness repro checks mirroring the consumer failure: a positional record returned from a `[Remote, Execute]` command, taken as a parameter, and nested as a property of a discovered DTO — each serializer round-tripped on the trimmed client (the `EventRelaySmokeTest` shape). @@ -105,3 +105,4 @@ Filled after implementation, before the Step 5 gate. ## Notes - TRIM-002 will reuse the bucket walk for `[Factory]` entity property descent — keep the classification a single shared code path, not per-call-site logic. +- Plan review (2026-07-06, APPROVED — `../reviews/001-plan-review.md`) carry-alongs: anchor the unit-test PreserveType regex to `PreserveType<(.+?)>\(\)` and confirm TS-010/TS-014 count-assertions stay Register-only (B4); expect new additive `PreserveType<>` lines in existing record-target registrars (`InterfaceFactoryRecordTargets`) once descent enters record graphs (B5); Step 6 doc edits must leave no sentence implying `PreserveType` is emitted nowhere — the event-path removal statement at `docs/trimming.md:300` stays accurate, the factory-signature path is distinct (A1). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-plan-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-plan-review.md new file mode 100644 index 00000000..e830d370 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-plan-review.md @@ -0,0 +1,39 @@ +# TRIM-001 Plan Review — 2026-07-06 + +**Reviewer:** plan-reviewer agent (two-pass: A = documented requirements, B = codebase) +**Verdict: APPROVED** — no veto-tier findings in either pass. Five callout-tier findings, all folded into the draft before implementation (see "Disposition"). + +--- + +## Pass A — vs. documented requirements + +Docs consulted: `src/Design/CLAUDE-DESIGN.md` (FAQ ~295; DTO-registry section ~768-788), `src/Design/Design.Domain/FactoryPatterns/AllPatterns.cs` (~452-463), `docs/trimming.md` (243-306). + +- **No veto findings.** The only documented-behavior change (record exclusion wording at `docs/trimming.md:267`, `CLAUDE-DESIGN.md:780`) is exactly the delta the parent todo's Goal and Acceptance Criterion 1 sanction, and Step 6 targets those sites. +- `CLAUDE-DESIGN.md:772` ("not in DI and not in the registry → `CreateObject` not set") stays accurate: `PreserveType()` deliberately does not populate `TryCreate`; records are claimed by `RecordBypassConverterFactory` first. +- Callout A1: after this plan, `PreserveType` is emitted again (by the factory-signature path) — Step 6 doc edits must leave no sentence implying it is emitted nowhere. `docs/trimming.md:300` (event pipeline removal) itself stays accurate — distinct path. +- Callout A2: the stale `FactoryEventHandlerPattern.cs` comments describing the *removed event-path* emission are already routed to TRIM-003 — keep the two doc deltas from colliding. + +## Pass B — vs. codebase + +Reality check passed: the gate (`DtoTypeWalker.cs:145`), dead `WalkEventRoot` (173-231, stale header 3-4), unemitted `PreserveType` (`DtoConstructorRegistry.cs:43`), and the three-renderer seam enumeration were all confirmed complete (no other `DtoReturnTypes` consumer exists). + +- **B1 (key):** The incremental-cache boundary is the transform output `TypeInfo` (`FactoryGenerator.cs:19-31/54-64`) — `FactoryModelBuilder.Build` runs inside `RegisterSourceOutput`, so the three factory *models* are not cache keys. Thread the second bucket as `EquatableArray` on `TypeInfo` (`Types.cs:71`) and `MethodInfo`/`TypeFactoryMethodInfo` (646/521); a plain `List`/`IReadOnlyList` field there would silently break incremental caching with **no failing test**. +- **B2 (key):** Do not copy `WalkEventRoot`'s root semantics — it forces the root into the PreserveType bucket regardless of ctor shape (`DtoTypeWalker.cs:196-197`, correct for event roots only). The factory-return root must bucket **by ctor shape**; a literal port would degrade parameterless class DTO returns to the reflection path under trimming. Only the *nested* bucket-sort (220-227) is the reusable shape. +- **B3:** `record struct` is the one shape where bucket rule and runtime detection diverge: Roslyn reports the synthesized parameterless ctor (→ Register bucket) but reflection `GetConstructors()` omits it (→ `RecordBypassConverterFactory` claims it). Benign (Register also carries DAM-All; bypass round-trips structs), but the "detection rule matches the bucket rule" parity claim isn't exact. +- **B4:** `NestedDtoDiscoveryTests.GetRegisteredDtoTypes` regex (`:24-26`) counts `Register<>` only; TS-010 (`:416`) and TS-014 (`:526`) count-assertions stay Register-only and remain green (fixtures are parameterless class DTOs). Anchor the PreserveType twin to `PreserveType<(.+?)>\(\)`. +- **B5:** Removing the ctor gate means property descent now enters record graphs — existing untrimmed targets (`InterfaceFactoryRecordTargets.cs`) will gain new `PreserveType<>` lines in their registrars. Additive, idempotent, intended; no test asserts their absence. + +Infrastructure sweep: no snapshot/golden tests on generated text; `CombinationTestGenerator` unaffected; `init`-only properties are walked (`GetMethod != null`, `DtoTypeWalker.cs:248`); record `EqualityContract` filtered by the `Public` check (245); FQN rendering path identical to today's `Register<>` (nullable modifiers already stripped). + +## Recommendations → Disposition + +| # | Finding | Disposition | +|---|---------|-------------| +| B1 | Equatability lives on `TypeInfo`/`MethodInfo`, not models | Plan Framework Alignment + Step 2 corrected | +| B2 | Root buckets by ctor shape; only nested walk reusable | Plan Scope + Step 1 corrected | +| B3 | `record struct` bucket/detection divergence | Parity claim softened in Framework Alignment; keyboard note kept | +| B4 | Regex anchoring; TS-010/TS-014 stay Register-only | Added to plan Notes | +| A1 | No doc sentence may imply PreserveType unemitted | Added to plan Notes (Step 6 checklist) | + +Calibration note per workflow: diagnoses adopted; prescriptions treated as advisory (all five were adopted as-is — they matched the code walk). From bed0651bacc8dd946e672cfc730a86393377eb62 Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Mon, 6 Jul 2026 14:57:05 -0500 Subject: [PATCH 3/5] feat(generator): preserve positional-record DTOs in factory signatures (TRIM-001) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DtoTypeWalker's factory walk required a public parameterless ctor, so positional records in factory signatures got no trimming preservation — a publish-trimmed client threw DeserializeNoConstructor on them (the zTreatment StartVisitResultV2 cut-over failure). Discovered types now bucket-sort by ctor shape at every level of the walk: parameterless -> DtoConstructorRegistry.Register(() => new T()), parameterized-only -> DtoConstructorRegistry.PreserveType() (rooting only; deserialization flows through RecordBypassConverterFactory). The bucket threads through the incremental pipeline as EquatableArray on the transform-output records; all three registrar renderers emit both calls. The dead WalkEventRoot walker is retired into the shared bucketed walk. - Unit: RecordDtoDiscoveryTests pins bucket assignment across return, parameter, nested (both directions), collection, mixed-ctor, interface/ class/static renderer paths, and the unchanged exclusions - Trimmed harness: RecordDtoSmokeTest deserializes records never constructed in client code (negative control verified: disabling the emission fails the harness with exit 1) - Docs: trimming.md, CLAUDE-DESIGN.md, AllPatterns.cs updated to the two-bucket contract Co-Authored-By: Claude Fable 5 --- ...ositional-record-signature-preservation.md | 23 +- .../todo.md | 5 + docs/trimming.md | 17 +- src/Design/CLAUDE-DESIGN.md | 15 +- .../FactoryPatterns/AllPatterns.cs | 5 + src/Generator/Builder/FactoryModelBuilder.cs | 9 +- src/Generator/DtoTypeWalker.cs | 112 +++--- src/Generator/FactoryGenerator.Types.cs | 53 ++- src/Generator/Model/ClassFactoryModel.cs | 10 +- src/Generator/Model/InterfaceFactoryModel.cs | 10 +- src/Generator/Model/StaticFactoryModel.cs | 10 +- .../Renderer/ClassFactoryRenderer.cs | 7 +- .../Renderer/InterfaceFactoryRenderer.cs | 7 +- .../Renderer/StaticFactoryRenderer.cs | 7 +- .../RemoteFactory.TrimmingTests/Program.cs | 7 + .../RecordDtoSmokeTest.cs | 80 +++++ .../TrimTestCommands.cs | 23 ++ .../DtoDiscovery/RecordDtoDiscoveryTests.cs | 320 ++++++++++++++++++ 18 files changed, 600 insertions(+), 120 deletions(-) create mode 100644 src/Tests/RemoteFactory.TrimmingTests/RecordDtoSmokeTest.cs create mode 100644 src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md index cea0d228..3215878b 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md @@ -3,7 +3,7 @@ **Plan #:** 001 **Date:** 2026-07-06 **Related Todo:** [../todo.md](../todo.md) -**Status:** Draft +**Status:** In Progress **Last Updated:** 2026-07-06 **Plan-review opt-in:** Yes (changes the generator's emission contract for every consumer's registrar; touches documented behavior in docs/trimming.md and CLAUDE-DESIGN.md; incremental-pipeline equality semantics involved) **Code-review opt-in:** Yes (behavior-changing generator work) @@ -59,12 +59,12 @@ Make the factory-signature DTO walk preserve positional records (types with only Tier note: `[trimmed-harness]` is a project-local tier — a named check in `RemoteFactory.TrimmingTests` executed under `PublishTrimmed=true` by the CI trimming gate (TRIM-004). -- [ ] The generator emits `PreserveType()` for a positional record appearing as a factory-method return type, as a non-service parameter, and as a property of a discovered DTO; `Register()` emission for parameterless DTOs is unchanged. `[unit]` -- [ ] A positional record returned from a `[Remote, Execute]` command round-trips through the Neatoo serializer on a publish-trimmed client (the zTreatment `StartVisitResultV2` shape). `[trimmed-harness]` -- [ ] Record-as-parameter and record-nested-in-a-discovered-DTO shapes round-trip on the publish-trimmed client. `[trimmed-harness]` -- [ ] `[Factory]` types, `System.*`/primitive, and abstract/interface types still produce no preservation emission of either kind. `[unit]` -- [ ] Full solution build/test green on net9.0 + net10.0; CI trimming gate green. `[explicit-skip: build/test/CI gates]` -- [ ] `docs/trimming.md`, CLAUDE-DESIGN.md, and `AllPatterns.cs` comments describe the two-bucket emission as shipped. `[explicit-skip: doc delta, reviewed at code review]` +- [x] The generator emits `PreserveType()` for a positional record appearing as a factory-method return type, as a non-service parameter, and as a property of a discovered DTO; `Register()` emission for parameterless DTOs is unchanged. `[unit]` +- [x] A positional record returned from a `[Remote, Execute]` command round-trips through the Neatoo serializer on a publish-trimmed client (the zTreatment `StartVisitResultV2` shape). `[trimmed-harness]` +- [x] Record-as-parameter and record-nested-in-a-discovered-DTO shapes round-trip on the publish-trimmed client. `[trimmed-harness]` +- [x] `[Factory]` types, `System.*`/primitive, and abstract/interface types still produce no preservation emission of either kind. `[unit]` +- [x] Full solution build/test green on net9.0 + net10.0; CI trimming gate green. `[explicit-skip: build/test/CI gates]` *(local logs green — one unrelated flaky relay-timing test failed under parallel load and passed isolated; CI gate verifies on the PR)* +- [x] `docs/trimming.md`, CLAUDE-DESIGN.md, and `AllPatterns.cs` comments describe the two-bucket emission as shipped. `[explicit-skip: doc delta, reviewed at code review]` --- @@ -88,11 +88,16 @@ Walked 2026-07-06 on `TRIM` (post PR #68 merge, a566538): ## Test Evidence -Filled after implementation, before the Step 5 gate. +Filled 2026-07-06, before the Step 5 gate. All test classes are in `RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests` unless noted. | Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | |---|---|---|---| -| — | — | — | — | +| PreserveType emitted for return / parameter / nested; Register unchanged | `[unit]` | `PositionalRecordAsReturnType_PreserveTypeEmitted`, `PositionalRecordAsParameter_PreserveTypeEmitted`, `PositionalRecordNestedInClassDto_BothBucketsEmitted`, `ClassDtoNestedInPositionalRecord_DescentEntersRecordGraph`, `CollectionOfRecords_UnwrappedAndPreserved`, `RecordWithBothCtorShapes_StaysInRegisterBucket`, `PositionalRecordFromInterfaceFactory_PreserveTypeEmitted` (all three renderer paths: static, class, interface) | ✓ | +| Record-as-return round-trips on publish-trimmed client | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` return shape (`TrimRecordResult`) — trimmed run exit 0; **negative control**: PreserveType emission disabled → harness FAILED "record DTO preservation", exit 1 | ✓ | +| Record-as-parameter and nested-record shapes round-trip trimmed | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` parameter shape (`TrimRecordCommand`) + nested (`TrimRecordDetail` property) | ✓ | +| Exclusions intact (no emission of either kind) | `[unit]` | `FactoryAnnotatedType_NoEmissionOfEitherKind`, `PrivateCtorOnlyType_NoEmissionOfEitherKind`; existing `NestedDtoDiscoveryTests` suite stays green | ✓ | +| Build/test/CI gates | `[explicit-skip]` | `reviews/001-build.log` (0 errors, 2 pre-existing warnings), `reviews/001-test.log` (full suite), `reviews/001-test-relay-rerun.log` (unrelated flaky `RelayTimingTests` re-run green in isolation) | ✓ | +| Docs describe two-bucket emission | `[explicit-skip]` | `docs/trimming.md`, `CLAUDE-DESIGN.md` (registry section + criteria table + FAQ row), `AllPatterns.cs` `ExampleRecordResult` remarks | ✓ | --- diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 0a746939..e33e4aa7 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -71,6 +71,11 @@ Execution order: 004 → 001 → 002 → 003 → 005 (rows listed in execution o - **Decision:** Amend. - **Follow-up:** n/a. +### 2026-07-06 — TRIM-001 (unrelated flaky test observed at gate) +- **Finding:** `RelayTimingTests.Relay_FiresAfterCallerSynchronousWriteOnContinuation` (integration, event relay) failed with `TimeoutException` on net9.0 under full-suite parallel load, passed in isolation and on the next full run. Unrelated to TRIM's generator changes — timing-sensitive test. +- **Decision:** Defer. +- **Follow-up:** flagged to user — out-of-goal tech debt; queue as sibling todo or accept as known flake (not queued in TRIM). + ### 2026-07-06 — TRIM-004 (server-only over-retention) - **Finding:** A trimmed client retains the `IServerOnlyRepository` TypeDef and `DoServerWork` member ref: generated `LocalCreate` bodies are rooted by delegate registration and their early-`throw` guard + `try/catch` defeats ILLink unreachable-code elimination. Implementations are correctly trimmed. Contradicts `docs/trimming.md` "should return no matches" / "dead code is removed" claims. TRIM-004's CI grep narrowed to implementation types (Plan Amendment 3). - **Decision:** Defer. diff --git a/docs/trimming.md b/docs/trimming.md index 66e82223..a320f4c1 100644 --- a/docs/trimming.md +++ b/docs/trimming.md @@ -248,7 +248,12 @@ Normal Blazor WASM apps don't hit this because their assemblies aren't trimmed ### How RemoteFactory Handles It -The source generator discovers plain DTO return types from factory method signatures at compile time and emits `DtoConstructorRegistry.Register(() => new T())` calls. The `[DynamicallyAccessedMembers(All)]` annotation on the generic parameter tells the trimmer to preserve the entire type — constructors, properties, and all metadata that `System.Text.Json` needs for deserialization. +The source generator discovers DTO types in factory method signatures at compile time — return types **and** non-service parameters — and emits one of two preservation calls per discovered type, chosen by constructor shape: + +- **Public parameterless constructor** → `DtoConstructorRegistry.Register(() => new T())`. The registered lambda replaces reflection-based construction at deserialization time. +- **Only parameterized public constructors** (positional records) → `DtoConstructorRegistry.PreserveType()`. No constructor lambda is registered; deserialization flows through `RecordBypassConverterFactory`, and the call exists purely to root the type for the trimmer. + +Both calls carry `[DynamicallyAccessedMembers(All)]` on the generic parameter, which tells the trimmer to preserve the entire type — constructors, properties, and all metadata that `System.Text.Json` needs for deserialization. This covers all factory patterns: @@ -260,18 +265,18 @@ The generator unwraps `Task`, nullable `T?`, and collection types (like `IRea ### What Qualifies as a DTO -Not every return type needs this treatment. The generator preserves a return type when it: +Not every signature type needs this treatment. The generator preserves a discovered type when it: -- Has a public parameterless constructor +- Has at least one public constructor (parameterless → `Register`; parameterized-only, e.g. positional records → `PreserveType`) - Is **not** a `[Factory]`-annotated type (those are already preserved via DI registration) -- Is **not** a record with only parameterized constructors (handled separately by `RecordBypassConverterFactory`) - Is **not** a primitive, string, or framework type +- Is **not** abstract or an interface ### What You Need to Know -If you return a plain DTO class through any factory method, it is automatically trimming-safe. You do not need to take any action. +If you return or accept a plain DTO class **or a positional record** through any factory method, it is automatically trimming-safe. You do not need to take any action. -**Nested DTOs are automatically discovered.** The generator recursively walks public instance properties (including inherited properties) of each discovered DTO type to find nested DTOs that also need registration. Collection properties (`List`, `IReadOnlyList`, arrays) and nullable properties (`T?`) are unwrapped to find the inner type. The same eligibility criteria apply to nested DTOs as to direct return types. Cycle detection prevents infinite recursion from circular references. +**Nested DTOs are automatically discovered.** The generator recursively walks public instance properties (including inherited properties) of each discovered DTO type — classes and records alike — to find nested DTOs that also need preservation. Collection properties (`List`, `IReadOnlyList`, arrays) and nullable properties (`T?`) are unwrapped to find the inner type. The same eligibility criteria and bucket rule apply to nested DTOs as to direct signature types. Cycle detection prevents infinite recursion from circular references. For example, if a factory method returns `ParentDto` which has a `List Children` property, both `ParentDto` and `ChildDto` are automatically registered — no additional action is needed. diff --git a/src/Design/CLAUDE-DESIGN.md b/src/Design/CLAUDE-DESIGN.md index 699a0dc2..670387e5 100644 --- a/src/Design/CLAUDE-DESIGN.md +++ b/src/Design/CLAUDE-DESIGN.md @@ -292,7 +292,7 @@ services.AddNeatooRemoteFactory(NeatooFactory.Remote, typeof(Order).Assembly); | Can I handle multiple event types in one class? | Yes, stack multiple `[FactoryEventHandler]` attributes | `PersonEventHandler.cs` (Person example) | Generator finds one matching method per attribute | | How do I defer loading of related data? | Use `LazyLoad` property with constructor-initialization pattern | `LazyLoadExample.cs` | Value is passive (no auto-load); call LoadAsync() explicitly; two-slot ordinal encoding | | Can I use BCL `Lazy`? | No -- use `LazyLoad` instead | `SerializationTests.cs` | BCL `Lazy` has no serialization support; `LazyLoad` serializes Value + IsLoaded | -| Do I need to register DTOs for IL trimming? | No -- the generator auto-registers DTO return types from factory methods | `DtoConstructorRegistry.cs` | Generator emits `() => new Dto()` lambdas; `NeatooJsonTypeInfoResolver` uses them instead of `Activator.CreateInstance` | +| Do I need to register DTOs for IL trimming? | No -- the generator auto-preserves DTO types from factory signatures, records included | `DtoConstructorRegistry.cs` | Parameterless ctor → `Register(() => new T())` lambda used by `NeatooJsonTypeInfoResolver`; positional records → `PreserveType()` rooting, deserialized via `RecordBypassConverterFactory` | | What if my nested DTO fails to deserialize under trimming? | Check that it is reachable as a public property of a discovered DTO; if not, return it from a factory method or register manually | `docs/trimming.md` | The generator recursively walks properties of discovered DTOs; only unreachable types need manual registration | | Can auth methods receive factory method parameters? | Yes -- parameters are matched by type | `ParamAuthOrder.cs`, `ParamAuthOrderAuth.cs` | Auth method `CanFetch(Guid orderId)` receives the Guid from `Fetch(Guid orderId)` for per-entity access control | | Can auth methods receive the target entity? | Yes -- on write operations (Insert/Update/Delete) | `ParamAuthOrder.cs`, `ParamAuthOrderAuth.cs` | Auth method `CanWrite(IEntity target)` inspects entity state; suppresses CanInsert/CanUpdate/CanDelete generation but CanSave gets two overloads | @@ -767,25 +767,26 @@ This mechanism is internal to the generator and library. Users do not need to em #### DTO Constructor Registry for Trimming -The generator emits `DtoConstructorRegistry.Register(() => new Dto())` calls in `FactoryServiceRegistrar` for plain DTO return types discovered in factory method signatures. This creates static constructor references that survive IL trimming — without them, `System.Text.Json` deserialization fails because `DefaultJsonTypeInfoResolver` uses reflection to discover constructors, and the trimmer strips that metadata from types in assemblies marked `IsTrimmable=true`. +The generator emits preservation calls in `FactoryServiceRegistrar` for DTO types discovered in factory method signatures (return types and non-service parameters), bucket-sorted by constructor shape: `DtoConstructorRegistry.Register(() => new Dto())` for types with a public parameterless constructor, `DtoConstructorRegistry.PreserveType()` for types with only parameterized public constructors (positional records). Both create static references that survive IL trimming — without them, `System.Text.Json` deserialization fails because `DefaultJsonTypeInfoResolver` uses reflection to discover constructors, and the trimmer strips that metadata from types in assemblies marked `IsTrimmable=true`. At runtime, `NeatooJsonTypeInfoResolver` uses the registered lambda instead of `Activator.CreateInstance` (which also fails under trimming). If a type is not in DI and not in the DTO registry, `CreateObject` is not set — STJ uses its default behavior, which produces a clear error if the constructor was trimmed. -**DTO discovery criteria** — the generator registers a return type when it: +**DTO discovery criteria** — the generator preserves a discovered signature type when it: | Criterion | Why | |-----------|-----| -| Has a public parameterless constructor | Required for `() => new Dto()` lambda | +| Has at least one public constructor | Parameterless → `Register(() => new T())`; parameterized-only (positional records) → `PreserveType()`, deserialized via `RecordBypassConverterFactory` | | Is NOT a `[Factory]`-annotated type | Already DI-registered; uses `GetRequiredService` path | -| Is NOT a record (no parameterless ctor + has parameterized ctors) | Handled by `RecordBypassConverterFactory` | | Is NOT a primitive, string, or framework type | STJ handles these natively | | Is NOT abstract or an interface | Cannot be instantiated | -The generator unwraps `Task`, nullable `T?`, and generic collection types (`IReadOnlyList`, `List`, etc.) to discover the inner DTO type. The `Register` method carries `[DynamicallyAccessedMembers(All)]` on the type parameter, which instructs the trimmer to preserve the entire type — constructors, properties, and all metadata. +(`record struct` edge: Roslyn reports the synthesized parameterless ctor, so value-type records land in the `Register` bucket; at runtime `RecordBypassConverterFactory` still claims them because reflection omits the implicit struct ctor. Both mechanisms preserve and round-trip them — the divergence is benign.) + +The generator unwraps `Task`, nullable `T?`, and generic collection types (`IReadOnlyList`, `List`, etc.) to discover the inner DTO type. Both `Register` and `PreserveType` carry `[DynamicallyAccessedMembers(All)]` on the type parameter, which instructs the trimmer to preserve the entire type — constructors, properties, and all metadata. `PreserveType` deliberately does not populate the constructor registry — parameterized-ctor types never take the `CreateObject` path. Duplicate registrations from multiple factories returning the same DTO type are idempotent (`ConcurrentDictionary.TryAdd`). -**Nested DTO discovery:** The generator recursively walks public instance properties (including inherited properties via base type chain) of each discovered DTO to find nested DTOs that also need registration. Collection properties (`List`, `IReadOnlyList`, arrays) and nullable properties (`T?`) are unwrapped to find the inner DTO type. The same eligibility criteria apply to nested DTOs as to direct return types. Cycle detection prevents infinite recursion from circular references (e.g., `DtoA` -> `DtoB` -> `DtoA`). +**Nested DTO discovery:** The generator recursively walks public instance properties (including inherited properties via base type chain) of each discovered DTO — classes and records alike — to find nested DTOs that also need preservation. Collection properties (`List`, `IReadOnlyList`, arrays) and nullable properties (`T?`) are unwrapped to find the inner DTO type. The same eligibility criteria and bucket rule apply to nested DTOs as to direct signature types. Cycle detection prevents infinite recursion from circular references (e.g., `DtoA` -> `DtoB` -> `DtoA`). **Factory event type preservation.** `FactoryEventBase` itself carries `[FactoryEvent]` and `[DynamicallyAccessedMembers(PublicConstructors | PublicProperties)]`, both with `Inherited = true`. Every descendant is therefore automatically discoverable by the runtime `FactoryEventTypeRegistry` and has its constructors and properties preserved through IL trimming with **no generator emission and no per-event annotation**. Inheriting `FactoryEventBase` is sufficient; consumers never apply `[FactoryEvent]` directly. diff --git a/src/Design/Design.Domain/FactoryPatterns/AllPatterns.cs b/src/Design/Design.Domain/FactoryPatterns/AllPatterns.cs index 2d74b189..865dd02f 100644 --- a/src/Design/Design.Domain/FactoryPatterns/AllPatterns.cs +++ b/src/Design/Design.Domain/FactoryPatterns/AllPatterns.cs @@ -457,6 +457,11 @@ public class ExampleDto /// without $id/$ref metadata, so parameterized constructors are not blocked /// by System.Text.Json's reference handling limitation. /// +/// IL trimming: because this record has no parameterless constructor, the +/// generator emits DtoConstructorRegistry.PreserveType<ExampleRecordResult>() +/// in the FactoryServiceRegistrar (plain DTOs with a parameterless ctor get +/// Register<T>(() => new T()) instead). No consumer action needed. +/// /// Do NOT add [Factory] here -- that would route serialization through the /// ordinal converter. Plain records fall through to standard STJ handling. /// diff --git a/src/Generator/Builder/FactoryModelBuilder.cs b/src/Generator/Builder/FactoryModelBuilder.cs index 7091fa7a..96a0cfd2 100644 --- a/src/Generator/Builder/FactoryModelBuilder.cs +++ b/src/Generator/Builder/FactoryModelBuilder.cs @@ -89,7 +89,8 @@ private static FactoryGenerationUnit BuildStaticFactory(TypeInfo typeInfo) signatureText: typeInfo.SignatureText, isPartial: typeInfo.IsPartial, delegates: delegates, - dtoReturnTypes: typeInfo.DtoReturnTypes.ToList()); + dtoReturnTypes: typeInfo.DtoReturnTypes.ToList(), + dtoPreserveTypes: typeInfo.DtoPreserveTypes.ToList()); return new FactoryGenerationUnit( @namespace: typeInfo.Namespace, @@ -141,7 +142,8 @@ private static FactoryGenerationUnit BuildInterfaceFactory(TypeInfo typeInfo) serviceTypeName: typeInfo.ServiceTypeName, implementationTypeName: typeInfo.ImplementationTypeName, methods: methods, - dtoReturnTypes: typeInfo.DtoReturnTypes.ToList()); + dtoReturnTypes: typeInfo.DtoReturnTypes.ToList(), + dtoPreserveTypes: typeInfo.DtoPreserveTypes.ToList()); return new FactoryGenerationUnit( @namespace: typeInfo.Namespace, @@ -275,7 +277,8 @@ private static FactoryGenerationUnit BuildClassFactory(TypeInfo typeInfo) hasDefaultSave: hasDefaultSave, requiresEntityRegistration: requiresEntityRegistration, registerOrdinalConverter: registerOrdinalConverter, - dtoReturnTypes: typeInfo.DtoReturnTypes.ToList()); + dtoReturnTypes: typeInfo.DtoReturnTypes.ToList(), + dtoPreserveTypes: typeInfo.DtoPreserveTypes.ToList()); return new FactoryGenerationUnit( @namespace: typeInfo.Namespace, diff --git a/src/Generator/DtoTypeWalker.cs b/src/Generator/DtoTypeWalker.cs index 90353d53..441dcd49 100644 --- a/src/Generator/DtoTypeWalker.cs +++ b/src/Generator/DtoTypeWalker.cs @@ -1,7 +1,9 @@ // DtoTypeWalker.cs // Shared walker for discovering DTO types reachable from a root symbol. -// Used by both the factory-return path (MethodInfo.DiscoverDtoTypes) and the -// [FactoryEventHandler] event-type preservation path (FactoryGenerator.RelayHandler). +// Used by the factory-signature path (MethodInfo.DiscoverDtoTypes) for both +// return types and non-service parameters. Discovered types bucket-sort by +// constructor shape: parameterless -> DtoConstructorRegistry.Register(), +// parameterized-only -> DtoConstructorRegistry.PreserveType(). using System.Collections.Generic; using System.Linq; @@ -130,104 +132,66 @@ public static bool HasParameterlessCtor(INamedTypeSymbol namedType) } /// - /// Factory-return walker: recursively discovers DTO types reachable from the given root. - /// Only accepts types that pass BOTH IsDtoStructureCandidate and HasParameterlessCtor. - /// Walks public instance properties (including inherited) to find nested DTOs. - /// Uses visited for cycle suppression; appends FQNs to dtoTypes. + /// Whether the named type has at least one public constructor with parameters. /// - public static void WalkFactoryReturn(ITypeSymbol typeSymbol, List dtoTypes, HashSet visited) + public static bool HasParameterizedPublicCtor(INamedTypeSymbol namedType) + { + return namedType.Constructors.Any(c => + c.DeclaredAccessibility == Accessibility.Public && c.Parameters.Length > 0); + } + + /// + /// Factory-signature walker: recursively discovers DTO types reachable from the + /// given root and bucket-sorts every discovered type (roots and nested alike) by + /// constructor shape: + /// - public parameterless ctor → registerTypes (Register<T>(() => new T())) + /// - only parameterized public ctors (positional records) → preserveTypes + /// (PreserveType<T>(); deserialization flows through RecordBypassConverterFactory) + /// - no public ctor at all → skipped (not deserializable) + /// Walks public instance properties (including inherited) of both buckets' types + /// to find nested DTOs. Both buckets share the visited set for cycle suppression. + /// + public static void WalkDtoGraph( + ITypeSymbol typeSymbol, + List registerTypes, + List preserveTypes, + HashSet visited) { if (!(typeSymbol is INamedTypeSymbol namedType)) { return; } - if (!IsDtoStructureCandidate(namedType) || !HasParameterlessCtor(namedType)) + if (!IsDtoStructureCandidate(namedType)) { return; } - var fullyQualifiedName = namedType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - - if (!visited.Add(fullyQualifiedName)) + var hasParameterless = HasParameterlessCtor(namedType); + if (!hasParameterless && !HasParameterizedPublicCtor(namedType)) { return; } - dtoTypes.Add(fullyQualifiedName); - - WalkProperties(namedType, WalkFactoryReturnNested); - - void WalkFactoryReturnNested(ITypeSymbol nested) => WalkFactoryReturn(nested, dtoTypes, visited); - } + var fullyQualifiedName = namedType.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - /// - /// Event-root walker: the root type itself is ALWAYS added to parameterizedTypes - /// (PreserveType<T>() bucket), regardless of whether it has a parameterless ctor — - /// event records deserialize through RecordBypassConverterFactory. - /// Nested properties bucket-sort by HasParameterlessCtor: - /// - parameterless → parameterlessCtorTypes (Register<N>(() => new N())) - /// - parameterized → parameterizedTypes (PreserveType<N>()) - /// Both buckets share the visited set for dedupe. - /// - public static void WalkEventRoot( - ITypeSymbol eventRoot, - List parameterlessCtorTypes, - List parameterizedTypes, - HashSet visited) - { - if (!(eventRoot is INamedTypeSymbol namedRoot)) + if (!visited.Add(fullyQualifiedName)) { return; } - if (!IsDtoStructureCandidate(namedRoot)) + if (hasParameterless) { - return; + registerTypes.Add(fullyQualifiedName); } - - var rootFqn = namedRoot.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - - if (!visited.Add(rootFqn)) + else { - return; + preserveTypes.Add(fullyQualifiedName); } - // The root event type always goes to the PreserveType bucket, regardless of ctor shape. - parameterizedTypes.Add(rootFqn); - - WalkProperties(namedRoot, WalkNested); - - void WalkNested(ITypeSymbol nested) - { - if (!(nested is INamedTypeSymbol namedNested)) - { - return; - } - - if (!IsDtoStructureCandidate(namedNested)) - { - return; - } - - var fqn = namedNested.ToDisplayString(SymbolDisplayFormat.FullyQualifiedFormat); - - if (!visited.Add(fqn)) - { - return; - } - - if (HasParameterlessCtor(namedNested)) - { - parameterlessCtorTypes.Add(fqn); - } - else - { - parameterizedTypes.Add(fqn); - } + WalkProperties(namedType, WalkNested); - WalkProperties(namedNested, WalkNested); - } + void WalkNested(ITypeSymbol nested) => WalkDtoGraph(nested, registerTypes, preserveTypes, visited); } /// diff --git a/src/Generator/FactoryGenerator.Types.cs b/src/Generator/FactoryGenerator.Types.cs index 8ce9ae2d..dc81bf6f 100644 --- a/src/Generator/FactoryGenerator.Types.cs +++ b/src/Generator/FactoryGenerator.Types.cs @@ -233,16 +233,23 @@ public TypeInfo(TypeDeclarationSyntax syntax, INamedTypeSymbol symbol, SemanticM this.FactoryMethods = new EquatableArray([.. factoryMethodsList]); - // Aggregate DTO return types from all factory methods (deduplicated) + // Aggregate DTO types from all factory methods (deduplicated, per bucket) var allDtoTypes = new HashSet(); + var allPreserveTypes = new HashSet(); foreach (var method in factoryMethodsList) { foreach (var dtoType in method.DtoReturnTypes) { allDtoTypes.Add(dtoType); } + + foreach (var preserveType in method.DtoPreserveTypes) + { + allPreserveTypes.Add(preserveType); + } } this.DtoReturnTypes = new EquatableArray([.. allDtoTypes]); + this.DtoPreserveTypes = new EquatableArray([.. allPreserveTypes]); // Collect properties for ordinal serialization (only for non-interface, non-static types) if (!this.IsInterface && !this.IsStatic) @@ -312,11 +319,19 @@ private static TypeFactoryMethodInfo CreatePrimaryConstructorFactoryMethod( public EquatableArray AuthMethods { get; set; } = []; /// - /// Deduplicated fully-qualified names of plain DTO types discovered across all factory methods. - /// Used by renderers to emit DtoConstructorRegistry.Register calls for IL trimming support. + /// Deduplicated fully-qualified names of plain DTO types (public parameterless ctor) + /// discovered across all factory methods. Used by renderers to emit + /// DtoConstructorRegistry.Register calls for IL trimming support. /// public EquatableArray DtoReturnTypes { get; } = []; + /// + /// Deduplicated fully-qualified names of DTO types without a public parameterless ctor + /// (positional records) discovered across all factory methods. Used by renderers to emit + /// DtoConstructorRegistry.PreserveType calls for IL trimming support. + /// + public EquatableArray DtoPreserveTypes { get; } = []; + /// /// Indicates if this type is nested inside another type. /// Nested types require special handling for code generation. @@ -679,7 +694,7 @@ protected MethodInfo(IMethodSymbol methodSymbol, BaseMethodDeclarationSyntax met } // Discover plain DTO types for constructor registration (IL trimming support) - this.DtoReturnTypes = DiscoverDtoTypes(methodSymbol); + (this.DtoReturnTypes, this.DtoPreserveTypes) = DiscoverDtoTypes(methodSymbol); } /// @@ -709,7 +724,7 @@ protected MethodInfo(IMethodSymbol constructorSymbol, RecordDeclarationSyntax re } // Record primary constructors return [Factory]-annotated types (excluded by DiscoverDtoTypes) - this.DtoReturnTypes = DiscoverDtoTypes(constructorSymbol); + (this.DtoReturnTypes, this.DtoPreserveTypes) = DiscoverDtoTypes(constructorSymbol); } public string Name { get; set; } @@ -730,24 +745,32 @@ protected MethodInfo(IMethodSymbol constructorSymbol, RecordDeclarationSyntax re /// public EquatableArray DtoReturnTypes { get; private set; } = []; + /// + /// Deduplicated FQNs of DTO types without a public parameterless constructor + /// (positional records) discovered in this method's signature. Rendered as + /// DtoConstructorRegistry.PreserveType<T>() calls for IL trimming support. + /// + public EquatableArray DtoPreserveTypes { get; private set; } = []; + /// /// Discovers plain DTO types in a method's return type and non-service parameters that need - /// constructor registration for IL trimming support. Unwraps Task, nullable, and generic - /// collections. Excludes primitives, [Factory] types, abstract/interface types, and types - /// without parameterless ctors. Recursively walks public properties of discovered DTOs to - /// find nested types. Delegates to DtoTypeWalker.WalkFactoryReturn — shared with the - /// event-type preservation path. + /// preservation for IL trimming support. Unwraps Task, nullable, and generic collections. + /// Excludes primitives, [Factory] types, and abstract/interface types. Recursively walks + /// public properties of discovered DTOs to find nested types. Delegates to + /// DtoTypeWalker.WalkDtoGraph, which bucket-sorts by constructor shape: parameterless → + /// Register bucket, parameterized-only (positional records) → PreserveType bucket. /// - private static EquatableArray DiscoverDtoTypes(IMethodSymbol methodSymbol) + private static (EquatableArray RegisterTypes, EquatableArray PreserveTypes) DiscoverDtoTypes(IMethodSymbol methodSymbol) { var visited = new HashSet(); - var dtoTypes = new List(); + var registerTypes = new List(); + var preserveTypes = new List(); // Discover from return type var returnCandidates = DtoTypeWalker.UnwrapType(methodSymbol.ReturnType, unwrapTask: true); foreach (var candidate in returnCandidates) { - DtoTypeWalker.WalkFactoryReturn(candidate, dtoTypes, visited); + DtoTypeWalker.WalkDtoGraph(candidate, registerTypes, preserveTypes, visited); } // Discover from non-service, non-CancellationToken parameters @@ -764,11 +787,11 @@ private static EquatableArray DiscoverDtoTypes(IMethodSymbol methodSymbo var paramCandidates = DtoTypeWalker.UnwrapType(parameter.Type, unwrapTask: false); foreach (var candidate in paramCandidates) { - DtoTypeWalker.WalkFactoryReturn(candidate, dtoTypes, visited); + DtoTypeWalker.WalkDtoGraph(candidate, registerTypes, preserveTypes, visited); } } - return new EquatableArray([.. dtoTypes]); + return (new EquatableArray([.. registerTypes]), new EquatableArray([.. preserveTypes])); } } diff --git a/src/Generator/Model/ClassFactoryModel.cs b/src/Generator/Model/ClassFactoryModel.cs index 99661f7b..9a5c8325 100644 --- a/src/Generator/Model/ClassFactoryModel.cs +++ b/src/Generator/Model/ClassFactoryModel.cs @@ -18,7 +18,8 @@ public ClassFactoryModel( bool hasDefaultSave = false, bool requiresEntityRegistration = false, bool registerOrdinalConverter = false, - IReadOnlyList? dtoReturnTypes = null) + IReadOnlyList? dtoReturnTypes = null, + IReadOnlyList? dtoPreserveTypes = null) { TypeName = typeName; ServiceTypeName = serviceTypeName; @@ -30,6 +31,7 @@ public ClassFactoryModel( RequiresEntityRegistration = requiresEntityRegistration; RegisterOrdinalConverter = registerOrdinalConverter; DtoReturnTypes = dtoReturnTypes ?? System.Array.Empty(); + DtoPreserveTypes = dtoPreserveTypes ?? System.Array.Empty(); } public string TypeName { get; } @@ -47,6 +49,12 @@ public ClassFactoryModel( /// public IReadOnlyList DtoReturnTypes { get; } + /// + /// Positional-record DTO types (no public parameterless ctor) that need + /// PreserveType registration for IL trimming support. + /// + public IReadOnlyList DtoPreserveTypes { get; } + /// /// True if ALL factory methods are internal (excluding [Remote] methods, which are promoted to public). /// When true, the generated factory interface is internal. diff --git a/src/Generator/Model/InterfaceFactoryModel.cs b/src/Generator/Model/InterfaceFactoryModel.cs index af99cb12..a768e776 100644 --- a/src/Generator/Model/InterfaceFactoryModel.cs +++ b/src/Generator/Model/InterfaceFactoryModel.cs @@ -12,12 +12,14 @@ public InterfaceFactoryModel( string serviceTypeName, string implementationTypeName, IReadOnlyList? methods = null, - IReadOnlyList? dtoReturnTypes = null) + IReadOnlyList? dtoReturnTypes = null, + IReadOnlyList? dtoPreserveTypes = null) { ServiceTypeName = serviceTypeName; ImplementationTypeName = implementationTypeName; Methods = methods ?? System.Array.Empty(); DtoReturnTypes = dtoReturnTypes ?? System.Array.Empty(); + DtoPreserveTypes = dtoPreserveTypes ?? System.Array.Empty(); } public string ServiceTypeName { get; } @@ -28,4 +30,10 @@ public InterfaceFactoryModel( /// Plain DTO types that need constructor registration for IL trimming support. /// public IReadOnlyList DtoReturnTypes { get; } + + /// + /// Positional-record DTO types (no public parameterless ctor) that need + /// PreserveType registration for IL trimming support. + /// + public IReadOnlyList DtoPreserveTypes { get; } } diff --git a/src/Generator/Model/StaticFactoryModel.cs b/src/Generator/Model/StaticFactoryModel.cs index 1ab9e180..75d4438c 100644 --- a/src/Generator/Model/StaticFactoryModel.cs +++ b/src/Generator/Model/StaticFactoryModel.cs @@ -12,13 +12,15 @@ public StaticFactoryModel( string signatureText, bool isPartial = false, IReadOnlyList? delegates = null, - IReadOnlyList? dtoReturnTypes = null) + IReadOnlyList? dtoReturnTypes = null, + IReadOnlyList? dtoPreserveTypes = null) { TypeName = typeName; SignatureText = signatureText; IsPartial = isPartial; Delegates = delegates ?? System.Array.Empty(); DtoReturnTypes = dtoReturnTypes ?? System.Array.Empty(); + DtoPreserveTypes = dtoPreserveTypes ?? System.Array.Empty(); } public string TypeName { get; } @@ -30,4 +32,10 @@ public StaticFactoryModel( /// Plain DTO types that need constructor registration for IL trimming support. /// public IReadOnlyList DtoReturnTypes { get; } + + /// + /// Positional-record DTO types (no public parameterless ctor) that need + /// PreserveType registration for IL trimming support. + /// + public IReadOnlyList DtoPreserveTypes { get; } } diff --git a/src/Generator/Renderer/ClassFactoryRenderer.cs b/src/Generator/Renderer/ClassFactoryRenderer.cs index 1236782a..cfea275c 100644 --- a/src/Generator/Renderer/ClassFactoryRenderer.cs +++ b/src/Generator/Renderer/ClassFactoryRenderer.cs @@ -1538,7 +1538,7 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, ClassFactory } // DTO constructor registrations (IL trimming support) - if (model.DtoReturnTypes.Count > 0) + if (model.DtoReturnTypes.Count > 0 || model.DtoPreserveTypes.Count > 0) { sb.AppendLine(); sb.AppendLine(" // DTO constructor registrations (IL trimming support)"); @@ -1546,6 +1546,11 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, ClassFactory { sb.AppendLine($" DtoConstructorRegistry.Register<{dtoType}>(() => new {dtoType}());"); } + + foreach (var dtoType in model.DtoPreserveTypes) + { + sb.AppendLine($" DtoConstructorRegistry.PreserveType<{dtoType}>();"); + } } // Ordinal converter registration diff --git a/src/Generator/Renderer/InterfaceFactoryRenderer.cs b/src/Generator/Renderer/InterfaceFactoryRenderer.cs index 4662a2b0..a42080a7 100644 --- a/src/Generator/Renderer/InterfaceFactoryRenderer.cs +++ b/src/Generator/Renderer/InterfaceFactoryRenderer.cs @@ -477,7 +477,7 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, InterfaceFac } // DTO constructor registrations (IL trimming support) - if (model.DtoReturnTypes.Count > 0) + if (model.DtoReturnTypes.Count > 0 || model.DtoPreserveTypes.Count > 0) { sb.AppendLine(); sb.AppendLine(" // DTO constructor registrations (IL trimming support)"); @@ -485,6 +485,11 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, InterfaceFac { sb.AppendLine($" DtoConstructorRegistry.Register<{dtoType}>(() => new {dtoType}());"); } + + foreach (var dtoType in model.DtoPreserveTypes) + { + sb.AppendLine($" DtoConstructorRegistry.PreserveType<{dtoType}>();"); + } } sb.AppendLine(" }"); diff --git a/src/Generator/Renderer/StaticFactoryRenderer.cs b/src/Generator/Renderer/StaticFactoryRenderer.cs index be349809..fb0bb54a 100644 --- a/src/Generator/Renderer/StaticFactoryRenderer.cs +++ b/src/Generator/Renderer/StaticFactoryRenderer.cs @@ -114,7 +114,7 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, StaticFactor sb.AppendLine(" }"); // DTO constructor registrations (IL trimming support) - if (model.DtoReturnTypes.Count > 0) + if (model.DtoReturnTypes.Count > 0 || model.DtoPreserveTypes.Count > 0) { sb.AppendLine(); sb.AppendLine(" // DTO constructor registrations (IL trimming support)"); @@ -122,6 +122,11 @@ private static void RenderFactoryServiceRegistrar(StringBuilder sb, StaticFactor { sb.AppendLine($" DtoConstructorRegistry.Register<{dtoType}>(() => new {dtoType}());"); } + + foreach (var dtoType in model.DtoPreserveTypes) + { + sb.AppendLine($" DtoConstructorRegistry.PreserveType<{dtoType}>();"); + } } sb.AppendLine(" }"); diff --git a/src/Tests/RemoteFactory.TrimmingTests/Program.cs b/src/Tests/RemoteFactory.TrimmingTests/Program.cs index 4d0b6b54..e7354072 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/Program.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/Program.cs @@ -105,6 +105,13 @@ failedChecks.Add("event relay smoke"); } +// Record DTO smoke test (TRIM-001): positional records in factory signatures are +// preserved by generator-emitted PreserveType and deserialize on the trimmed client. +if (!RecordDtoSmokeTest.Run()) +{ + failedChecks.Add("record DTO preservation"); +} + Console.WriteLine($"IsServerRuntime: {NeatooRuntime.IsServerRuntime}"); Console.WriteLine($"Class factory resolved: {factory != null}"); Console.WriteLine($"Static factory delegate resolved: {doWorkDelegate != null}"); diff --git a/src/Tests/RemoteFactory.TrimmingTests/RecordDtoSmokeTest.cs b/src/Tests/RemoteFactory.TrimmingTests/RecordDtoSmokeTest.cs new file mode 100644 index 00000000..06259390 --- /dev/null +++ b/src/Tests/RemoteFactory.TrimmingTests/RecordDtoSmokeTest.cs @@ -0,0 +1,80 @@ +using Microsoft.Extensions.DependencyInjection; +using Neatoo.RemoteFactory; +using Neatoo.RemoteFactory.Internal; + +namespace RemoteFactory.TrimmingTests; + +/// +/// End-to-end trimming smoke test for positional-record DTO preservation (TRIM-001). +/// +/// The records (TrimRecordResult, TrimRecordCommand, TrimRecordDetail) appear in a +/// [Remote, Execute] signature but are never constructed in client-reachable code — +/// this test deserializes them from JSON literals, so their parameterized constructors +/// are rooted ONLY by the generator-emitted DtoConstructorRegistry.PreserveType<T>() +/// calls in the FactoryServiceRegistrar. If that emission is missing, the trimmer +/// strips the ctor metadata and deserialization fails — exactly the consumer-side +/// DeserializeNoConstructor failure this plan closes. +/// +/// Covers all three TRIM-001 shapes: +/// - record as [Execute] return type (TrimRecordResult) +/// - record nested in a discovered record (TrimRecordDetail, property of the result) +/// - record as non-service parameter (TrimRecordCommand) +/// +public static class RecordDtoSmokeTest +{ + public static bool Run() + { + var services = new ServiceCollection(); + services.AddNeatooRemoteFactory(NeatooFactory.Remote, typeof(RecordDtoSmokeTest).Assembly); + + using var sp = services.BuildServiceProvider(); + var serializer = sp.GetRequiredService(); + + // Record as return type, with nested record property. + TrimRecordResult? result; + try + { + result = serializer.Deserialize( + "{\"Id\":42,\"Message\":\"trim-smoke\",\"Detail\":{\"Notes\":\"nested\"}}"); + } + catch (Exception ex) + { + Console.WriteLine($"Record DTO smoke FAILED: return-shape deserialization threw {ex.GetType().Name}: {ex.Message}"); + return false; + } + + if (result is null || result.Id != 42 || result.Message != "trim-smoke") + { + Console.WriteLine($"Record DTO smoke FAILED: return-shape values lost. Got Id={result?.Id}, Message=\"{result?.Message}\"."); + return false; + } + + if (result.Detail is null || result.Detail.Notes != "nested") + { + Console.WriteLine($"Record DTO smoke FAILED: nested record lost. Got Notes=\"{result.Detail?.Notes}\"."); + return false; + } + + // Record as non-service parameter. + TrimRecordCommand? command; + try + { + command = serializer.Deserialize( + "{\"PatientId\":7,\"Reason\":\"checkup\"}"); + } + catch (Exception ex) + { + Console.WriteLine($"Record DTO smoke FAILED: parameter-shape deserialization threw {ex.GetType().Name}: {ex.Message}"); + return false; + } + + if (command is null || command.PatientId != 7 || command.Reason != "checkup") + { + Console.WriteLine($"Record DTO smoke FAILED: parameter-shape values lost. Got PatientId={command?.PatientId}, Reason=\"{command?.Reason}\"."); + return false; + } + + Console.WriteLine("Record DTO smoke PASSED: positional records survived trimming via generator-emitted PreserveType (return, parameter, and nested shapes)."); + return true; + } +} diff --git a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs index 37d06f39..3c264742 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs @@ -2,6 +2,17 @@ namespace RemoteFactory.TrimmingTests; +/// +/// Positional-record DTOs carried by . +/// None of these are constructed anywhere in client-reachable code — their +/// constructors and properties survive trimming only through the generator-emitted +/// DtoConstructorRegistry.PreserveType<T>() calls (TRIM-001). RecordDtoSmokeTest +/// deserializes them from JSON literals to prove that preservation. +/// +public record TrimRecordDetail(string Notes); +public record TrimRecordResult(int Id, string Message, TrimRecordDetail Detail); +public record TrimRecordCommand(int PatientId, string Reason); + /// /// Static factory used to test IL trimming of server-only dependencies. /// Static factories use delegate types (not factory interfaces), which are @@ -16,4 +27,16 @@ private static Task _DoWork(string input, [Service] IServerOnlyRepositor { return Task.FromResult(repo.DoServerWork(input)); } + + // Positional records as [Execute] return type (with a nested record) and as a + // non-service parameter — the zTreatment StartVisitResultV2 shape (TRIM-001). + [Remote] + [Execute] + private static Task _ProcessRecord(TrimRecordCommand command, [Service] IServerOnlyRepository repo) + { + return Task.FromResult(new TrimRecordResult( + command.PatientId, + repo.DoServerWork(command.Reason), + new TrimRecordDetail("processed"))); + } } diff --git a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs new file mode 100644 index 00000000..92af59b0 --- /dev/null +++ b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs @@ -0,0 +1,320 @@ +using RemoteFactory.UnitTests.TestContainers; + +namespace RemoteFactory.UnitTests.FactoryGenerator.DtoDiscovery; + +/// +/// Verifies the two-bucket DTO preservation emission (TRIM-001): positional records +/// (no public parameterless ctor) get DtoConstructorRegistry.PreserveType<T>() calls, +/// while plain DTOs keep getting Register<T>(() => new T()) — as return types, as +/// non-service parameters, and nested through property graphs in both directions. +/// +public class RecordDtoDiscoveryTests +{ + private static string RunAndGetGeneratedSource(string source) + { + var (_, _, runResult) = DiagnosticTestHelper.RunGenerator(source); + return string.Join("\n", runResult.GeneratedTrees.Select(t => t.GetText()?.ToString() ?? "")); + } + + /// + /// Extracts DtoConstructorRegistry.Register type arguments from generated source. + /// + private static HashSet GetRegisteredDtoTypes(string generatedSource) + { + var registered = new HashSet(); + var matches = System.Text.RegularExpressions.Regex.Matches( + generatedSource, + @"DtoConstructorRegistry\.Register<(.+?)>\(\(\)"); + + foreach (System.Text.RegularExpressions.Match match in matches) + { + registered.Add(match.Groups[1].Value); + } + + return registered; + } + + /// + /// Extracts DtoConstructorRegistry.PreserveType type arguments from generated source. + /// Anchored to the no-argument call shape so it never cross-captures Register calls. + /// + private static HashSet GetPreservedDtoTypes(string generatedSource) + { + var preserved = new HashSet(); + var matches = System.Text.RegularExpressions.Regex.Matches( + generatedSource, + @"DtoConstructorRegistry\.PreserveType<(.+?)>\(\)"); + + foreach (System.Text.RegularExpressions.Match match in matches) + { + preserved.Add(match.Groups[1].Value); + } + + return preserved; + } + + [Fact] + public void PositionalRecordAsReturnType_PreserveTypeEmitted() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record StartVisitResult(int VisitId, string Status); + + [Factory] + public static partial class VisitCommands + { + [Remote] + [Execute] + internal static Task _StartVisit(int patientId) + => Task.FromResult(new StartVisitResult(patientId, ""started"")); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.StartVisitResult", GetPreservedDtoTypes(generated)); + Assert.DoesNotContain("global::TestNamespace.StartVisitResult", GetRegisteredDtoTypes(generated)); + } + + [Fact] + public void PositionalRecordAsParameter_PreserveTypeEmitted() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record VisitCommand(int PatientId, string Reason); + + [Factory] + public static partial class VisitCommands + { + [Remote] + [Execute] + internal static Task _StartVisit(VisitCommand command) + => Task.FromResult(command.PatientId); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.VisitCommand", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void PositionalRecordNestedInClassDto_BothBucketsEmitted() + { + var source = @" +using Neatoo.RemoteFactory; + +namespace TestNamespace +{ + public record BannerInfo(string Text, string Severity); + + public class DashboardDto + { + public int Id { get; set; } + public BannerInfo Banner { get; set; } + } + + [Factory] + public partial class MyEntity + { + [Create] + internal DashboardDto Create() => new DashboardDto(); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.DashboardDto", GetRegisteredDtoTypes(generated)); + Assert.Contains("global::TestNamespace.BannerInfo", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void ClassDtoNestedInPositionalRecord_DescentEntersRecordGraph() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public class DetailInfo + { + public string Notes { get; set; } + } + + public record ResultRecord(int Id, DetailInfo Detail); + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _Run(int id) + => Task.FromResult(new ResultRecord(id, new DetailInfo())); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.ResultRecord", GetPreservedDtoTypes(generated)); + Assert.Contains("global::TestNamespace.DetailInfo", GetRegisteredDtoTypes(generated)); + } + + [Fact] + public void CollectionOfRecords_UnwrappedAndPreserved() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Collections.Generic; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record ContactResult(int Id, string Name); + + [Factory] + public static partial class SearchCommands + { + [Remote] + [Execute] + internal static Task> _Search(string term) + => Task.FromResult(new List()); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.ContactResult", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void RecordWithBothCtorShapes_StaysInRegisterBucket() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record FlexibleDto + { + public FlexibleDto() { } + public FlexibleDto(int id) { Id = id; } + public int Id { get; set; } + } + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _Run() => Task.FromResult(new FlexibleDto()); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.FlexibleDto", GetRegisteredDtoTypes(generated)); + Assert.DoesNotContain("global::TestNamespace.FlexibleDto", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void PositionalRecordFromInterfaceFactory_PreserveTypeEmitted() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record LookupResult(int Id, string Name); + + [Factory] + public interface ILookupService + { + [Remote] + Task GetByIdAsync(int id); + } + + public class LookupService : ILookupService + { + public Task GetByIdAsync(int id) + => Task.FromResult(new LookupResult(id, ""x"")); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.LookupResult", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void FactoryAnnotatedType_NoEmissionOfEitherKind() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + [Factory] + public partial class OtherEntity + { + [Create] + internal void Create() { } + } + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _Load([Service] OtherEntity entity) + => Task.FromResult(entity); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.DoesNotContain("global::TestNamespace.OtherEntity", GetRegisteredDtoTypes(generated)); + Assert.DoesNotContain("global::TestNamespace.OtherEntity", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void PrivateCtorOnlyType_NoEmissionOfEitherKind() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public class Opaque + { + private Opaque() { } + public int Id { get; set; } + } + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _Run() => Task.FromResult(null); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.DoesNotContain("global::TestNamespace.Opaque", GetRegisteredDtoTypes(generated)); + Assert.DoesNotContain("global::TestNamespace.Opaque", GetPreservedDtoTypes(generated)); + } +} From 4151dc29059769302df940985337eb236ceccc06 Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Mon, 6 Jul 2026 15:13:32 -0500 Subject: [PATCH 4/5] =?UTF-8?q?test:=20close=20TRIM-001=20test-review=20ga?= =?UTF-8?q?te=20=E2=80=94=20genuine=20negative=20controls,=20record-struct?= =?UTF-8?q?=20+=20dedupe=20coverage?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The gate caught false trimmed-harness coverage: _ProcessRecord's constructed body rooted the record ctors (guarded-dead bodies are retained on the client, the TRIM-005 behavior), so the return/nested checks passed even with PreserveType emission disabled. The harness method now returns null — discovery is signature-based — and two-stage negative controls prove each shape depends solely on the emission. Adds RecordStruct_LandsInRegisterBucket and SameRecordFromTwoMethods_SinglePreserveTypeEmission; queues the pre-existing incremental-cache test hole as TRIM-006. Co-Authored-By: Claude Fable 5 --- ...ositional-record-signature-preservation.md | 12 +++- .../006-incremental-cache-regression-test.md | 11 ++++ .../reviews/001-test-review.md | 36 +++++++++++ .../todo.md | 9 ++- .../TrimTestCommands.cs | 12 ++-- .../DtoDiscovery/RecordDtoDiscoveryTests.cs | 61 +++++++++++++++++++ 6 files changed, 132 insertions(+), 9 deletions(-) create mode 100644 docs/todos/TRIM-dto-trimming-preservation-gaps/plans/006-incremental-cache-regression-test.md create mode 100644 docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-test-review.md diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md index 3215878b..9d864824 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/001-positional-record-signature-preservation.md @@ -93,8 +93,8 @@ Filled 2026-07-06, before the Step 5 gate. All test classes are in `RemoteFactor | Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | |---|---|---|---| | PreserveType emitted for return / parameter / nested; Register unchanged | `[unit]` | `PositionalRecordAsReturnType_PreserveTypeEmitted`, `PositionalRecordAsParameter_PreserveTypeEmitted`, `PositionalRecordNestedInClassDto_BothBucketsEmitted`, `ClassDtoNestedInPositionalRecord_DescentEntersRecordGraph`, `CollectionOfRecords_UnwrappedAndPreserved`, `RecordWithBothCtorShapes_StaysInRegisterBucket`, `PositionalRecordFromInterfaceFactory_PreserveTypeEmitted` (all three renderer paths: static, class, interface) | ✓ | -| Record-as-return round-trips on publish-trimmed client | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` return shape (`TrimRecordResult`) — trimmed run exit 0; **negative control**: PreserveType emission disabled → harness FAILED "record DTO preservation", exit 1 | ✓ | -| Record-as-parameter and nested-record shapes round-trip trimmed | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` parameter shape (`TrimRecordCommand`) + nested (`TrimRecordDetail` property) | ✓ | +| Record-as-return round-trips on publish-trimmed client | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` return shape (`TrimRecordResult`) — trimmed run exit 0; **negative control v2** (after the test-review gate redesigned the harness so no record is ever constructed): emission disabled → *return-shape* check itself throws `NotSupportedException` on `TrimRecordResult`, exit 1 — proving the ctor is rooted solely by `PreserveType` | ✓ | +| Record-as-parameter and nested-record shapes round-trip trimmed | `[trimmed-harness]` | `RecordDtoSmokeTest.Run` parameter shape (`TrimRecordCommand`) + nested (`TrimRecordDetail` property); parameter shape independently proven by negative control v1 (failed on `TrimRecordCommand` while the return shape was still body-rooted) | ✓ | | Exclusions intact (no emission of either kind) | `[unit]` | `FactoryAnnotatedType_NoEmissionOfEitherKind`, `PrivateCtorOnlyType_NoEmissionOfEitherKind`; existing `NestedDtoDiscoveryTests` suite stays green | ✓ | | Build/test/CI gates | `[explicit-skip]` | `reviews/001-build.log` (0 errors, 2 pre-existing warnings), `reviews/001-test.log` (full suite), `reviews/001-test-relay-rerun.log` (unrelated flaky `RelayTimingTests` re-run green in isolation) | ✓ | | Docs describe two-bucket emission | `[explicit-skip]` | `docs/trimming.md`, `CLAUDE-DESIGN.md` (registry section + criteria table + FAQ row), `AllPatterns.cs` `ExampleRecordResult` remarks | ✓ | @@ -103,7 +103,13 @@ Filled 2026-07-06, before the Step 5 gate. All test classes are in `RemoteFactor ## Plan Amendments -(None yet.) +### 2026-07-06 — Harness records must never be constructed; gate findings closed + +- **Section affected:** Steps 5 (harness repro design), Test Evidence +- **Original said:** `_ProcessRecord` constructs and returns the record result (realistic server body). +- **What changed:** the test-review gate caught that the constructed-body design made the return/nested trimmed checks vacuous — the guarded-dead `_ProcessRecord` body is retained on the client (the TRIM-005 over-retention behavior) and roots the ctors regardless of `PreserveType`. Negative control v1 proved it: with emission disabled, the *parameter* shape failed while the return shape passed. `_ProcessRecord` now returns `Task.FromResult(null)` — discovery is signature-based, so preservation is unaffected, and no record is constructed anywhere in the harness. Negative control v2 then failed on the return shape itself. Also added from the gate's should-cover tier: `RecordStruct_LandsInRegisterBucket` and `SameRecordFromTwoMethods_SinglePreserveTypeEmission` unit tests; queued the pre-existing incremental-cache test hole as TRIM-006. +- **Why:** a check that passes with the feature disabled pins nothing — the gate's independent eye caught false coverage the self-authored evidence map could not. +- **Discovery Log link:** 2026-07-06 — TRIM-001 (gate closed). --- diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/006-incremental-cache-regression-test.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/006-incremental-cache-regression-test.md new file mode 100644 index 00000000..011f2284 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/006-incremental-cache-regression-test.md @@ -0,0 +1,11 @@ +# TRIM-006 — Incremental-generator caching regression test + +**Plan #:** 006 +**Status:** Draft +**Plan-review opt-in:** TBD at draft +**Code-review opt-in:** TBD at draft +**Related Todo:** [../todo.md](../todo.md) + +## Scope + +Add a driver-level regression test for the generator's incremental caching, closing the project-wide hole plan-review B1 (TRIM-001) exposed: the pipeline cache boundary lives on the transform-output records (`TypeInfo` / `TypeFactoryMethodInfo` / `MethodInfo`), and a non-`EquatableArray` field added there silently breaks caching for every consumer with **no failing test** — `DiagnosticTestHelper.RunGenerator` runs the generator exactly once and never asserts cached steps. The test should run the driver twice with `GeneratorDriverOptions`/`WithTrackingIncrementalGeneratorSteps`, apply an unrelated edit between runs, and assert the factory-generation steps report `Cached`/`Unchanged` — guarding all current and future transform-output fields (including TRIM-001's `DtoPreserveTypes`). Surfaced by the TRIM-001 test-review gate as pre-existing tech debt (2026-07-06). Does NOT change generator behavior. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-test-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-test-review.md new file mode 100644 index 00000000..7385530d --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-test-review.md @@ -0,0 +1,36 @@ +# TRIM-001 Test Review (Step 5 Gate) — 2026-07-06 + +**Reviewer:** test-reviewer agent, two passes (initial + closure). +**Logs:** `001-build.log` (0 errors), `001-test.log` (final: 572+572 unit, 563+563 integration, 0 failed, net9.0+net10.0), `001-test-relay-rerun.log`, `001-publish.log`. +**Gate result: CLEARED** — no open must-cover or should-cover findings. + +## Initial pass + +Evidence map verified honest (all cited methods exist, assert what they claim, at declared tier; renderer coverage static/class/interface real; no vacuous assertions; no sacred tests touched — Program.cs/TrimTestCommands.cs additive only). Findings: + +1. **should-cover (quality):** trimmed-harness negative control didn't isolate the return/nested shapes — `_ProcessRecord`'s constructed body could root the ctors (guarded-dead bodies are retained, per TRIM-005), making those checks potentially vacuous. +2. **should-cover (plan-related):** `record struct` bucket assignment untested (plan-review B3 edge). +3. **should-cover (tech-debt):** no incremental-cache regression test exists project-wide (plan-review B1) — nothing guards the `EquatableArray` requirement on transform-output fields. +4. **nice-to-have:** cross-method preserve-bucket dedupe; abstract/nullable record edges. + +## Response and closure + +| Finding | Disposition | +|---|---| +| Negative-control isolation | **CLOSED** — `_ProcessRecord` now returns `null` (no record constructed anywhere in the harness; discovery is signature-based). Two-stage control: v1 (constructed body, emission off) failed on the *parameter* shape — proving the return shape had been body-rooted, exactly as the reviewer suspected; v2 (null body, emission off) failed on the *return* shape itself (`NotSupportedException` on `TrimRecordResult`, exit 1). Emission restored → all green, exit 0. | +| `record struct` | **CLOSED** — `RecordStruct_LandsInRegisterBucket` pins Register-bucket assignment + Preserve-bucket absence. | +| Incremental-cache tech debt | **CLOSED via queue** — TRIM-006 stub + Index row (not absorbed into this plan). | +| Cross-method dedupe | **CLOSED** — `SameRecordFromTwoMethods_SinglePreserveTypeEmission`. | +| Abstract/nullable record edges | **ACCEPTED-WITH-REASON** — shared `IsDtoStructureCandidate`/`UnwrapType` gates already exercised for class DTOs (`NestedDtoDiscoveryTests` TS-005/TS-010/TS-011); low-risk. | +| Untrimmed `record struct` round-trip | **ACCEPTED-WITH-REASON** — runtime bypass-converter struct behavior is pre-existing and untouched by this plan. | + +## Closing tier picture + +- must-cover: none (never open). +- should-cover: none open. +- nice-to-have: dedupe added; two declines accepted with recorded reasons. +- tech-debt: queued as TRIM-006. + +**Reviewer's closing note:** the Test Evidence map "now survives an independent read with no overreach"; the previously-unproven return/nested trimmed-harness controls are genuine. The gate's marquee catch — false trimmed-harness coverage that the self-authored evidence map could not see — is recorded in the plan's Amendments and the todo Discovery Log. + +Also observed at this gate (unrelated to the plan): `RelayTimingTests.Relay_FiresAfterCallerSynchronousWriteOnContinuation` flaked once under parallel load (net9.0, TimeoutException), green in isolation and on both subsequent full runs — logged in the Discovery Log, flagged to the user, not queued in TRIM. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index e33e4aa7..1ea92f1b 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -45,8 +45,9 @@ A third suspected gap turned out to be already fixed: event records derive `Fact | 002 | Draft | [`[Factory]` entity property-graph DTO discovery](./plans/002-factory-entity-property-dto-discovery.md) | `WalkFactoryReturn` bails on `[Factory]` roots without descending; zTreatment `TreatmentBanner` / `DashboardContactResult` hotfixes | | 003 | Draft | [Verify event-record preservation needs no consumer entries](./plans/003-verify-event-record-preservation.md) | `FactoryEventBase` DAM annotation shipped v1.4.0; consumer entries predate it, never re-tested | | 005 | Draft | [Server-only reference over-retention in trimmed clients](./plans/005-server-only-reference-over-retention.md) | TRIM-004 discovery: guarded-dead `LocalCreate` bodies retain server-only interface refs, contradicting `docs/trimming.md` | +| 006 | Draft | [Incremental-generator caching regression test](./plans/006-incremental-cache-regression-test.md) | TRIM-001 gate: no test asserts cached pipeline steps — non-EquatableArray transform fields regress silently (plan review B1) | -Execution order: 004 → 001 → 002 → 003 → 005 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. +Execution order: 004 → 001 → 002 → 003 → 005 → 006 (rows listed in execution order; numbering stays monotonic by creation). Branching: todo/plan docs commit on the `TRIM` branch; each plan's implementation gets its own branch off `TRIM`. ## Skipped Steps @@ -76,6 +77,12 @@ Execution order: 004 → 001 → 002 → 003 → 005 (rows listed in execution o - **Decision:** Defer. - **Follow-up:** flagged to user — out-of-goal tech debt; queue as sibling todo or accept as known flake (not queued in TRIM). +### 2026-07-06 — TRIM-001 (gate closed) +- **Finding:** Test-review gate returned zero must-cover gaps but caught false trimmed-harness coverage: the constructed-body harness design let the return/nested checks pass with the emission disabled (guarded-dead bodies root ctors — the TRIM-005 behavior). Harness redesigned so no record is ever constructed; negative controls v1+v2 now prove each shape depends on `PreserveType`. Added `record struct` + cross-method dedupe unit tests from the should-cover tier. Long form: TRIM-001 Plan Amendment + `reviews/001-test-review.md`. +- **Decision:** Amend. +- **Index changes:** add TRIM-006 (incremental-cache regression test — pre-existing tech debt, plan review B1), executed last. +- **Follow-up:** TRIM-006. + ### 2026-07-06 — TRIM-004 (server-only over-retention) - **Finding:** A trimmed client retains the `IServerOnlyRepository` TypeDef and `DoServerWork` member ref: generated `LocalCreate` bodies are rooted by delegate registration and their early-`throw` guard + `try/catch` defeats ILLink unreachable-code elimination. Implementations are correctly trimmed. Contradicts `docs/trimming.md` "should return no matches" / "dead code is removed" claims. TRIM-004's CI grep narrowed to implementation types (Plan Amendment 3). - **Decision:** Defer. diff --git a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs index 3c264742..445f6ae1 100644 --- a/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs +++ b/src/Tests/RemoteFactory.TrimmingTests/TrimTestCommands.cs @@ -30,13 +30,15 @@ private static Task _DoWork(string input, [Service] IServerOnlyRepositor // Positional records as [Execute] return type (with a nested record) and as a // non-service parameter — the zTreatment StartVisitResultV2 shape (TRIM-001). + // DTO discovery is signature-based, so the body deliberately never constructs + // the records: a `new TrimRecordResult(...)` here would root the ctor from the + // (retained, guarded-dead) method body and make RecordDtoSmokeTest pass even + // without the generator's PreserveType emission — a vacuous check. [Remote] [Execute] - private static Task _ProcessRecord(TrimRecordCommand command, [Service] IServerOnlyRepository repo) + private static Task _ProcessRecord(TrimRecordCommand command, [Service] IServerOnlyRepository repo) { - return Task.FromResult(new TrimRecordResult( - command.PatientId, - repo.DoServerWork(command.Reason), - new TrimRecordDetail("processed"))); + repo.DoServerWork(command.Reason); + return Task.FromResult(null); } } diff --git a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs index 92af59b0..8abe1e38 100644 --- a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs +++ b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/DtoDiscovery/RecordDtoDiscoveryTests.cs @@ -256,6 +256,67 @@ public Task GetByIdAsync(int id) Assert.Contains("global::TestNamespace.LookupResult", GetPreservedDtoTypes(generated)); } + [Fact] + public void RecordStruct_LandsInRegisterBucket() + { + // record struct: Roslyn reports the synthesized public parameterless ctor, + // so value-type records take the Register bucket (the runtime bypass + // converter still claims them — benign divergence, see plan review B3). + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record struct PointResult(int X, int Y); + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _Locate() => Task.FromResult(new PointResult(1, 2)); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + Assert.Contains("global::TestNamespace.PointResult", GetRegisteredDtoTypes(generated)); + Assert.DoesNotContain("global::TestNamespace.PointResult", GetPreservedDtoTypes(generated)); + } + + [Fact] + public void SameRecordFromTwoMethods_SinglePreserveTypeEmission() + { + var source = @" +using Neatoo.RemoteFactory; +using System.Threading.Tasks; + +namespace TestNamespace +{ + public record SharedResult(int Id); + + [Factory] + public static partial class Commands + { + [Remote] + [Execute] + internal static Task _First() => Task.FromResult(new SharedResult(1)); + + [Remote] + [Execute] + internal static Task _Second() => Task.FromResult(new SharedResult(2)); + } +} +"; + var generated = RunAndGetGeneratedSource(source); + + var emissions = System.Text.RegularExpressions.Regex.Matches( + generated, + @"DtoConstructorRegistry\.PreserveType\(\)"); + Assert.Single(emissions); + } + [Fact] public void FactoryAnnotatedType_NoEmissionOfEitherKind() { From 83b7d86ead6aa1ad094cc2dac3d117aec266c2ee Mon Sep 17 00:00:00 2001 From: Keith Voels Date: Mon, 6 Jul 2026 15:21:13 -0500 Subject: [PATCH 5/5] =?UTF-8?q?docs(todo):=20TRIM-001=20code=20review=20cl?= =?UTF-8?q?ean=20=E2=80=94=20callout=20routed=20to=20TRIM-002=20scope?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-Authored-By: Claude Fable 5 --- ...2-factory-entity-property-dto-discovery.md | 2 +- .../reviews/001-code-review.md | 20 +++++++++++++++++++ .../todo.md | 6 ++++++ 3 files changed, 27 insertions(+), 1 deletion(-) create mode 100644 docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-code-review.md diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md index d9f0f74d..56faa53f 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/002-factory-entity-property-dto-discovery.md @@ -8,4 +8,4 @@ ## Scope -Extend DTO discovery to descend into `[Factory]`-annotated types' public property graphs without treating the entity itself as a DTO. Today `WalkFactoryReturn` rejects a `[Factory]` root (correct — entities are preserved via DI registration) but returns before walking its properties, so a plain DTO reachable *only* as an entity property is never discovered and gets trimmed on the client. Consumer evidence from the zTreatment cut-over: `TreatmentBanner` (a record property on the `[Execute]`-opened `TreatmentContext` aggregate) and `DashboardContactResult` (a `List` property on the `PatientSearchQuery` factory entity) both required manual LinkerConfig entries. The descent must reuse the same bucket-sort emission as TRIM-001 (Register vs PreserveType), share the visited-set for cycle safety across entity graphs (entities referencing entities, child lists), and skip entity-typed properties themselves while walking through them for DTO-typed leaves. This is the most design-open of the three plans — settle the walk's boundary rules (which factory-rooted types get their properties walked: all `[Factory]` types in the compilation, or only those reachable from factory method signatures) at draft time. Includes a publish-trimmed `RemoteFactory.TrimmingTests` case (DTO reachable only via entity property) and a `docs/trimming.md` update. Does NOT change entity preservation itself (already handled by `NeatooFactoryRegistrar` + DI registration). +Extend DTO discovery to descend into `[Factory]`-annotated types' public property graphs without treating the entity itself as a DTO. Today `WalkFactoryReturn` rejects a `[Factory]` root (correct — entities are preserved via DI registration) but returns before walking its properties, so a plain DTO reachable *only* as an entity property is never discovered and gets trimmed on the client. Consumer evidence from the zTreatment cut-over: `TreatmentBanner` (a record property on the `[Execute]`-opened `TreatmentContext` aggregate) and `DashboardContactResult` (a `List` property on the `PatientSearchQuery` factory entity) both required manual LinkerConfig entries. The descent must reuse the same bucket-sort emission as TRIM-001 (Register vs PreserveType), share the visited-set for cycle safety across entity graphs (entities referencing entities, child lists), and skip entity-typed properties themselves while walking through them for DTO-typed leaves. This is the most design-open of the three plans — settle the walk's boundary rules (which factory-rooted types get their properties walked: all `[Factory]` types in the compilation, or only those reachable from factory method signatures) at draft time. While in the candidate checks, also tighten the pre-existing `ns.StartsWith("System")` prefix match to a segment match (`ns == "System" || ns.StartsWith("System.")`) — TRIM-001 code-review callout: a consumer namespace like `Systems.Domain` is currently excluded from preservation. Includes a publish-trimmed `RemoteFactory.TrimmingTests` case (DTO reachable only via entity property) and a `docs/trimming.md` update. Does NOT change entity preservation itself (already handled by `NeatooFactoryRegistrar` + DI registration). diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-code-review.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-code-review.md new file mode 100644 index 00000000..6235009d --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/001-code-review.md @@ -0,0 +1,20 @@ +# TRIM-001 Code Review (Step 5, opt-in) — 2026-07-06 + +**Reviewer:** code-reviewer agent, findings-only (no grade). Range `3752405..4151dc2` (bed0651 feat + 4151dc2 gate closure). Logs: `001-build.log`, `001-test.log` (grepped, not re-run). + +**Result: no veto-tier findings.** Deliverable landed cleanly, shape verified correct. + +## Verified + +- **Plan-review B1:** `DtoPreserveTypes` is `EquatableArray` on all three transform-output records (`TypeInfo`, `MethodInfo`, `TypeFactoryMethodInfo`); models correctly relax to `IReadOnlyList` (not cache keys). +- **Plan-review B2:** `WalkDtoGraph` buckets roots by ctor shape; `WalkEventRoot`'s root-always-Preserve rule fully retired, zero references remain; stale header comment fixed. +- **Emission placement:** all three registrars emit `PreserveType()` unguarded alongside `Register()` — matching Register's client/server-agnostic placement; no fourth site exists. +- **Semantics preserved:** rejection (structure or no-public-ctor) happens before `visited.Add`, matching prior behavior; the Register path is behaviorally identical to old `WalkFactoryReturn`; the new walk is a strict superset (records now descend). +- **Runtime parity:** Preserve bucket rule exactly matches `RecordBypassConverterFactory.CanConvert`; `PreserveType` deliberately does not populate the ctor registry. +- **Repo rules:** no reflection added; sacred tests untouched (additive-only harness changes); no DDD tutorial prose; build 0 errors (2 pre-existing WASM workload warnings); 2276 tests, 0 failed. +- **Plan-review A1 doc coherence:** no surviving sentence implies `PreserveType` is emitted nowhere; event-path removal sentence stays correctly scoped; CLAUDE-DESIGN/trimming.md/AllPatterns accurate to shipped behavior. + +## Callout-tier findings + +1. **Pre-existing:** `IsDtoStructureCandidate` excludes by `ns.StartsWith("System")` — a prefix match, so a consumer namespace like `Systems.Domain` would be silently excluded from both buckets (`DtoTypeWalker.cs:97`). Low confidence / negligible likelihood; unchanged by this plan (Constraints preserved exclusions intact). **Disposition:** routed to TRIM-002's draft-time scope — that plan already reworks the candidate checks at this exact seam (tighten to `ns == "System" || ns.StartsWith("System.")`). +2. **For the record only:** `record struct` trimmed round-trip has emission-side coverage only — already ACCEPTED-WITH-REASON at the test gate; no action. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index 1ea92f1b..bc9e4dca 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -83,6 +83,12 @@ Execution order: 004 → 001 → 002 → 003 → 005 → 006 (rows listed in exe - **Index changes:** add TRIM-006 (incremental-cache regression test — pre-existing tech debt, plan review B1), executed last. - **Follow-up:** TRIM-006. +### 2026-07-06 — TRIM-001 (code review clean) +- **Finding:** Opt-in code review returned zero veto findings (B1/B2 compliance, emission placement, semantics, docs all verified — `reviews/001-code-review.md`). One low-confidence pre-existing callout: `IsDtoStructureCandidate`'s `StartsWith("System")` prefix match would exclude a consumer namespace like `Systems.Domain` from preservation. +- **Decision:** Amend. +- **Index changes:** none — the hardening is folded into TRIM-002's stub scope (that plan already reworks the candidate checks at the same seam). +- **Follow-up:** TRIM-002. + ### 2026-07-06 — TRIM-004 (server-only over-retention) - **Finding:** A trimmed client retains the `IServerOnlyRepository` TypeDef and `DoServerWork` member ref: generated `LocalCreate` bodies are rooted by delegate registration and their early-`throw` guard + `try/catch` defeats ILLink unreachable-code elimination. Implementations are correctly trimmed. Contradicts `docs/trimming.md` "should return no matches" / "dead code is removed" claims. TRIM-004's CI grep narrowed to implementation types (Plan Amendment 3). - **Decision:** Defer.