Move generic expansion out of the per-keystroke path (S7)

openspec/changes/messagepack-sourcegen-validation/design.md

@@ -382,6 +382,20 @@ Measured on the `SourceGeneratorBenchmarks` corpus (which references `Akka.Remot
 
 *`ManifestPrefix` expansion is cross-assembly-correct, but not yet migrated to the shared stage.* Decision 18's expansion mechanism (`ExpandClosedGenericRegistration`/`TryGetClosedSetMembers`, `AkkaSerializerGenerator.Extraction.cs`) still runs inside the per-serializer-node extraction step, not inside `CompilationFacts` or a new shared stage. It now calls a referenced-assembly counterpart of the local walk (`EnumerateReferencedAssemblyMarkedImplementors`) directly, so a closed set spanning the assembly boundary expands correctly -- proven byte-identically in the golden corpus (a `ManifestPrefix` registration over a protocol whose implementors sit on both sides of the boundary). What this does NOT yet do is share `CompilationFacts`' own cached walk result: `ExtractSerializerCore` (this method's caller) is itself the per-node transform `CompilationFacts` is built FROM (via the collected `serializers` array), so it cannot consume `CompilationFacts`' cached value without a circular pipeline dependency. Hoisting the expansion itself into a new per-compilation stage that consumes `CompilationFacts` and re-resolves each member's symbol (the "construction extraction" the setup instructions describe) is a functionally-motivated but not yet implemented refinement: it would let a `ManifestPrefix` registration's own IDE-performance caching match the ideal `CompilationFacts` already achieves for top-level dispatch and the implicit-union rules, at the cost of a larger pipeline restructuring than this change makes. Flagged here as a deliberate scoping decision, not an oversight.
 
+**Implementation addendum (S7).** The walk flagged above as a deliberate scoping gap is closed in this pass, which stacks on F3 (`ab82136da`).
+
+*The walk moved, and where it lives now.* `ExpandClosedGenericRegistration`/`TryGetClosedSetMembers`/`AddDiscoveredClosedSetMembers`/`ComputeLocalMarkedProtocolImplementors`/`EnumerateReferencedAssemblyMarkedImplementors` are gone from `AkkaSerializerGenerator.Extraction.cs`. In their place: the per-node serializer extraction transform still classifies each `ManifestPrefix` target's own type-argument positions (`TryClassifyPrefixArgumentPosition`, `BuildPrefixExpansionSpec`) using only the argument's own symbol -- no walk of the compilation's declared types -- and records a light, symbol-free `PrefixExpansionSpec` per registration (`SerializerInfo.PrefixExpansions`) whenever at least one position is protocol- or `[AkkaUnion]`-flavored. An invalid target, an unresolvable fixed argument, or no closed-set-flavored position at all is still resolved and reported as an AKKASG040 error entirely in the transform, exactly as before -- none of those depend on a whole-compilation walk. A new per-compilation stage, `ComputeClosedGenericExpansions` (`AkkaSerializerGenerator.Expansion.cs`, wired next to `CompilationFacts`/`ComputeMetadataSchemas`), consumes every serializer's own `PrefixExpansions` plus `CompilationFacts`' own `LocalMarkedImplementorsByClosedSetKey`/`ReferencedAssemblyImplementorsByProtocol` buckets, re-resolves each candidate's `TypeKey` back to a symbol, runs the SAME cartesian product and manifest formula F2 ran inline, and extracts each construction's schema through the same `ExtractMessageCore`. `CompilationFacts`' own closed-set-key request list (`ComputeDistinctClosedSetKeys`, `AkkaSerializerGenerator.Facts.cs`) is extended to also include a `PrefixExpansionSpec`'s own discovered-mode keys, so the shared buckets this stage reads already cover them.
+
+*The merge point.* The expansion stage's own output, `PrefixExpansionTable` (keyed by serializer), is folded back into each `SerializerInfo`'s `ClosedGenericRegistrations`/`ClosedGenericSchemas` by one function, `SerializerInfo.WithClosedGenericExpansion` (called from `MergeSerializerClosedGenericExpansions` immediately after the expansion stage runs, producing `effectiveSerializers`). Every OTHER consumer of either property -- `ResolveSerializerMessages`, validation, `MessageTypeLocationKey`, the placement diagnostics, and Decision 16's own metadata-schema seed scan (`ComputeReferencedTypeKeys`) -- reads `effectiveSerializers` instead of the raw collected array and needs no other change: none of them know the expansion table exists.
+
+*Byte-identical output needed a splice, not an append.* F2/F3 expanded a registration's own members INLINE, in the same per-attribute loop iteration that added its literal entry (if any) -- so a serializer with an early literal registration, a `ManifestPrefix` registration, and a later literal registration produced dispatch arms interleaved in that exact order. Simply appending every expansion member after every literal one (the first working version of this hoist) reordered the golden corpus's own dispatch switch and failed the byte-identical gate. Each `PrefixExpansionSpec` now also records `RegistrationInsertionIndex`/`SchemaInsertionIndex` -- `ClosedGenericRegistrations`'/`ClosedGenericSchemas`' own array length at the moment the transform reached that attribute -- and `SpliceExpansionGroups<T>` (`AkkaSerializerGenerator.Expansion.cs`) inserts each `PrefixExpansionGroup`'s own members back at that exact position. This is what makes the hoist behavior-neutral at the byte level, not merely at the round-trip level.
+
+*A second symbol-resolution bug the golden corpus caught immediately.* `Compilation.GetTypeByMetadataName` only ever resolves a type's own OPEN DEFINITION -- a generic type's metadata name carries its arity, never its type arguments -- so re-resolving a "fixed" position's own candidate (Decision 18's "G&lt;H&lt;M&gt;&gt;" nested case, e.g. a sibling registration's `Envelope<AcceptCassette>`) with a single `GetTypeByMetadataName` call silently produced the unsubstituted `Envelope<T>` instead. `ResolveTypeSymbol` (`AkkaSerializerGenerator.Expansion.cs`) resolves the definition, then recursively resolves and `Construct()`s each of `TypeKey.TypeArguments`' own entries; a non-generic key (the overwhelming common case: an implementor `CompilationFacts` already found) degenerates to the original single call.
+
+*What the transform still does for a prefix registration, and what it does not.* Still in the transform: reading the target's own validity (`IsFullyClosed`, definition attribute), `AllowEmpty`, the raw `explicitManifestByTarget`/`SerializerInfo.ExplicitManifestsByTarget` map (read once from the serializer's own small, fixed attribute list, not a compilation-wide walk), and classifying each position -- including fully resolving an EXPLICIT `[AkkaUnion(typeof(A), typeof(B))]` position's own member list, since that needs only the argument's own attribute, not a search over every declared type. No longer in the transform: discovering a DISCOVERED-mode position's own member set (that read now goes through `CompilationFacts`), constructing any closed generic symbol, and extracting any expansion member's schema.
+
+*Run reasons.* `ComputeClosedGenericExpansions` is wired as a "raw" stage (`context.CompilationProvider.Combine(serializers).Combine(compilationFacts).Select(...)`) whose own record-struct output bundles the value-equatable `PrefixExpansionTable` with this run's own `LocationBag` (an expanded member's `[AkkaField]` property locations, resolved from the SAME substituted symbols `ExtractMessageCore`'s inlined capture already produces for a literal registration -- these always point back to the GENERIC DEFINITION's own declared property, so no attribute-location plumbing of the expansion stage's own is needed for type-level diagnostics; `MessageTypeLocationKey` was changed to redirect an expansion member's own lookup through `ClosedGenericRegistrationInfo.ExpansionGroup` -- the SAME base-attribute location entry `BuildSerializerLocationBag` already records -- instead of a per-member entry, which is what let this stage skip producing one). `TrackingNames.ClosedGenericExpansions` names the TABLE-only projection of that raw stage, exactly mirroring `SerializerSchemas`'/`MessageSchemas`' own split from `ExtractedSerializers`/`ExtractedMessages`: an edit that touches neither a requested closed set nor any location the raw stage's own `LocationBag` half carries leaves the raw stage's own produced value equal to its previous run, which lets this projection report `Cached` (skipped entirely, not merely recomputed to an equal value) -- the SAME "Unchanged upstream lets a trivial downstream projection go Cached" shape this codebase already documents for `SerializerSchemas` in `GeneratorIncrementalScenariosSpec`'s scenario (a). An edit that shifts ONLY a location the raw stage's `LocationBag` half carries (for example, editing a message declared before the generic definition in the same file) still reports `Cached` at the projection -- the location shift never propagates past `.Table`. An edit that adds a new local marked implementor of the expanded protocol changes `CompilationFacts` (`Modified`) and, through it, this stage's own table (`Modified`); `ResolvedSerializers`, built from `effectiveSerializers`, then re-emits only the serializer whose own closed set changed. See `GeneratorIncrementalScenariosSpec`'s scenarios (i)/(j)/(k).
+
 ### 20. The Envelope Payload Attribute Is Removed: object Is The Boundary
 
 `[AkkaEnvelopePayload]` is removed. A property typed `object`, or `object?`, is now the serializer boundary. The generator writes it as the envelope frame: serializer id, manifest, bytes. The static type of the property selects the encoding. Nothing else does.

openspec/changes/messagepack-sourcegen-validation/tasks.md

@@ -458,3 +458,80 @@ Decision 19 addendum).
       freshly compiled, distinct reference over identical source -- and neutralizes an incidental
       confound in the test harness itself (its own base reference set independently qualifies the
       executing test assembly for this same walk) by warming that entry before measuring
+
+## 16. ManifestPrefix Expansion Hoisted To A Per-Compilation Stage (S7)
+
+Stacks on F3 (`feature/serialization-v2-implementor-walk`, commit `ab82136da`). Closes the
+caching-granularity gap 15's own intro text and design.md's F3 addendum both flagged and
+deliberately deferred: `ManifestPrefix` expansion (Decision 18) still ran its own closed-set walk
+and construction/schema extraction once per registration, inside the per-node serializer
+extraction transform, instead of sharing `CompilationFacts`' own cached buckets the way top-level
+dispatch and the implicit-union rules already do. Behavior-neutral: no new diagnostic, no changed
+diagnostic text, no changed generated output for any existing fixture.
+
+- [x] 16.1 Replace `ExpandClosedGenericRegistration`/`TryGetClosedSetMembers`/`AddDiscoveredClosedSetMembers`/
+      `ComputeLocalMarkedProtocolImplementors`/`EnumerateReferencedAssemblyMarkedImplementors`
+      (`AkkaSerializerGenerator.Extraction.cs`) with `BuildPrefixExpansionSpec`/`TryClassifyPrefixArgumentPosition`:
+      classifies each `ManifestPrefix` target's own type-argument position from the argument's own
+      symbol alone (no walk of the compilation's declared types), recording a light
+      `PrefixExpansionSpec` (`SerializerInfo.PrefixExpansions`) for a registration with at least one
+      protocol- or `[AkkaUnion]`-flavored position. An invalid target, an unresolvable fixed
+      argument, or no closed-set-flavored position at all is still resolved as an AKKASG040 error
+      entirely in the transform, exactly as before -- none of those depend on a whole-compilation
+      walk. `ImplementsOrDerivesFromClosedSetKey` (`AkkaSerializerGenerator.Facts.cs`), whose only
+      callers were the two removed walks, is deleted alongside them
+- [x] 16.2 Add the S7 expansion stage, `ComputeClosedGenericExpansions` (`AkkaSerializerGenerator.Expansion.cs`),
+      a per-compilation stage next to `CompilationFacts`/`ComputeMetadataSchemas`: consumes the
+      collected serializers' own `PrefixExpansions` plus `CompilationFacts`' `LocalMarkedImplementorsByClosedSetKey`/
+      `ReferencedAssemblyImplementorsByProtocol` buckets, re-resolves each candidate `TypeKey` back
+      to a symbol (`ResolveTypeSymbol`, recursing through `TypeKey.TypeArguments` for a candidate
+      that is itself a closed generic construction -- `Compilation.GetTypeByMetadataName` alone only
+      ever resolves a type's own open definition), runs the same cartesian product and manifest
+      formula F2 ran inline, and extracts each construction's schema through the same
+      `ExtractMessageCore`. `ComputeDistinctClosedSetKeys` (`AkkaSerializerGenerator.Facts.cs`) is
+      extended to also request every `PrefixExpansionSpec`'s own discovered-mode keys, so
+      `CompilationFacts`' shared buckets already cover them. Tracking name
+      `TrackingNames.ClosedGenericExpansions`, added to `TrackingNames.All`
+      (`GeneratorIncrementalScenariosSpec`'s stage-count pin updated 9 -> 10)
+- [x] 16.3 Merge the expansion stage's own table back into each `SerializerInfo` with ONE function,
+      `SerializerInfo.WithClosedGenericExpansion` (called from `MergeSerializerClosedGenericExpansions`,
+      producing `effectiveSerializers`, which `ComputeMetadataSchemas`, the per-serializer resolve
+      stage, and every diagnostics output now combine instead of the raw collected array), splicing
+      each `PrefixExpansionGroup` back into `ClosedGenericRegistrations`/`ClosedGenericSchemas` at
+      the exact insertion point (`PrefixExpansionSpec.RegistrationInsertionIndex`/`SchemaInsertionIndex`,
+      `SpliceExpansionGroups<T>`) F2/F3's own inline, per-attribute expansion left them at -- an
+      append-only first version of this merge reordered the golden corpus's own dispatch switch and
+      failed the byte-identical gate
+- [x] 16.4 Redirect `MessageTypeLocationKey` (`AkkaSerializerGenerator.Locations.cs`) to resolve an
+      expansion member's own type-level location through `ClosedGenericRegistrationInfo.ExpansionGroup`
+      (the base attribute's own location entry `BuildSerializerLocationBag` already records) instead
+      of a per-member entry, so the expansion stage needs no attribute-location plumbing of its own;
+      field-level locations for an expanded member's own `[AkkaField]` properties still resolve
+      through `ExtractMessageCore`'s existing inlined capture, carried in the expansion stage's own
+      raw `LocationBag` half (`ExpandedPrefixRegistrations`) and merged into `allLocations`
+- [x] 16.5 Add three caching-proof scenarios to `GeneratorIncrementalScenariosSpec.cs` over a new
+      fixture with a `ManifestPrefix` registration (scenarios (i)/(j)/(k)): an unrelated edit and a
+      whitespace edit inside a message both leave `ResolvedSerializers` `Cached` and re-emit nothing
+      (the expansion stage's own tracked, table-only projection reports `Cached` when its raw input
+      compares equal -- the same "Unchanged upstream lets a trivial downstream projection go
+      Cached" shape `SerializerSchemas`/`MessageSchemas` already document -- or `Unchanged` when a
+      location-only shift forces the raw stage to rerun but its own table half still compares
+      equal); adding a new local marked implementor of the expanded protocol marks `CompilationFacts`
+      and the expansion stage `Modified` and re-emits only the serializer whose closed set changed
+- [x] 16.6 Add a `PrefixVariant` corpus variant to `SourceGeneratorBenchmarks`/`SourceGeneratorBenchmarkCorpus`
+      (`src/benchmark/Akka.Benchmarks/Serialization/SourceGeneratorBenchmarks.cs`): one serializer
+      additionally registers `Envelope0<IProtocol0>` with `ManifestPrefix`, expanding over all 100 of
+      its own protocol messages. Measured against `ab82136da` (ShortRun, same machine, back to
+      back): the existing (non-prefix) corpus lands within ~1% on allocations for every row; the
+      prefix variant's rows that do not force a re-emission (trailing whitespace/comment edit,
+      unrelated file edited) drop ~8-9% on allocations, and the fresh/cold run drops ~2% (one fewer
+      redundant walk over the same closed set `CompilationFacts` already covers for top-level
+      dispatch); the field-rename row drops a smaller ~3% because it still forces a genuine
+      re-emission of the owning serializer's file (the renamed message's own field is part of the
+      emitted text), independent of this hoist. Wall-clock (`Mean`) numbers were noisy under
+      `ShortRun`'s three iterations on a shared machine; `Allocated` (deterministic per run) is the
+      metric actually compared
+- [x] 16.7 Add design.md's own S7 addendum under Decision 18: what moved, where it lives now, the
+      merge point, why an append-only first version of the merge failed the byte-identical gate and
+      needed a splice instead, the `ResolveTypeSymbol` fix for a nested closed-generic candidate, and
+      the run-reason shape for the three new caching-proof scenarios