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 index f14b040a..c25393d4 100644 --- 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 @@ -3,7 +3,7 @@ **Plan #:** 006 **Date:** 2026-08-11 **Related Todo:** [../todo.md](../todo.md) -**Status:** In Progress +**Status:** Done **Last Updated:** 2026-08-12 **Plan-review opt-in:** No (test-first plan, narrow blast radius, emitted output unchanged; the diagnosis was verified at the keyboard at draft time rather than inherited — see Current State) **Code-review opt-in:** Yes (touches generator transform-output types) @@ -87,7 +87,7 @@ Filled 2026-08-12, before the Step 5 gate. Logs in the session scratchpad; per-r | Acceptance bullet (short) | Tier declared | Test method | Tier confirmed | |---|---|---|---| | Every branch stays cached across an unrelated edit | `[unit]` | `IncrementalCacheTests.UnrelatedEdit_TransformOutputStaysCached` — `[Theory]`, 4 cases (`FactoryClass`, `FactoryInterface`, `RelayHandler`, `FactoryEvents`); non-vacuity backed by `Fixture_ExercisesEveryPipelineBranch` | `[unit]` | -| Emitted output byte-identical before/after | `[unit]` | `UnrelatedEdit_GeneratedOutputIsIdentical` covers run-to-run determinism. The *before/after-this-plan* half is **not** a test — it is verified by zero `git status` drift in the committed `Generated/` trees of both solutions after a full rebuild that re-emitted them (relay-handler file re-emitted 09:42:33, checked 09:44:01) | `[unit]` + repo-artifact diff | +| Emitted output byte-identical before/after | `[unit]` | `UnrelatedEdit_GeneratedOutputIsIdentical` covers run-to-run determinism. The *before/after-this-plan* half is **not** a test — it is a one-off measurement: `IntegrationTests`' full generated tree emitted with the pre-fix generator (`710498c^:RelayHandlerModel.cs`) into a clean directory, then with HEAD's, diffed recursively → **256 files byte-identical, including all 16 `.FactoryEventHandler.g.cs`**. (The original evidence here — zero `git status` drift under `Generated/` — was **vacuous**: `.gitignore:405` is `**/Generated/`, so git could not have reported drift. Corrected at close-out audit V2.) | `[unit]` + one-off emission diff | | Relay-handler branch caches | `[unit]` | Same `[Theory]`, `RelayHandler` case — observed `Modified` → `Unchanged` across the fix | `[unit]` | | Guard sensitivity (negative control) | `[explicit-skip]` | Performed at the keyboard, not committed — see Implementation Record. All four branches proven to go red on a reference-equality field | `[explicit-skip]` — honored | | Existing diagnostic suite unchanged | `[explicit-skip]` | Covered by full-suite run; `RunGenerator`'s signature and single-run behavior untouched (extraction of `BuildReferences` only) | `[explicit-skip]` — honored | @@ -101,7 +101,7 @@ Filled 2026-08-12, before the Step 5 gate. Logs in the session scratchpad; per-r **Red → green (Step 5).** Guard against unmodified HEAD: `RelayHandler` failed with reason `Modified`, the other three cached — pre-flight's prediction confirmed exactly, and the guard proven non-vacuous before it was made to pass. After moving `RelayHandlerModel.Usings/Entries/Diagnostics` and `EventHandlerEntry.Parameters/ServiceParameters/AllParameters` onto `EquatableArray`: 6/6 green. Constructors normalize incoming sequences rather than taking the array type, so a future call site cannot reintroduce the defect by passing a plain list. -**Negative control (Step 6).** Added a `IReadOnlyList` auto-property with a fresh-allocation initializer to `TypeInfo` (branches 1–2) and `FactoryEventInfo` (branch 4), covering the three branches that were *already* passing — the RelayHandler red above is the control for branch 3. Result: `FactoryClass`, `FactoryInterface`, `FactoryEvents` all went red with reason `Modified`; `RelayHandler` stayed `Unchanged`. Probes and the throwaway dump test removed; `git status` confirms no residue. +**Negative control (Step 6).** Added a `IReadOnlyList` auto-property with a fresh-allocation initializer to `TypeInfo` (branches 1–2) and `FactoryEventInfo` (branch 4), covering the three branches that were *already* passing — the RelayHandler red above is the control for branch 3. Result: `FactoryClass`, `FactoryInterface`, `FactoryEvents` all went red with reason `Modified`; `RelayHandler` stayed `Unchanged`. Probes and the throwaway dump test removed; `git status` confirms no residue. **This control tested transform-output *roots* only** — it did not reach nested element types, which is why the `EventHandlerEntry` gap survived it. See Plan Amendments; a fourth control at the nested level was added post-merge. **Finding — the guard can read stale generator code locally.** The first negative-control run *passed*, which was wrong. Cause: the generator is loaded once per process via `Assembly.LoadFrom` into a static `Lazy`, so rebuilding only the generator and re-running with `--no-build` let a surviving testhost serve the previously loaded assembly. A dump of all tracked steps proved the probe was live in the DLL while the guard reported green. Rebuilding the test project produced the correct red. Documented in `RunGeneratorTracked`'s remarks; CI is unaffected because every run starts cold. Recorded in the todo's Discovery Log — this will bite the next person doing a red/green cycle on any dynamically-loaded-generator test, not just this one. @@ -111,7 +111,13 @@ Filled 2026-08-12, before the Step 5 gate. Logs in the session scratchpad; per-r ## Plan Amendments -(None yet.) +**2026-08-12 — post-merge correction from the close-out audit (V1, V2).** Recorded as an amendment because Step 4 was reported met when it was not. + +- **Step 4 was unmet and reported as met.** The fixture declared two matching handlers for the *same* event type, which is the NF0502 ambiguous-match shape: the transform reports the diagnostic and `continue`s without adding an entry. `RelayHandlerModel.Entries` was therefore empty, and `EventHandlerEntry.Parameters` / `ServiceParameters` / `AllParameters` — the three fields this plan converted — were never constructed and never guarded. The `RelayHandler` red→green transition recorded below was real but was driven entirely by `Usings` and `Diagnostics`; the nested type was never in play. The fixture's own comment asserted the opposite. Fixed: two distinct event types, one matching handler each; comment corrected to name the NF0502 trap; coverage re-proven by reverting `EventHandlerEntry.Parameters` to `IReadOnlyList` and observing the `RelayHandler` case go red — coverage the guard provably did not have before. +- **Two fixture-health tests added**, because "the fixture silently stopped exercising the thing" was the actual failure mode and nothing detected it: `Fixture_ProducesNoDiagnostics` (a diagnostic means some transform took an early-out and never built the model under guard) and `Fixture_EmitsRelayHandlerOutput` (the relay output stage returns early on `Entries.Count == 0`, so an empty-entry fixture emits no file). The first caught a **second** latent defect on its first run: `[Fetch]` on an interface-factory member (NF0106), which had been degrading the `FactoryInterface` branch too. +- **The byte-identity evidence was vacuous** and is replaced — see the Test Evidence row. + +Root cause worth carrying: both defects were *self-reported as verified*. The lesson is the same one TRIM-005 produced — a claim of verification is not verification — and it recurred inside the very plan written to enforce non-vacuity. --- diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md index c6731eac..b09aed69 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/plans/007-subscribe-only-event-preservation-fix.md @@ -3,8 +3,8 @@ **Plan #:** 007 **Date:** 2026-07-13 **Related Todo:** [../todo.md](../todo.md) -**Status:** In Progress -**Last Updated:** 2026-07-13 +**Status:** Done +**Last Updated:** 2026-08-12 (status header reconciled with the Plan Index at close-out audit V3; the plan completed and merged via PR #71 on 2026-07-13) **Plan-review opt-in:** Yes (new incremental-generator pipeline branch; generator emission contract change; corrects documented behavior that currently overpromises) **Code-review opt-in:** Yes (behavior-changing generator work) diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/close-out-audit.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/close-out-audit.md new file mode 100644 index 00000000..ee493e35 --- /dev/null +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/reviews/close-out-audit.md @@ -0,0 +1,97 @@ +# TRIM — Close-Out Audit + +**Date:** 2026-08-12 +**Mode:** Step 7 close-out (whole arc, findings-only) +**Branch audited:** `TRIM` at `3aed354` (fast-forwarded to `main`, all plan PRs merged) +**Verdict:** **CONCERNS** — 4 veto-tier findings, all since addressed (see Disposition below) + +--- + +## Headline + +AC1–AC3 are genuinely verified in a publish-trimmed artifact at HEAD, with CI evidence (GitHub Actions run **31658596913** at `3aed354`, linux-x64 self-contained, `TrimMode=full`, `IsServerRuntime: False` — all four preservation smokes PASSED). This is **not** a repeat of TRIM-003, where a preservation claim was assumed and came back red. + +The veto findings concentrated in TRIM-006 — the one plan that received no independent per-plan gate, its gates having been folded into this audit. + +--- + +## Veto-Tier Findings and Disposition + +### V1 — TRIM-006's guard never exercised `EventHandlerEntry` — FIXED + +The fixture declared two matching static handlers (`Notify`, `Audit`) for the **same** event type on one `[FactoryEventHandler]` class. That is the NF0502 ambiguous-match shape: `FactoryGenerator.RelayHandler.cs:160-174` reports the diagnostic and `continue`s **without adding an entry**. + +Consequences, all confirmed independently before accepting the finding: + +- `RelayHandlerModel.Entries` was empty, so `EventHandlerEntry.Parameters` / `ServiceParameters` / `AllParameters` — the exact three fields TRIM-006 converted to `EquatableArray` — were never constructed and never guarded. +- The observed `RelayHandler` red→green transition was driven entirely by `RelayHandlerModel.Usings` and `.Diagnostics`. The nested element type was never in play. +- `FactoryGenerator.cs:101` returns early on `Entries.Count == 0`, so `RelayHandlerRenderer.Render` never ran either. +- The fixture's own doc comment asserted the opposite — a false statement in a test, worse than no comment. +- Plan Step 4 ("populate every collection-bearing field on each transform output") was unmet with no Plan Amendment recording the divergence. + +**Fix:** fixture now carries two *distinct* event types with one matching handler each, so `Entries` holds two values and each entry's three parameter collections are populated. Comment corrected to state the NF0502 trap explicitly. Two fixture-health tests added: `Fixture_ProducesNoDiagnostics` and `Fixture_EmitsRelayHandlerOutput`. + +**Verified, not assumed:** with the corrected fixture, reverting `EventHandlerEntry.Parameters` to `IReadOnlyList` turns the `RelayHandler` case **red** — coverage the guard provably did not have before. Restored after. + +**The health assertion immediately earned its keep:** it caught a *second* latent fixture defect on its first run — `[Fetch]` on an interface-factory member (NF0106; the interface *is* the boundary, so operation attributes are invalid there). That had been silently degrading the `FactoryInterface` branch too. + +### V2 — the "byte-identical output" evidence was vacuous — FIXED + +The plan cited "zero `git status` drift in the committed `Generated/` trees," explicitly framed as "verified empirically rather than by inference." + +`.gitignore:405` is `**/Generated/` and `git ls-files | grep "Generated/"` returns **zero** tracked files. Those trees are untracked and ignored, so `git status` could not report drift regardless of what the generator emitted. The check proved nothing. (`reviews/005-plan-review.md:83` had already recorded that generated files are not git-tracked.) + +**Fix:** replaced with a real measurement. Emitted `RemoteFactory.IntegrationTests`' full generated tree with the pre-fix generator (`710498c^:src/Generator/Model/RelayHandlerModel.cs`) into a clean directory, then with HEAD's generator, and diffed recursively: **256 files, byte-identical, including all 16 `.FactoryEventHandler.g.cs` files**. The conclusion was correct; the original evidence for it was not. + +### V3 — TRIM-007's plan file contradicted the Plan Index — FIXED + +`plans/007-...md:6` read `Status: In Progress` while `todo.md` showed `007 | Done` and commit `602a6d4` was titled "mark TRIM-007 Done." Header reconciled, with a note that the reconciliation is dated 2026-08-12 while the plan itself completed 2026-07-13. + +### V4 — three verified `005-plan-review.md` findings had no disposition — FIXED + +B8 and B10 were routed nowhere. Both re-verified rather than inherited: + +- **B8** — `grep -rn "AppContext.SetSwitch" src/` returns nothing, and `"Server-only method called in non-server runtime."` appears only in generator renderers, never in a test assertion. **Nothing pins the guard's runtime throw.** +- **B10** — `InternalVisibilityTests.cs:235-236,274-275,280-281,322-323` slices generated text with naive `IndexOf` arithmetic delimited by the next member name; an emission reorder mis-slices and the `DoesNotContain` assertions pass vacuously — the same false-green class TRIM-001's test gate caught as its marquee finding. + +**Fix:** both given explicit dispositions in the Deferred Work Carrying Forward table in `todo.md`. + +--- + +## Acceptance Criteria Trace + +| Criterion | Evidence | Holds? | +|---|---|---| +| **AC1** — positional record as return / parameter / nested property deserializes publish-trimmed | Bucket walk `DtoTypeWalker.WalkDtoGraph:156-197`; trimmed proof `RecordDtoSmokeTest.cs:Run:25-79` (JSON-literal deserialization, no record constructed anywhere). CI 31658596913: "Record DTO smoke PASSED" | **Yes** | +| **AC2** — DTO reachable only as a `[Factory]` entity property survives trimming | `DtoTypeWalker.WalkEntityProperties:207-214` via `FactoryGenerator.Types.cs:257-272`; trimmed proof `EntityPropertyDtoSmokeTest.cs:Run:21-67`. CI: "Entity property DTO smoke PASSED" | **Yes** | +| **AC3** — subscribe-only `FactoryEventBase` record deserializes publish-trimmed, verified not assumed | Branch 4 `FactoryGenerator.cs:113-130`; `EventPreservationRenderer.cs:27-103` (registrar targets a **generated** type at `:71`); trimmed proof `EventSubscribeOnlySmokeTest.cs:Run:74-146` (string-literal `TypeFullName`, no `typeof`, no construction). CI: "Subscribe-only event smoke PASSED" | **Yes** | +| **AC4** — `docs/trimming.md` updated + release notes | Docs half landed (`docs/trimming.md:254`, `:266-285`, `:287-314`). Release half **open**: version still `1.6.1`, no `v1.7.0.md` | **Open — deliberately held** | +| **AC5** — consumer proof via zTreatment PCB-003 | Not started; blocked on the held release | **Open** | + +--- + +## Build & Test Evidence + +- Build: `Build succeeded. 3 Warning(s), 0 Error(s)` — 2× WASM workload warnings in an unrelated example project, 1× pre-existing `CA1062`. +- Tests: **2324 passed, 0 failed, 10 skipped** — UnitTests 601+601, IntegrationTests 561+561 (5 skips × 2 TFMs). +- Design solution: 86 + 86 passed, 0 failed. +- Skipped inventory: `RelayTimingTests.cs:56,:105` (user decision, this arc) + 3 pre-existing "Optional Performance Demo" tests. +- Trimmed-artifact gate at HEAD: CI run 31658596913, all four smokes passed, "Server-only implementation types absent from trimmed assembly." One pre-existing `IL2057` from `ServiceAssemblies.FindType`, untouched by this arc. + +--- + +## Container Integrity + +7 plan files, 7 Index rows, no orphans, numbering monotonic with no duplicates. TRIM-005's Abandoned status carries a substantive Abandonment Reason naming the real seam and the successor lesson. Skipped Steps entries exist for all three non-run gates (004, 003, 006). Out of Scope holds — `git diff v1.6.1..HEAD -- src/` contains zero `IFactorySaveMeta` hits and no zTreatment-side work. + +**Sacred tests:** only comment-only edits to `FactoryEventHandlerTests.cs` and `FactoryEventBaseAttributeTests.cs`, plus two `[Fact(Skip=...)]` additions carrying user-decision reasons. No assertions removed, no expected values bent, no reflection introduced. + +--- + +## Verified Non-Findings + +Recorded because they were explicitly challenged: + +- **No other transform-output type carries a reference-equality collection at HEAD.** The full reachable graph of all three transform outputs (`TypeInfo`, `RelayHandlerModel`, `FactoryEventInfo` → `TypeFactoryMethodInfo`, `TypeAuthMethodInfo`, `MethodInfo`, `MethodParameterInfo`, `OrdinalPropertyInfo`, `AspAuthorizeInfo`, `DiagnosticInfo`, `EventHandlerEntry`, `ParameterModel`) uses `EquatableArray` throughout. +- **`ExecuteDelegateModel.ServiceParameters` is outside the cache boundary**, as suspected but unverified when flagged. It is constructed only at `Builder/FactoryModelBuilder.cs:482`, and `FactoryModelBuilder.Build` is called only from the two `RegisterSourceOutput` lambdas — output-stage, not transform-output. No defect. +- **`skills/.../class-factory.md:318,333-334` and `advanced-patterns.md:227` describe *class* factories**, which `ClassFactoryRenderer.cs:54` targets at the generated type and which the 2026-08-11 trimmed probe showed trim correctly. These were **over-listed** as falsified in the 2026-08-11 Discovery Log entry; they are arguably true as written. diff --git a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md index b1f37252..5d47b00c 100644 --- a/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md +++ b/docs/todos/TRIM-dto-trimming-preservation-gaps/todo.md @@ -46,7 +46,7 @@ A third suspected gap turned out to be already fixed: event records derive `Fact | 003 | Done | [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 — **verification came back RED**, re-split → TRIM-007 | | 007 | Done | [Subscribe-only event preservation fix](./plans/007-subscribe-only-event-preservation-fix.md) | TRIM-003 finding: inherited DAM doesn't flow to derived types under ILLink; fixed via generator-emitted per-assembly event-preservation registrar | | 005 | Abandoned | [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` — **diagnosis falsified at plan review**, see 2026-08-11 log entry | -| 006 | In Progress | [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) | +| 006 | Done | [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 → 007 → 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`. (TRIM-003's red verification and TRIM-007's fix merged together via PR #71.) @@ -54,6 +54,36 @@ Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 (rows liste - TRIM-004 — `test-reviewer` gate skipped (test-infrastructure-only plan: every Acceptance bullet is `explicit-skip`; the harness itself is the test artifact, evidence recorded in the plan's Test Evidence table). - TRIM-003 — `test-reviewer` gate skipped (verification-only plan whose deliverable is the finding itself; non-vacuity proven by the red-trimmed / green-untrimmed / green-annotated triplet recorded in the plan's Test Evidence; the repro check lands under TRIM-007's full gate). +- TRIM-006 — per-plan `test-reviewer` and `code-reviewer` gates **folded into the Step 7 close-out audit** (user decision 2026-08-12) rather than run separately. PR #73 merged ahead of the gate; the close-out audit covers TRIM-006's code as part of the whole-arc pass. Note this is a genuine deviation, not a no-op: the per-plan `test-reviewer` pass that would have scrutinised the guard's own coverage never happened, so the close-out audit is the only independent review this plan receives. **This deviation cost something real** — the audit found the guard's fixture never exercised `EventHandlerEntry` at all (V1), a gap a per-plan test review would have been far likelier to catch before merge. Fixed post-merge; see `reviews/close-out-audit.md`. + +## Close-Out Audit + +**2026-08-12 — Verdict: CONCERNS → addressed.** Full record: [`reviews/close-out-audit.md`](./reviews/close-out-audit.md). + +AC1–AC3 confirmed genuinely verified in a publish-trimmed artifact at HEAD (CI run 31658596913 at `3aed354`) — not a repeat of TRIM-003's assumed-preservation failure. Four veto-tier findings, all concentrated in TRIM-006 (the plan whose per-plan gates were folded into this audit) plus container bookkeeping; all four fixed: + +- **V1** — the caching guard's fixture hit NF0502 (two handlers, one event), so `RelayHandlerModel.Entries` was empty and `EventHandlerEntry`'s three converted collections were never constructed *or guarded*. Fixture corrected to two distinct events; two fixture-health tests added; coverage re-proven by reverting a nested field and observing red. The new health test immediately caught a second latent defect (NF0106 `[Fetch]` on an interface-factory member). +- **V2** — the "byte-identical output" claim rested on `git status` over `**/Generated/`, which is **gitignored** — the check could not have detected drift. Replaced with a real 256-file recursive diff of pre-fix vs post-fix emission. Conclusion held; the evidence had not. +- **V3** — TRIM-007's plan header said `In Progress` against a `Done` Index row. Reconciled. +- **V4** — `005-plan-review.md` findings B8 and B10 had no disposition anywhere. Now carried in the table below. + +## Deferred Work Carrying Forward + +| # | Item | Destination | Cost if it stays open | +|---|---|---|---| +| 1 | **Registrar-DAM over-preservation** — `[Remote]` bodies for `[Execute]` and `[FactoryEventHandler]` classes ship to the browser decompilable | Built-in plan mode (user decision 2026-08-11); **release-blocking**, so it cannot be silently dropped | Highest-cost item. Its only record is a Discovery Log entry that archives when this todo closes — give it a durable home if the todo closes before the fix lands | +| 2 | **Release held (AC4 + AC5)** — version stays `1.6.1`, no v1.7.0 notes | Reopens when item 1 merges | zTreatment PCB-003 blocked since July. Deliberate trade: consumer unblock-time vs. publishing false IP guidance | +| 3 | **`DiagnosticTestHelper` stale-generator hazard** — a generator fix can appear verified when it was never loaded; affects the whole generator suite | Documented at the seam (`DiagnosticTestHelper.cs`); durable fix (fail fast when the generator DLL predates the test assembly) explicitly not done | Local-iteration only (CI is cold-build). Already produced one false green during TRIM-006 | +| 4 | **B8 — nothing pins the guard's runtime throw.** No `AppContext.SetSwitch` anywhere; `"Server-only method called in non-server runtime."` never asserted | **Accepted with reason:** pre-existing, not introduced by this arc, and the trimmed-harness CI gate covers the property that actually matters (server-only types absent from the trimmed artifact). Queue if the guard's message or shape is ever edited | A regression deleting the throw ships silently in untrimmed/server scenarios | +| 5 | **B10 — 16 emission assertions can pass vacuously.** `InternalVisibilityTests` / `CanMethodVisibilityTests` slice generated text with naive `IndexOf` bounded by the next member name | **Queued, unowned.** Not fixed here: out of TRIM-006's scope, and rewriting 16 assertions in sacred tests needs its own plan with its own review | False-green on the generated-code visibility contract — the same class of defect TRIM-001's test gate caught as its marquee finding | +| 6 | **B9 — harness cannot verify the relay-handler leg** (no relay-handler target touches a server-only service) | Tied to item 1 | Item 1 would ship fixed but unverified | +| 7 | **Falsified TRIM-005 story in live artifacts** — `.github/workflows/build.yml:111-112`, `TrimmingTests/README.md:31`, `TrimTestCommands.cs:35`. The CI grep's `(?` spurious `Register>` emission** | **Accepted**, TRIM-002 Amendment — idempotent and harmless; removal needs its own trimmed verification | Cosmetic registrar noise | +| 13 | **Interface-factory implementation classes get no property walk** | **Accepted by design**, TRIM-002 Constraint; documented at `docs/trimming.md:285` | A consumer serializing state off such a class still needs manual preservation | ## Discovery Log @@ -89,6 +119,7 @@ Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 (rows liste ### 2026-07-13 — TRIM-007 (gates cleared, merged) - **Finding:** Generator-emission fix landed (PR #71, CI green first run): fourth pipeline branch + per-assembly `NeatooEventPreservationRegistrar`; TRIM-003's red check green in the pure consumer shape incl. nested record. Plan review's veto (accessibility gate — the repo's own private nested test events would have broken every consumer build) folded pre-implementation. Test gate cleared (10 unit tests incl. determinism + FQN-decoy guards). Code review caught 3 veto doc findings — the falsified DAM claim surviving in `FactoryEventRelayPattern.cs`, `docs/factory-events.md`, and the smoke test's own summary — all fixed, plus skill-reference and IL2026-justification callouts. Reviews: `reviews/007-*.md`. - **Decision:** Amend. +- **Correction (2026-08-12, close-out audit C8):** "Amend" is the wrong word and no Plan Amendment was ever written, which is why `plans/007-...md` still reads "Plan Amendments — (None yet.)". What actually happened was a *pre-implementation plan revision*: the plan-review veto was folded into the Steps before any code was written, which the workflow treats as ordinary Step-2 revision, not a post-hoc amendment. Nothing is missing from the plan; the decision word here was imprecise. Entry left standing rather than rewritten, per the append-only rule. - **Follow-up:** n/a — remaining queue: TRIM-005, TRIM-006, then todo-level release step (AC4 release notes deferred there). ### 2026-07-07 — TRIM-003 (verification RED, re-split → TRIM-007) @@ -135,3 +166,9 @@ Execution order: 004 → 001 → 002 → 003 → 007 → 005 → 006 (rows liste - **Finding:** `DiagnosticTestHelper` loads `Neatoo.Generator.dll` once per process via `Assembly.LoadFrom` into a static `Lazy`. Rebuilding **only** the generator project and re-running `dotnet test --no-build` can let a surviving testhost serve the previously loaded assembly, so a test reports on code that is no longer on disk. Caught during TRIM-006's negative control: the probe field was provably present in the loaded DLL (`grep` on the binary, plus a full tracked-step dump showing `Modified`) while the guard reported 6/6 green; rebuilding the test project produced the correct red. This is not specific to the caching guard — it applies to **every** test that goes through `DiagnosticTestHelper`, which is effectively the whole generator suite. A generator fix can appear verified when it was never loaded. - **Decision:** Document at the seam rather than re-architect the loader — `RunGeneratorTracked`'s remarks now carry the warning. CI is unaffected (cold build every run), so the exposure is local iteration only. - **Follow-up:** If this bites again, the durable fix is to make the helper fail fast on a generator DLL older than the test assembly. Deliberately not done here — out of TRIM-006's scope, and the plan's Constraints forbid reworking the shared helper. + +### 2026-08-12 — Release held: shipping now would publish falsified IP-protection claims +- **Finding:** With all plans landed, the arc is functionally releasable — AC1–AC3 are met and verified in the trimmed artifact, `docs/trimming.md` and the Design projects were updated during TRIM-001/002/007, and the `feat:` commits since `v1.6.1` put the bump at 1.7.0. But the registrar-DAM defect (2026-08-11 entry) falsifies live documentation that would ship with it. `docs/trimming.md:35` claims `[Execute]` static factories' "registration lambdas and their captured dependencies" are removed by the trimmer; `:222` then accurately describes the very mechanism that makes that false ("preserve all methods on the referenced type"). The two passages contradict each other in the same published page, and `:7` sells IP protection as a headline benefit. Releasing would tell users their `[Remote]` bodies are stripped from `[Execute]` and `[FactoryEventHandler]` classes when they demonstrably are not. +- **Decision:** **Hold the release** (user decision 2026-08-12) until the registrar-DAM fix lands in plan mode, then ship one version carrying both the trimming-preservation work and the over-retention fix, with the docs true again on publication. The alternative considered and rejected was to narrow the doc claims and release immediately. +- **Consequence, stated plainly:** zTreatment PCB-003 stays blocked for the duration — it has been waiting since the framework-side goal was met in July. This is a deliberate trade of consumer unblock-time for not publishing false security guidance. +- **Follow-up:** AC4 (release notes + version bump) and AC5 (consumer proof) stay open; the todo does **not** close on this audit. Reopen the release step once the registrar-DAM fix merges. diff --git a/src/Generator/DtoTypeWalker.cs b/src/Generator/DtoTypeWalker.cs index ce7d26be..27aabc54 100644 --- a/src/Generator/DtoTypeWalker.cs +++ b/src/Generator/DtoTypeWalker.cs @@ -1,7 +1,11 @@ // DtoTypeWalker.cs // Shared walker for discovering DTO types reachable from a root symbol. -// Used by the factory-signature path (MethodInfo.DiscoverDtoTypes) for both -// return types and non-service parameters. Discovered types bucket-sort by +// Three callers, all bucketing into the same two registries: +// - factory signatures (MethodInfo.DiscoverDtoTypes) — return types and +// non-service parameters +// - [Factory] entity property graphs (FactoryGenerator.Types.cs, TRIM-002) +// - FactoryEventBase descendant graphs (FactoryGenerator.Events.cs, TRIM-007) +// Discovered types bucket-sort by // constructor shape: parameterless -> DtoConstructorRegistry.Register(), // parameterized-only -> DtoConstructorRegistry.PreserveType(). diff --git a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/Core/IncrementalCacheTests.cs b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/Core/IncrementalCacheTests.cs index 790ff74c..728edc29 100644 --- a/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/Core/IncrementalCacheTests.cs +++ b/src/Tests/RemoteFactory.UnitTests/FactoryGenerator/Core/IncrementalCacheTests.cs @@ -56,8 +56,9 @@ public interface INotifier // populates DtoReturnTypes / DtoPreserveTypes on TypeInfo. public record OrderSummary(int Id, string Label); - // Branch 4: FactoryEventBase descendant. + // Branch 4: FactoryEventBase descendants. public record OrderPlacedEvent(int OrderId, string CustomerEmail) : FactoryEventBase; + public record OrderShippedEvent(int OrderId, string Carrier) : FactoryEventBase; // Branch 1: [Factory] class — multiple methods, services, DTO in the signature. [Factory] @@ -80,18 +81,25 @@ public void Create(string name) } } - // Branch 2: [Factory] interface. + // Branch 2: [Factory] interface. No operation attributes on the members — the + // interface IS the remote boundary, and [Fetch] here is NF0106. [Factory] public interface IOrderReader { - [Fetch] OrderSummary? Read(int id); } - // Branch 3: [FactoryEventHandler] — two handlers, each with non-service - // parameters, service parameters, and a CancellationToken, so Entries and all - // three parameter collections on EventHandlerEntry are non-empty. + // Branch 3: [FactoryEventHandler] — TWO DISTINCT event types, one matching + // handler each, so RelayHandlerModel.Entries holds two EventHandlerEntry values + // and each entry's Parameters / ServiceParameters / AllParameters are non-empty. + // + // The two attributes must name DIFFERENT events. Two handlers for the SAME event + // is the NF0502 ambiguous-match shape: the transform reports the diagnostic and + // `continue`s WITHOUT adding an entry, leaving Entries empty and every + // EventHandlerEntry collection unconstructed — the guard would then cover none of + // them while appearing to. Fixture_ProducesNoDiagnostics pins this. [FactoryEventHandler] + [FactoryEventHandler] public static partial class OrderHandlers { internal static async Task Notify( @@ -103,11 +111,11 @@ internal static async Task Notify( } internal static async Task Audit( - OrderPlacedEvent orderEvent, + OrderShippedEvent orderEvent, [Service] INotifier notifier, CancellationToken cancellationToken) { - await notifier.SendAsync(""audit@example.com"", ""audited""); + await notifier.SendAsync(""audit@example.com"", orderEvent.Carrier); } } } @@ -166,6 +174,46 @@ public void UnrelatedEdit_TransformOutputStaysCached(string trackingName) + "value-equatable all the way down."); } + /// + /// Fixture health. A branch can report tracked steps while its transform bailed + /// early on a diagnostic, leaving the very collections under test unconstructed — + /// the guard then covers nothing and still passes. That is not hypothetical: the + /// first version of this fixture declared two handlers for one event, hit NF0502, + /// and silently guarded an empty Entries for the whole of TRIM-006. + /// + [Fact] + public void Fixture_ProducesNoDiagnostics() + { + var (_, second) = DiagnosticTestHelper.RunGeneratorTracked(Fixture, UnrelatedAppendix); + + var nf = second.Diagnostics.Where(d => d.Id.StartsWith("NF")).ToList(); + + Assert.True( + nf.Count == 0, + "The caching fixture must generate cleanly. A diagnostic means some transform took an " + + "early-out path and never built the model the guard is supposed to protect: " + + string.Join("; ", nf.Select(d => $"{d.Id} {d.GetMessage()}"))); + } + + /// + /// Pins that the relay-handler branch actually reached emission. Complements + /// : the relay output stage returns early + /// when Entries.Count == 0, so an empty-entry fixture produces no file here. + /// + [Fact] + public void Fixture_EmitsRelayHandlerOutput() + { + var (_, second) = DiagnosticTestHelper.RunGeneratorTracked(Fixture, UnrelatedAppendix); + + var files = second.GeneratedTrees.Select(t => Path.GetFileName(t.FilePath)).ToList(); + + Assert.True( + files.Any(f => f.EndsWith(".FactoryEventHandler.g.cs")), + "No relay-handler source was emitted, so RelayHandlerModel.Entries was empty and " + + "EventHandlerEntry's collections were never constructed — the RelayHandler case of " + + $"the caching guard would be vacuous. Emitted: {string.Join(", ", files)}"); + } + [Fact] public void Fixture_ExercisesEveryPipelineBranch() {