[PLAN-0002] Milestone 2: Core Composition Engine¶
Status: Done
Implements: ADR-0010 (composition request/descriptor, synchronous provider pipeline, failure semantics, diagnostics tracing), ADR-0011 (scope, shared values, recursion detection, CreateMany semantics), ADR-0012 (path identity, deterministic random forking, CreateMany seed derivation), ADR-0013 (collection generation semantics), ADR-0014 (reflection-free collection dispatch via generator-emitted plans)
These five ADRs supersede the plan's original ADR-0007/0008/0009 basis after a deep design review of this plan's first draft — see each ADR's Links section for the supersession chain. ADR-0014 was added mid-Phase-2 (PR #11 review) to hold the collection-dispatch redesign originally recorded as ADR-0010's Amendment 3.
Goal¶
composer.Create<Customer>() and composer.CreateMany<Customer>(3) resolve a typical object graph (built-in primitives, enums, nullable value types, common collections, nested composable types via generated plans) through the real 9-stage resolution pipeline (docs/architecture.md), deterministically from a seed, with shared values reused correctly within one composition graph, recursive graphs failing with a readable diagnostic that names the cycle's edges instead of a stack overflow, and provider precedence covered by tests — per docs/mvp.md's Milestone 2 exit criteria.
Scope¶
Per docs/mvp.md's Milestone 2 section, mapped onto ADR-0010/0011/0012/ 0013's concrete shape:
- The real
CompositionContext(replacing Milestone 1'sPlaceholderCompositionContext), the publicCompositionRequestDescriptorgenerated code passes, and the internalCompositionRequestthe context expands it into (ADR-0010). - The hybrid pipeline model: context-owned deterministic stages (explicit values, shared/scoped values, exact registrations, generated-plan dispatch, diagnostic failure) plus extensible
ICompositionProvidercollection stages (profile rules, semantic providers, test-double providers, built-in providers) — only the built-in-providers stage has anything registered in Milestone 2 (ADR-0010). CompositionResult(NotHandled/Successfor ordinary providers; authoritativeFailurereserved to context-owned stages).CompositionPath(structuredPathSegmentchain — constructor parameter/member name, collection index, dictionary key/value role) and a distinct active-construction-frame stack for recursion detection (ADR-0011, ADR-0012).IRandomSource— rootCompositionSeed, FNV-1a-keyed structural forking overPathSegmentdata, a Compono-owned PRNG (notSystem.Random) (ADR-0012).- Built-in value providers for every type
docs/mvp.mdlists (string,bool, integral/floating-point types,decimal,Guid,DateTime,DateTimeOffset,DateOnly,TimeOnly,TimeSpan, enums, nullable value types, arrays,List<T>,IReadOnlyList<T>,HashSet<T>,Dictionary<TKey, TValue>) — this list may shrink if implementation complexity threatens the milestone, perdocs/mvp.md. Collection providers built against ADR-0013's semantics (default size 3, bounded duplicate-value retry for bothDictionary<TKey, ...>keys andHashSet<T>elements, no ordering guarantee forHashSet<T>/Dictionary<TKey, TValue>), dispatching element/key/value resolution through a cached generic-delegate bridge rather than per-value reflection (ADR-0010's amendment). CompositionScope(one per root composition operation, type-keyed sharing) and an internal, type-keyed exact-registration store the pipeline's stage 3 queries (mechanism only — no public builder API; see ADR-0011's scope note).- The allocation-free-on-success diagnostics trace buffer (ADR-0010) and the structured diagnostic it materializes on failure.
Composer.Create<T>()rewired onto the realCompositionContext(replacing the Milestone 1 placeholder), plus the newComposer.CreateMany<T>(count).
Explicitly deferred (later milestones per docs/mvp.md):
- Any public configuration surface —
Composer.Create(builder => ...),.Register(),.AddProfile(), profiles, collection-size configuration — all Milestone 3. Milestone 2 exercises registrations/scope through an internal test seam only (ADR-0011). - The
[Shared]attribute and any xUnit-specific composition context — Milestone 4. Milestone 2 builds the scope mechanism[Shared]will attach to, not the attribute itself. - NSubstitute/Bogus providers — Milestones ⅚. Their pipeline stages (⅘/6) exist and run as empty provider collections in Milestone 2.
- Name/qualifier-keyed sharing, semantic hints, custom-attribute-based request metadata, generic context, requested lifetime, "test double acceptable" — all deferred per ADR-0010/0011 until a later milestone has a concrete consumer.
- An async provider contract — explicitly out of scope per ADR-0010; add only if a real need appears, as a distinct opt-in contract.
- Promoting any internal engine type (
CompositionRequest,ICompositionProvider,CompositionResult,IRandomSource) topublic— deferred until the milestone that actually gives it a consumer (ADR-0010's Visibility decision).
Execution Flow: composer.Create<Customer>()¶
Written out once, in full, so "does the root type go through the same pipeline as nested types" and "how does a generated plan avoid recursively redispatching itself" have one unambiguous answer instead of being inferred per-phase. Assume:
public sealed record Customer(string FirstName, string LastName, Address HomeAddress);
public sealed record Address(string Street, string City);
- Root request creation.
Composer.Create<Customer>()calls the internalCompositionContext.ResolveRoot<Customer>()— a distinct entry point from the public, descriptor-basedResolve<T>()generated code uses (ADR-0010's amendment). There is no root case onCompositionRequestDescriptor/CompositionRequestKind— a root request has no name and isn't generator-emitted, so it never needs the public member-request shape at all.ResolveRoot<T>()constructs the internalCompositionRequestdirectly (RequestedType = typeof(Customer), root path node, no segment) and hands off to the same private pipeline-execution method the descriptor path uses. - Scope and seed creation. A fresh
CompositionScope(empty, per ADR-0011 — one per root composition operation), a rootCompositionSeed(explicit if one was configured — no builder exists yet in Milestone 2, so this is always the generated case for now — or generated once for this call), an empty rootCompositionPath(just theCustomernode, no segment), an empty active-construction-frame stack, and a fresh (empty) diagnostics trace buffer are all created together and held by one newCompositionContextinstance scoped to this singleCreate<T>()call. - Root resolution — same pipeline as any nested type.
ResolveRoot<Customer>()and a nestedResolve<Address>()call insideCustomer's own generated plan both funnel into the exact same private pipeline-execution method — the entry point differs (internal root call versus public descriptor expansion), but the pipeline they run is identical. There is no special-cased root path: stages 1–7 are tried in order (explicit value → shared/scoped → registration → profile → semantic → test-double → built-in), and for an ordinary domain type likeCustomerall seven returnNotHandled(nothing claims a plain record type), falling through to stage 8. - Generated-plan discovery. Stage 8 reads
PlanCache<Customer>.Instance(ADR-0004's direct field read). It's non-null (the generator produced a plan forCustomer, reachable from this veryCreate<Customer>()call site). Before invoking it, the context checks the active-construction-frame stack fortypeof(Customer)— empty, no cycle — pushes aCustomerframe, and proceeds. - Invocation of
ICompositionPlan<Customer>.PlanCache<Customer>.Instance.Compose(context)runs. For each constructor parameter, generated code callscontext.Resolve<TParam>(descriptor)with a descriptor naming that parameter (Kind = ConstructorParameter,Ordinal = 0,Name = "firstName", etc. —Ordinalis the identity the fork key and active-construction bookkeeping actually use;Nameis carried for diagnostic display only, per ADR-0012's amendment) — never touchingCompositionPathor the frame stack directly. - Nested
Resolve<T>()calls. ForfirstName, the context expands the descriptor into an internalCompositionRequest, appends aConstructorParameter(Ordinal: 0, Name: "firstName")segment to the path (nowCustomer → firstName, hashed onOrdinal), and runs the same 9-stage pipeline against it, fresh, as its own independent evaluation. On return (success or failure), the appended segment is popped in afinally— the path is back to justCustomerbefore the next parameter is resolved. - Provider resolution. For
firstName(string), stage 7's built-in string provider claims it (Success) — stages 1–6 all declined. No recursion check is involved at all for this request, because it never reaches stage 8 (strings have no generated plan).lastNameresolves identically. - Nested generated-plan dispatch. For
homeAddress(Address), the same expansion happens (path becomesCustomer → homeAddress), stages 1–7 decline, stage 8 findsPlanCache<Address>.Instance, checks the frame stack (Customeris active,Addressis not — no cycle), pushes anAddressframe, and invokesPlanCache<Address>.Instance.Compose(context). This is a genuinely independent, nested invocation of the sameResolve<T>machinery — notCustomer's plan calling itself.Address's own constructor parameters (street,city) resolve the same way, with the path nowCustomer → homeAddress → street/→ city. WhenAddress'sComposereturns, its frame is popped infinally. - Path push/pop behavior, summarized. Because
Resolve<T>is an ordinary synchronous method calling itself recursively (through generated plans calling back into it), path segments and construction frames both push on entry and pop infinallyon exit — entirely as a side effect of the call stack. Neither generated code nor any provider ever manipulates either structure directly; there is no way to "forget" a pop, because there's no manual bookkeeping to forget. - Failure propagation. If, say,
city's resolution fails (every stage returnsNotHandled, reaching stage 9's diagnostic failure), thatResolve<string>call — which must return astring, not a wrapped result, sinceICompositionPlan<T>.Composereturns a plainT— throws aCompositionExceptioncarrying the materialized diagnostic (path at time of failure:Customer → homeAddress → city; seed; the trace-buffer slice for this failed request). That exception propagates up throughAddress'sCompose(unwinding itsfinallyblocks — theAddressframe and thehomeAddresssegment both pop as the stack unwinds), throughCustomer'sCompose(Customer's frame and the root pop too), and out ofComposer.Create<Customer>()to the caller. The pipeline's internalNotHandled/Successreturn-value convention (ADR-0010) never itself throws — only the outward-facingResolve<T>/Create<T>()boundary converts a terminal non-Successoutcome into the thrownCompositionExceptiondocs/public-api.mdalready shows consumers catching.
CreateMany<T>(count) repeats steps 1–10 count times, each with its own fresh scope/path/frame-stack/trace-buffer (ADR-0011's "fresh scope per item"), differing only in step 2: each item's root CompositionSeed is forked from the batch's root seed via "CreateMany" then the item's index (ADR-0012), rather than being the batch's root seed directly.
Phases¶
Ordered per the user's explicit preference during design review: seed, path identity, and the random source come before built-in providers are implemented, so providers are written against their real, final randomness source from the start — never against ad hoc randomness that a later phase has to rip out and rewire. Each phase either unblocks the next or closes a real gap against the exit criteria. Each phase ships as its own PR — don't bundle phases into one diff even if more than one is finished by the time a PR gets opened.
Phase 0 — Composition request, descriptor, and pipeline skeleton (Done)¶
Replaces Milestone 1's PlaceholderCompositionContext with the real CompositionContext, wired to the hybrid pipeline model (context-owned stages + extensible-provider collections). Stage 8 (generated-plan dispatch via PlanCache<T>) is already fully functional from Milestone 1 — this phase doesn't add it, it wires the rest of the pipeline around it. Stages 1–3 (explicit values, shared/scoped, exact registrations) are context-owned checks that are always NotHandled in this phase (no scope or registration store exists until Phase 3); stages 4–6 (profile/semantic/test-double) are provider collections that are always empty until Milestones 3/6/5 respectively; stage 7 (built-in providers) is an empty collection until Phase 2. So in this phase, a request resolves successfully only for a type with a generated plan reachable from a Create<T>()/CreateMany<T>() call site (Milestone 1 behavior, now running through the real pipeline instead of the placeholder) — everything else correctly reaches stage 9's diagnostic failure until later phases populate the stages that would have handled it.
-
CompositionRequestDescriptor(public readonly struct— plain, not arecord struct; equality/Deconstruct/ToStringaren't part of its contract, per ADR-0010's second amendment — withKind,Ordinal,Name,Nullability;Ordinalis identity,Nameis diagnostic-only) andCompositionRequestKindenum (ConstructorParameter,RequiredMember— noRootcase; the root never uses this type, seeResolveRoot<T>()below) -
CompositionRequest(internal:RequestedType,Nullability,Path,IsShared— expanded from a descriptor by the context) -
CompositionResult(internal,NotHandled/Successonly — noFailurecase an ordinary provider can construct) -
ICompositionProvider(internal, non-generic,TryCompose(CompositionRequest, ICompositionContext)) -
CompositionContextimplementingICompositionContext.Resolve<T>(in CompositionRequestDescriptor)(descriptor expansion path, used by generated code) and an internalResolveRoot<T>()(constructs the rootCompositionRequestdirectly, bypassing the descriptor entirely — ADR-0010's amendment); both funnel into one private pipeline-execution method running stages 1–9 in order - Internal test-seam constructor/factory on
CompositionContextaccepting explicit per-stage provider collections for stages 4–7 (ADR-0010's amendment), reachable fromCompono.TestsviaInternalsVisibleTo— this is how this phase's own pipeline-order tests inject fakeICompositionProvidertest doubles into a specific stage without any public configuration surface existing - Rewire
Composer.Create<T>()offPlaceholderCompositionContextonto the realCompositionContext(callingResolveRoot<T>()) - Unit tests: pipeline tries stages in documented order (using the internal test-seam factory to inject fake providers at specific stages and assert call order); a stage returning
Successshort-circuits later stages; a type with a generated plan resolves successfully through the real pipeline (Milestone 1 behavior preserved); a type with no generated plan and nothing else able to handle it throwsCompositionExceptionwith a structured diagnostic
Notes on what actually happened, versus what was scoped:
PathSegmentandCompositionPath(originally scoped to Phase 1) were pulled forward into this phase, minimally —CompositionRequest.Pathneeds a real type to compile, and push/pop needed to exist forCompositionContext.Resolve<T>'s recursive call structure to be correct from the start. What's implemented now is the structural chain only (an immutable, parent-pointing linked list — push/pop composes with the call stack,Type/PathSegmentper node); the FNV-1a random-fork key derivation from that structure is still genuinely Phase 1 scope and hasn't been touched.CompositionExceptionexists now as a minimalExceptionsubclass (constructor message only, naming the unresolvable type) — the full structured diagnostic (path display string, materialized provider trace, seed, remediation message) is still Phase 4 scope; this phase only needed a thrown exception at the pipeline's terminal stage, not the final diagnostic shape.- The active-construction-frame stack and stages 1–3's real scope/registration checks are not implemented yet, exactly as scoped — every request in this phase either resolves via an extensible provider (stages 4–7, all empty except whatever a test injects) or stage 8's
PlanCache<T>dispatch, or reaches stage 9's failure. - Generator-integration testing gap discovered during implementation:
Compono.Testsdoesn't referenceCompono.Generatorsas an analyzer (onlyCompono.csprojitself does — same as Milestone 1, per PLAN-0001's Phase 5 notes, which verified real generator dispatch via an external published package instead). Phase 0's tests exercise stage 8 dispatch by settingPlanCache<T>.Instancedirectly to a hand-written fakeICompositionPlan<T>— this is the correct unit-test-level boundary regardless (isolatingCompositionContext's dispatch logic from generator behavior, whichCompono.Generators.Testsalready covers separately), not a workaround for a gap. - Regression found and fixed mid-phase: changing
ICompositionContext.Resolve<T>'s public signature brokeCompono.Generators— its Scriban template (CompositionPlan.scriban) still emitted the Milestone 1 call shape (context.Resolve<T>(Nullability.NotNullable)), so generated code no longer compiled and 46Compono.Generators.Testssnapshot tests failed. Fixed in the same change, since a generator whose output doesn't compile against the runtime it targets isn't a deferred cleanup item: CompositionPlanEmitter's model now includes each constructor parameter'sName(previously dropped from the anonymous projection); the template emitsnew global::Compono.CompositionRequestDescriptor(Kind, ordinal, "name", nullability)for both constructor parameters and required members, using Scriban'sfor.indexas the ordinal (parameters: the selected constructor's own parameter order, unchanged; required members: see next bullet).RequiredMemberCollector's ordering was derived-type-before-base (EnumerateTypeAndBaseswalkedtypebeforetype.BaseType, keeping first-occurrence-wins for an override) — the opposite of ADR-0012's amendment 2 canonical algorithm (base-to-derived, base members get lower ordinals). Rewritten to walk base-to-derived, still deduping an override to its most-derived symbol (correct accessibility/type info) but assigning the ordinal from the name's first (base-most) appearance, with declaration order preserved within each type. Covered by a new test,RequiredMembersOnBaseAndDerivedType_BaseOrdinalsPrecedeDerivedInDeclarationOrder.- All 30+ existing
.verified.csgenerator snapshots were regenerated (content reviewed for correctness, e.g. the override-dedup case still emits its required member exactly once) — a normal Verify-workflow consequence of a template change, not scope creep. - Full suite green after the fix: 86
Compono.Generators.Tests(43 × 2 target frameworks), 20Compono.Tests,Compono.Benchmarksbuilds clean.
Phase 1 — Seed, path identity, and forkable random source (Done)¶
Built before any provider needs randomness, per the design review — no provider in Phase 2 is ever written against temporary/ad hoc randomness.
Reproducibility contract, stated once here since every task below implements it: a resolved value's random identity is derived exclusively from its structural position in the composition graph — the chain of PathSegment tags and Ordinal/Index values from the root — and never from CompositionRequest.RequestedType or any other type identity (docs/adr/0012-...'s second amendment). This is a guarantee to test directly, not an incidental property of the implementation.
-
PathSegmenthierarchy (ConstructorParameter(int Ordinal, string Name),RequiredMember(int Ordinal, string Name),CollectionElement(int Index),DictionaryKey(int Index),DictionaryValue(int Index)) andCompositionPath(an immutable, parent-pointing structural chain, wired intoCompositionContext.Resolve<T>'s push-on-entry/ pop-in-finallybehavior) — pulled forward into Phase 0, sinceCompositionRequest.Pathneeded a real type to compile; only the structural chain exists so far -
IRandomSourcefork-key derivation actually consumingPathSegment'sOrdinal/Index(identity) via FNV-1a — the hashing this phase is actually about, per ADR-0012's amendment;Namestays diagnostic-only - Required-member ordinal assignment in
Compono.Generators, per ADR-0012's second amendment's canonical algorithm — already implemented in Phase 0 as an unscoped fix (see Phase 0's Notes:RequiredMemberCollectorwas rewritten base-to-derived while fixing the Scriban-template regression), confirmed against the canonical algorithm text and left unchanged this phase -
CompositionSeed(root seed — explicit value or generated once per root composition operation) -
internal T Composer.CreateRootForTesting<T>(CompositionSeed seed)andinternal IReadOnlyList<T> Composer.CreateManyForTesting<T>(int count, CompositionSeed seed)(ADR-0011's second amendment — replaces the earlier, ambiguousCreateWithSeedname) — the internal test seams this phase's own determinism tests (and Phase 4'sCreateManystability/end-to-end tests) use to exercise the realComposer/CompositionContextflow with an explicit seed for exactly one root operation or one batch, since the publicWithSeed(...)builder doesn't exist until Milestone 3 -
IRandomSource(internal) with structural key-based forking: FNV-1a hash of eachPathSegment's tag +Ordinal/Indexpayload (neverName, never a formatted display string), combined with parent state to derive child state; type identity (CompositionPathNode.RequestedType) never feeds this hash (ADR-0012's amendment) — only ordinary, process-localTypeequality is used elsewhere (provider dispatch, active-construction frames) - The Compono-owned PRNG value generator (not
System.Random) backingIRandomSource— SplitMix64, used both to derive a node's value-stream state from its fork state and to advance that stream; seeRandomSource's remarks for why this is SplitMix64 alone rather than ADR-0009's "SplitMix64 feeding a xoshiro-family generator" phrasing (a deliberate scope simplification — Phase 2's built-in providers are the first real consumer of generated values, and swapping the generator later doesn't changeIRandomSource's internal contract) - A display-string derivation from
CompositionPath(using each segment'sName), for diagnostics only — never consumed by the hashing path above - Unit tests (via
Composer.CreateRootForTesting): same seed + same path produces identical output across two independent resolutions; two sibling requests at the same path depth with differentOrdinals (e.g. two same-typed constructor parameters) fork independently; renaming a constructor parameter/required member (no reordering) does not change its derived value, but reordering does; changing an unrelated member elsewhere in the graph doesn't change an already-resolved value's output - Tag-collision test (ADR-0012's second amendment): fork all five
PathSegmentkinds atOrdinal/Index = 0from the same parent state (ConstructorParameter(0, "x"),RequiredMember(0, "x"),CollectionElement(0),DictionaryKey(0),DictionaryValue(0)) and assert all five produce pairwise-distinct output — direct proof the per-kind tag byte actually discriminates kinds, not an inference from the design alone
Notes on what actually happened, versus what was scoped:
RandomSourcekeeps two independent 64-bit states per node: a fixed "fork state" derived purely from the structural path (only ever used to derive children, never mutated) and a "value state" seeded from the fork state and advanced byNextUInt64(). This wasn't spelled out in the plan's task list but follows directly from the reproducibility contract: without the split, how many random values a node's own provider draws would perturb its children's derived state, which the contract explicitly forbids.Fnv1a.Combinefolds the parent's state in as ordinary input bytes, always starting from FNV-1a's real offset basis, rather than using the parent's state as the initial hash accumulator directly. The first version did the latter and had a real bug caught in PR #10 review: state 0, theConstructorParametertag (0), and an all-zero ordinal payload all mix to 0, so a seed of0followed by any number ofConstructorParameter(0, ...)forks collapsed every one of those distinct structural positions to the same derived state — a silent violation of ADR-0012's independent-forking guarantee. Fixed by folding the state in as data instead; regression test:Fork_DoesNotCollapseToAFixedZeroState_FromAZeroSeedAtOrdinalZero.RandomSource.Forkencodes each segment'sOrdinal/Indexas fixed big-endian bytes (BinaryPrimitives.WriteInt32BigEndian), notBitConverter.GetBytes—BitConverteruses host machine endianness, which would silently vary the fork key's byte sequence on a big-endian host and break "same seed, same output" across machines. Not called out explicitly in the plan, but a direct consequence of the reproducibility contract already stated there.CompositionContext.ResolveCore's pre-existing null-_pathcheck (used to detect "this is the root call") already had to double as the random-source root check too — a descriptor-basedResolve<T>called directly on a fresh context with no precedingResolveRoot<T>()call (only reachable fromCompositionContextTests' own unit test exercising the descriptor path in isolation, never from generated code) hits this case. Handled by keying both_path's and_random's root-vs-nested branch on the same_path is nullcheck, rather than onsegment is nullfor_random— otherwise that test throws aNullReferenceException.- No changes to
CompositionRequestthis phase — Phase 2's real built-in providers are the first actual consumer ofIRandomSource, so threading it throughCompositionRequest(the natural place, alongsidePath) is deferred to when Phase 2 needs it, per the "don't design for hypothetical future requirements" standard. Phase 1's own determinism tests reach the current node's random source instead via an internalCompositionContext.Randomtest-observability property, consumed by a capturingICompositionPlan<T>test double (CompositionRandomIntegrationTests).
Phase 2 — Built-in value providers and collections (Done, PR #11 merged)¶
Every provider here is written directly against Phase 1's real IRandomSource — there is no ad hoc/temporary randomness at any point in this plan.
Corrected mid-phase, before implementation started, per ADR-0010's Amendment 3: the originally-scoped reflection-based collection-dispatch bridge (MakeGenericMethod + CreateDelegate) is removed — it violated ADR-0001's no-reflection-by-default rule, caught before any code was written against it. Collections are now built by Compono.Generators-emitted, strongly typed collection plans (no runtime reflection anywhere in this phase), per ADR-0014. The task list below reflects the corrected shape.
- Primitive/simple-type providers (
string,bool, integral types, floating-point types,decimal,Guid,DateTime,DateTimeOffset,DateOnly,TimeOnly,TimeSpan) — ordinaryICompositionProviders in stage 7's provider collection, unchanged from the plan's original shape - Enum provider (random valid enum member, via
IRandomSource) — ordinary stage-7 provider, unchanged - Nullable value type provider (composes the underlying type when it's a primitive/enum/recognized BCL value type; nullable-generation default beyond that is a still-open
docs/mvp.mditem this phase doesn't resolve — see ADR-0013) — ordinary stage-7 provider, unchanged.Nullable<T>over any other (composable, custom-struct)Tis not a stage-7 provider case at all — round 13 fixedLeafTypeClassifierto fall through to ordinary composable-type handling for thatTinstead of silently treating everyNullable<T>as resolved regardless of the underlying type; see the round 13 note below. -
CompositionRequestKindgainsCollectionElement/DictionaryKey/DictionaryValue;CompositionContext.Resolve<TValue>'s descriptor-to-segment switch extends to cover them (Ordinalmaps to the segment'sIndex,Nameunused) — ADR-0010 Amendment 3 -
CollectionPlanCache<T>(public, mirrorsPlanCache<T>exactly — ADR-0004's zero-overhead closed-generic-field dispatch shape) and theCompositionContext.ResolveCore<TValue>direct field-read check for it, positioned at stage 7 immediately after the ordinary built-in provider collection declines and before stage 8'sPlanCache<TValue>check -
UniqueValueResolver(public, generic, reflection-free): the bounded duplicate-value retry helper (ADR-0013) generatedHashSet<T>/Dictionary<TKey, ...>collection plans call once per element/key position; fork-derived deterministic retry indices (attempt 0 = the position unchanged; each retry attempt forks from a disjoint, deterministic index), boundedMaxAttempts, exhaustion reported by the generated plan throwingCompositionExceptionnaming the element/key type and requested count -
Compono.Generators:TransitiveClosureWalkerextended to recognize the five ADR-0013 shapes (T[],List<T>,IReadOnlyList<T>,HashSet<T>,Dictionary<TKey, TValue>) wherever they appear in the walked graph, including nested inside another collection; a recognized shape is not walked as an ordinary composable type — its element/key type(s) feed back into the same eligibility walk instead -
Compono.Generators: a new collection-plan emitter + template emitting onefile-scopedICompositionPlan<TCollection>per distinct closed collection type reached, each registering itself intoCollectionPlanCache<TCollection>.Instancevia a module initializer (same registration shape as an ordinary composition plan) — default size 3 (ADR-0013), each element/key/value its ownResolve<T>()call via aCollectionElement/DictionaryKey/DictionaryValuedescriptor;HashSet<T>/Dictionary<TKey, ...>plans callUniqueValueResolver.TryResolve<TValue>per element/key; unsupported collection shapes are never classified as collections in the first place, so they fall through to ordinary composable-type handling (and, having no usable constructor, to that path's existing diagnostics) exactly like any other unhandled shape — no distinct "unsupported collection" error path, per ADR-0013's unchanged Decision Outcome; no ordering guarantee documented or tested forHashSet<T>/Dictionary<TKey, TValue> - Unit tests per provider type (
PrimitiveValueProviderTests,EnumValueProviderTests,NullableValueProviderTests), plus a test confirmingCollectionPlanCachedispatch only applies after the built-in provider collection and registration/profile/semantic/ test-double stages have declined, and after it wins over stage 8'sPlanCache<TValue>(CollectionPlanCacheDispatchTests); a duplicate-value retry/retry-exhaustion test at theUniqueValueResolverlevel, covering both a successful retry and full exhaustion (UniqueValueResolverTests) - A duplicate-value retry-exhaustion test through an actual generated
HashSet<T>/Dictionary<TKey, ...>plan against a genuinely low-cardinality element/key type, and a collection-index path-construction test (e.g.List<Address>with 3 elements) asserting each element's independent output at the runtime level — closed viaGeneratorTestHelpers.CompileAndExecute(compiles source plus real generated output to an in-memory assembly, loads it, and invokes it by reflection) andGeneratedCollectionPlanExecutionTests'sHashSetRetryExhaustion_ThrowsCompositionException_ThroughARealDispatchedPlanandListElements_EachGetsIndependentOutput_ThroughARealDispatchedPlan;UniqueValueResolverTestscovers the retry/exhaustion algorithm directly (with a stub context), and the tag-collision/ structural-independence guaranteeCollectionElement(i)relies on is already covered by Phase 1'sRandomSourceTests/CompositionRandomIntegrationTests— this item added the remaining end-to-end runtime assertion through a real dispatched collection plan -
Compono.Generators.Tests: snapshot coverage for at least one generated plan per collection shape (array,List<T>,IReadOnlyList<T>,HashSet<T>,Dictionary<TKey, TValue>), plus a nested-collection case (List<List<int>>) proving the walker recurses into a collection's element type correctly (CollectionPlanVerifyTests)
Notes on what actually happened, versus what was scoped:
Compono.Generators's discovery pipeline (CreateInvocationDiscovery,ComposableAttributeDiscovery,ComposedTypeAnalyzer,TransitiveClosureWalker) changed its return shape fromEquatableArray<DiscoveredTypeInfo>to a newTransitiveClosureResult(Types+Collections) throughout, not just at the walker — every discovery entry point needed to carry collections alongside types forComponoIncrementalGeneratorto collect/dedupe/emit both. Not called out explicitly in the corrected task list above, but a direct consequence of "the walker discovers collections too."- Collection-shape recognition (
CollectionWellKnownTypes) is a distinct type fromWellKnownTypes, not an extension of its enum table — that type's debug self-check (AssertEnumAndTableInSync) assumes a metadata name derivable from the enum member's own name via a simple underscore-to-dot transform, which doesn't produce the generic-arity backtick suffix (`1/`2) closed BCL generic types need. Adding generic-shape entries there would have required changing that self-check's transform; a small dedicated cache sidesteps it entirely. - A real C# parser quirk, not caught until actually compiling generated output:
global::Some.Memberis not parsed as a qualified-alias member when it's the first token inside a string-interpolation hole ($"...{global::Foo.Bar}..."failsCS0103, "the name 'global' does not exist in the current context") — confirmed with a minimal repro outside this repo. Fixed by parenthesizing ({(global::Foo.Bar)}) inCollectionPlan.scriban's two retry-exhaustion diagnostic messages, the only places aglobal::-qualified reference appears as the first token of an interpolation hole in generated code. UniqueValueResolver's retry-index encoding (RetryIndex) uses a negative index for every retry attempt (attempt >= 1), disjoint by construction from any position's non-negative base index — simpler than an arithmetic scheme that could theoretically coincide with another position's own index, and avoids a false "this looks like position N's own value" reading in a fork-key trace.- Collection dispatch is a hybrid within stage 7, not a third
Providercollection:CompositionContext.ResolveCore<TValue>tries the ordinary_builtInProviderslist first (primitives/enum/nullable, unchanged), then readsCollectionPlanCache<TValue>.Instancedirectly — the same reasoning ADR-0010 already used to keep stage 8'sPlanCache<T>off theICompositionProviderinterface applies identically here. - The same-closed-collection-type-discovered-twice-with-different- nullability case (the collection-shape analogue of
CMP0010's conflicting-composition-metadata check for ordinary types) was initially shipped as first-discovered-wins with no diagnostic, flagged during PR #11 review, and fixed in the same PR:DiscoveredCollectionInfogained aDiagnosticsfield, andComponoIncrementalGenerator's collection dedupe now detects a genuine disagreement within aFullyQualifiedCollectionTypeNamegroup and reports CMP0011 (mirroringCMP0010's synthetic-conflict-entry shape exactly) instead of picking one discovery arbitrarily — seeCollectionPlanVerifyTests.SameClosedListReachedWithDifferentElementNullability_ReportsConflictDiagnostic. - Two more PR #11 review findings, both fixed in the same PR: (1)
LeafTypeClassifiernever gainedDateOnly/TimeOnly— those two types are new in Phase 2's built-in type list, but the generator's provider-resolved classification (Milestone 1 code) wasn't updated alongsidePrimitiveValueProvider, so a composed type with aDateOnly/TimeOnlymember failed constructor selection instead of reaching the new provider. Fixed by adding both toWellKnownTypeData/LeafTypeClassifier.IsRecognizedBclValueType. (2)LeafTypeClassifier.IsBuiltInSimpleTypealready classifiedchar/nint/nuintas provider-resolved (Milestone 1), butPrimitiveValueProvider's factory table never covered them, so a generated plan referencing any of the three compiled fine and then always failed at runtime withCompositionException. Fixed by adding factories for all three. - A second round of PR #11 review caught two more real gaps, both fixed in the same PR: (1) root-type discovery never applied
LeafTypeClassifier/collection classification to the requested type itself — only nested members got that check.Composer.Create<Guid>(),Composer.Create<string>(), andComposer.Create<List<int>>()all failed to compile (CMP0001, ambiguous constructor);Composer.Create<int>()/Composer.Create<DayOfWeek>()compiled but silently generated a deadPlanCache<T>entry that always produceddefault(T)(harmless only because stage 7's built-in provider always won first, but confusing generated output). Fixed by routing the root through the same classify-first logic as any member (TransitiveClosureWalker.EnqueueRoot), with one refinement beyond the review's own suggested fix: a root that's abstract or a delegate has no runtime provider either, so it must still reach constructor selection for its existingCMP0003diagnostic — a first pass that reusedLeafTypeClassifier.IsProviderResolvedverbatim for the root regressed three passing tests (AbstractType_ReportsDiagnostic,DelegateType_ReportsDiagnostic,ComposableAttributeOnInterface_ReportsDiagnostic) by silently skipping them instead. Fixed with a narrowerLeafTypeClassifier.IsRuntimeProviderResolved(enums, built-in simple types, recognized BCL value types only) used solely for the root check. (2)EnumValueProvidercalledEnum.GetValues(type)— an allocating call — on every resolution; cached per enum type via aConcurrentDictionary<Type, Array>instead. - A third round of PR #11 review caught the same root-classification gap had one more hole: array roots (
Composer.Create<Address[]>()) still failed to compile (CMP0006) —ComposedTypeAnalyzer.Analyze's ownrequestedType is not INamedTypeSymbolcheck runs beforeTransitiveClosureWalker.Walkis ever called, rejecting an array root regardless ofEnqueueRoot's fix, sinceIArrayTypeSymbolis never anINamedTypeSymbol. Fixed by checking collection classification inComposedTypeAnalyzer.Analyzeitself, before the named-type check, and wideningTransitiveClosureWalker.Walk/EnqueueRoot/EnqueueMember'sparentTypeparameter fromINamedTypeSymboltoITypeSymbol(defensively narrowed back at the one place — the composable-type fallback — that actually needs anINamedTypeSymbolto enqueue).CompositionPlanVerifyTests.ArrayTypeArgument_ReportsDiagnosticwas renamed toMultiDimensionalArrayTypeArgument_ReportsDiagnosticand changed to a rank-2 array (Customer[,], still genuinely unsupported —CollectionWellKnownTypesonly classifies rank-1 arrays) since its original rank-1 array premise is now correct, supported behavior. Also fixed in the same round:docs/architecture.md's stage-7 table row still described that stage as solely an orderedICompositionProvidercollection, silently stale against ADR-0010's third amendment (theCollectionPlanCache<T>hybrid dispatch) — updated to describe the actual current shape. - A fourth round of PR #11 review found two more real gaps and one pre-existing, out-of-scope property: (1)
EnumValueProviderusedEnum.GetValues(Type)— the non-generic,Type-based overload, which is annotated[RequiresDynamicCode](confirmed directly via reflection on the BCL method's attributes) and breaks under Native AOT — a real violation of ADR-0001's no-reflection-by-default rule, the same rule ADR-0010's third amendment retracted the reflection-based collection bridge over. Fixed by switching toEnum.GetValuesAsUnderlyingType(Type)+Enum.ToObject(Type, object)(neither carries the annotation, confirmed the same way) — with one subtlety caught before it shipped: a boxed underlying-type value (e.g. boxedint) unboxes correctly to a non-nullable enum type but throwsInvalidCastExceptionunboxing toNullable<TEnum>specifically, so the fix must box viaEnum.ToObject(boxed as the actual enum type), not hand back the underlying-type box directly —NullableValueProviderTests.ComposeNullableEnum_ComposesTheUnderlyingType_NeverNull(already existing) is the regression guard, and stayed green through the fix since it was never actually broken, only would have been by a more naive version of this fix. (2) A pointer/function-pointer-element rank-1 array root or member (composer.Create<int*[]>()) reached collection classification and a generated collection plan tried to emitcontext.Resolve<int*>()— a compiler error in generated code (pointer types can't be generic type arguments), confirmed directly.List<T>/HashSet<T>/Dictionary<TKey, TValue>can't have this problem (the C# compiler already rejects a pointer generic type argument before this code ever runs), so this was array-specific. Fixed inCollectionWellKnownTypes.TryClassify: a rank-1 array whose element type isIPointerTypeSymbol/IFunctionPointerTypeSymbolis left unclassified, falling through to the existing CMP0006 diagnostic path like any other unsupported shape. (3) The review's third finding —CollectionPlanCache<T>'s module initializer unconditionally overwritingInstanceacross multiple consuming assemblies in one process — was verified to be an unchanged property ofPlanCache<T>itself (Milestone 1, ADR-0004), not a defect Milestone 2 introduced;CollectionPlanCache<T>deliberately mirrorsPlanCache<T>'s exact registration shape. Deferred as a class-of-problem design question affecting both caches, not patched narrowly into just the new one — seedocs/architecture.md's Open Architectural Decisions, new "Cross-assembly plan-cache collision" entry. - A fifth round of PR #11 review found one more real gap (fixed) and correctly flagged that this phase had never actually done the required real manual verification for source-generator-facing work (
AGENTS.md's Build and test section,tasks/implement.mdstep 7) — every check so far had been generator snapshot tests plus directCompono.Tests/Compono.Generators.Testsunit tests, never a realdotnet packinto a throwaway consuming project, unlike every phase of PLAN-0001 (Milestone 1). Doing that verification is what actually caught the real gap: Nullable<T>was never added toLeafTypeClassifier— a real, previously-undiscovered bug no generator snapshot test had ever exercised (none used a nullable value type as a constructor parameter, and the existing nullable coverage was allComposer.CreateRootForTesting<T>-level runtime tests, which never go through the generator at all). Any nullable value type, root or member (int?, a nullable enum, etc.), reachedConstructorSelector, which seesNullable<T>'s two accessible constructors (the parameterless one andNullable(T value)) and reportsCMP0001ambiguous construction — caught immediately by the real consuming-project verification (Order'sPriority?constructor parameter failed to compile) before anything else did. Fixed by adding aNullable<T>case to bothLeafTypeClassifier.IsProviderResolvedandIsRuntimeProviderResolved(named.OriginalDefinition.SpecialType == SpecialType.System_Nullable_T), coveringNullable<T>regardless of whatTis — a custom struct'sNullable<T>isn't runtime-satisfiable either, butNullableValueProvideralready attempts and cleanly declines any unsupportedNullable<T>at runtime (reaching stage 9's diagnostic, correctly naming the request), so there's no compile-time distinction worth drawing between a supported and unsupportedNullable<T>here, the same reasoning already applied toEnum/built-in-simple/BCL-value-type roots. Added regression coverage:CompositionPlanVerifyTests.NullableValueTypeRootType_GeneratesNoPlan,NullableValueTypeConstructorParameter_LeftAsResolveCallNotRecursedInto.- Also fixed in the same round:
EnumValueProvider's per-enum-type cache usedConcurrentDictionary<Type, Array>, which strongly roots every enumTypeever composed for the process lifetime — a real concern for a long-lived host that loads/unloads consumer assemblies via a collectibleAssemblyLoadContext. Swapped forConditionalWeakTable<Type, Array>, whose keys don't root anything. - Real manual verification, done for the first time this phase:
dotnet pack'dComponointo a local feed (clearing both the local feed and~/.nuget/packages/componofirst, per PLAN-0001's Phase 2 stale-cache lesson) and referenced it from a genuinely separate throwaway console project. Composed a representative nested graph (Order→Address, exercising every Phase 2 built-in: primitives, an enum, a nullable enum,DateOnly, and all five collection shapes including a nestedList<List<int>>) plus four root-levelCreate<T>()calls (int,Guid,List<int>,string[]). Confirmed, in a real build/run rather than the test harness:PlanCache<Order>/PlanCache<Address>and everyCollectionPlanCache<T>slot actually populated by generated module initializers; every collection produced exactly 3 elements (the ADR-0013 default size); the nullable enum was always set, never null; and every composed value was plausible (defined enum members, non-empty strings, in-range dates). All checks passed after theNullable<T>fix above (the run before the fix failed to compile, confirming the bug was real and the fix's coverage). This satisfiestasks/implement.mdstep 7 for this phase, retroactively — should have been done alongside Phase 2's original implementation, not only once review flagged its absence. - A sixth round of PR #11 review found one more real gap: a private/protected collection element or key type (
composer.Create<List<PrivateEnum>>()called from insidePrivateEnum's own containing type) compiled fine at the call site (private members are accessible within their own containing type) but failed inside the generated collection plan withCS0122— every generated collection plan is a top-levelfile-scoped type outside any containing type, so it can never reference a private/protected element/key type even when the original call site legitimately could. Confirmed directly before fixing. Fixed by checkingcompilation.IsSymbolAccessibleWithin(elementType/keyType, compilation.Assembly)inTransitiveClosureWalker.ToDiscoveredCollectionInfo— the same accessibility APIConstructorSelector/RequiredMemberCollectoralready use for constructor/required-member accessibility — reporting a new diagnostic, CMP0012, instead of letting an inaccessible type reach emission.compilation/locationhad to be threaded throughEnqueueRoot/EnqueueMemberto reach this check (previously onlyWalk's top-level scope had them). Added regression coverage:CollectionPlanVerifyTests.PrivateElementTypeInCollectionRoot_ReportsDiagnostic. Scope note: the identical accessibility gap plausibly exists for an ordinary (non-collection) composable type's constructor-parameter type too (any generated composition plan is equally a top-level type), but constructing a legal, compiling repro for that case is significantly more constrained by C#'s own accessibility-consistency rules (CS0051/CS0053 already reject a public constructor exposing a less-accessible parameter type) — not fixed here since it wasn't what was flagged and a real repro wasn't confirmed; worth a follow-up look if it's ever hit in practice. - A seventh round of PR #11 review found a real gap in round 6's own fix: when the same inaccessible collection type was discovered from two different call sites, each
DiscoveredCollectionInfocarried the identical CMP0012 diagnostic but at a differentLocation— sinceDiagnosticInfo.EqualsincludesLocation, the two entries counted as "distinct" and the collections merge step (added for CMP0011 in round 3) incorrectly synthesized a locationless CMP0011 "conflicting nullability" diagnostic instead, erasing both real, actionable CMP0012 failures. Confirmed directly before fixing (a two-call-site repro produced exactly the wrong CMP0011). This is the exact class of bug the ordinary-type merge already guarded against (failures = distinct.Where(t => t.Diagnostics.Count > 0), present since round 3's original CMP0011 work modeled itself on it) — the guard just hadn't been ported to the newer collections merge branch when CMP0012 introduced the first real per-discovery collection failure. Fixed by adding the identical failures-preserved-before-conflict-detection branch to the collections merge inComponoIncrementalGenerator. Added regression coverage:CollectionPlanVerifyTests.SameInaccessibleCollectionFromTwoCallSites_ReportsCMP0012NotCMP0011. - An eighth round of PR #11 review found one real gap (fixed) and one legitimate process concern (also fixed, as a documentation reorganization): (1) no test compiled, loaded, and actually executed a real generated collection plan — every existing test either snapshotted/compiled generated source without running it (
CollectionPlanVerifyTestset al.), or exercised the runtime pipeline against a hand-written test double (UniqueValueResolverTests,CollectionPlanCacheDispatchTestsinCompono.Tests), so module-initializer registration, template output, and real fork-path integration regressions could all go undetected. This was already an explicitly-acknowledged open item in this plan (Phase 2's task list), not a newly-discovered gap, but review correctly pressed for it to actually close in this PR rather than stay deferred. Closed by addingGeneratorTestHelpers.CompileAndExecute(compiles source + real generated output to an in-memory assembly, loads it, and invokes a method via reflection) and a newGeneratedCollectionPlanExecutionTests: aHashSet<bool>retry-exhaustion test (bool's 2-value cardinality makes this deterministically fail regardless of randomness, exercisingUniqueValueResolver's exhaustion path through a real dispatchedHashSet<T>plan) and aList<Guid>test asserting 3 genuinely distinct elements through a real dispatchedList<T>plan. Both passed on the first run. (2) ADR-0010's Amendment 3 (the collection-dispatch-bridge retraction) should have been its own ADR, not appended in place to an already-AcceptedADR whose other decisions had already shipped in merged PRs (#9, #10) — the pre-implementation "amend in place" pattern ADR-0010's own Amendments ½ used was correct for those (nothing in the whole ADR had been built yet), but no longer fit once real code existed against parts of the ADR, even though the specific sub-decision being corrected (the reflection bridge) had itself never been implemented either. Fixed by extracting the full redesign into ADR-0014, reducing ADR-0010's Amendment 3 to a short pointer, and updating every cross-reference (ADR-0012, ADR-0013,docs/architecture.md, this plan'sImplements:line, and the handful of source-comment references) that pointed at "ADR-0010's [third] amendment" to point at ADR-0014 instead — except this plan's own historical round-3/round-4 narrative entries above, left as written since they're an accurate record of what was true at that point in time, not a live cross-reference. - A ninth round of PR #11 review found one more real gap, worse than described: a
dynamiccollection element or key type (composer.Create<HashSet<dynamic>>(),Dictionary<dynamic, int>) doesn't just make the generated plan emit an illegaltypeof(dynamic)(CS1962) — the generated plan class can't even implementICompositionPlan<HashSet<dynamic>>at all (CS1966, "cannot implement a dynamic interface"), confirmed directly, so no template-body fix could have addressed this; the element/key type has to be rejected at classification time, before a collection plan is ever attempted.dynamicis a materially different case from round 4's pointer-element-array fix: a pointer can only ever appear as a rank-1 array element (never as aList<T>/HashSet<T>/Dictionary<TKey, TValue>type argument — the C# compiler already rejectsList<int*>before Compono's code runs), butdynamicis legal as a type argument for all five shapes. Fixed inCollectionWellKnownTypes.TryClassify: refactored into aFindShapehelper returning the shape candidate, plus one shared rejection check (ElementType/KeyType.TypeKindisPointer/FunctionPointer/Dynamic) applied uniformly across all five shapes, rather than the array-only special case round 4 added. Rejected candidates fall through to the ordinary composable-type walk exactly like round 4's pointer-array fix — forHashSet<dynamic>/Dictionary<dynamic, int>specifically, that walk reachesCMP0001(ambiguous construction, since both types have multiple accessible constructors), notCMP0006— still a real Compono diagnostic instead of a compiler error inside generated code, which is what actually matters here. Added regression coverage:CollectionPlanVerifyTests.DynamicHashSetElement_ReportsDiagnostic_NotACompilerErrorInGeneratedCode,DynamicDictionaryKey_ReportsDiagnostic_NotACompilerErrorInGeneratedCode. - A tenth round of PR #11 review found two items, both resolved without a behavior-changing code fix. First, a checklist/status mismatch: this plan's own "generated-plan execution coverage" item (above) was still unchecked even though round 8's
GeneratedCollectionPlanExecutionTestshad already closed it — checked off, with a pointer to the tests that closed it. Second, a genuine but narrower-than-first-glance leak concern inCollectionPlanCache<T>: when a collection's type arguments are entirely BCL types (List<int>,Dictionary<Guid, string>), the CLR homes that closed generic instantiation in the non-collectible default context regardless of which assembly's module initializer populates it, so a consuming assembly loaded into a collectibleAssemblyLoadContextgets permanently rooted by its own plan instance - the same leak class the
EnumValueProvidercache fix (round 5) closed elsewhere, but not fixable the same way here, sinceCollectionPlanCache<T>.Instancebeing a plain closed-generic static field (rather than aType-keyed lookup) is exactly what makes stage 7 dispatch a direct field read per ADR-0004 - any fix able to key off the consuming assembly/ALC instead ofTwould reintroduce a per-resolve lookup on every collection composition, undoing that. Verified the CLR loader-context claim directly against howPlanCache<T>avoids the same problem for ordinary composable types (a collectible type's own closed instantiation stays tied to its own collectible context, so no external root survives unload) before accepting the finding as real. Deferred alongside the pre-existingPlanCache<T>/CollectionPlanCache<T>cross-assembly-collision item (round 4) with the same reasoning: neitherdocs/mvp.md's scope nor Compono's primary xUnit-test-runner consumer exercises collectible-ALC hosting today. Documented indocs/architecture.md's Open Architectural Decisions and inCollectionPlanCache<T>'s own XML doc comment (which also had a stale "ADR-0010's third amendment" reference from before the round-8 ADR-0014 extraction, fixed in the same pass). - An eleventh round of PR #11 review found one more real gap in round 9's
dynamic-rejection fix:CollectionWellKnownTypes.TryClassifyonly checked the immediate element/key type'sTypeKind, soList<dynamic[]>slipped through -dynamic[]'s ownTypeKindisArray, notDynamic, even though its element isdynamic. Confirmed directly before fixing: this reached the emitter and failed withCS1966(cannot implement a dynamic interface) on the outerICompositionPlan<List<dynamic[]>>interface itself, not just on aResolve<dynamic[]>()call inside the plan body -dynamicnested anywhere in an interface's constructed generic arguments triggersCS1966, not just at the top level. Fixed by replacing the shallowTypeKindcheck with a recursiveContainsUnsupportedTypeKindhelper that looks through both array element types and generic type arguments forDynamic/Pointer/FunctionPointerat any depth (a pointer array nested one level down, e.g.List<int*[]>, is equally legal C# and has the same problem class as the array case, so the fix covers both in one pass rather than patchingdynamicalone). Rejected candidates fall through to the sameCMP0001path round 9's directHashSet<dynamic>/Dictionary<dynamic, int>fix already established. Added regression coverage:CollectionPlanVerifyTests.DynamicArrayNestedInsideListElement_ReportsDiagnostic_NotACompilerErrorInGeneratedCode(the array-recursion case that reproduces the actual finding) andDynamicNestedInsideDictionaryValueTypeArgument_ReportsDiagnostic_NotACompilerErrorInGeneratedCode(the generic-type-argument-recursion case,Dictionary<int, List<dynamic>>, proving the fix isn't array-specific). - A twelfth round of PR #11 review found one real issue and one status question resolved as no-action. The real issue:
GeneratedCollectionPlanExecutionTests.ListElements_EachGetsIndependentOutput_ThroughARealDispatchedPlancalledComposer.Create()(a fresh, non-deterministic seed every run), so a failure couldn't be reproduced from a bug report, and theOnlyHaveUniqueItems()assertion carried a (astronomically small but nonzero) theoretical dependence on GUID-space collision. Fixed by switching the asserted call toComposer.CreateRootForTesting<T>with a fixed seed, the same seamCompositionRandomIntegrationTestsalready uses inCompono.Tests- which required grantingCompono'sInternalsVisibleToto"TestsAssembly", the fixed nameGeneratorTestHelpers.CompileAndExecutegives every in-memory compiled test assembly, alongside the existingCompono.Testsgrant. Since the generator only discovers a type from aComposer.Create<T>()call-site pattern (CreateInvocationDiscovery), not fromCreateRootForTesting, the test source keeps a discardedcomposer.Create<List<Guid>>()call purely to trigger plan generation/registration, then asserts againstCreateRootForTesting's fixed-seed result - both dispatch through the exact same generated,CollectionPlanCache-registered plan instance, so this is still real end-to-end dispatch, not a shortcut around it. The status question: whether Phase 2's header should already readDonenow that every checklist item is checked - resolved as no-action, consistent with round 10's answer to the same question and with this plan's own established convention: Phase 0 and Phase 1 both stayed at their working status until their PRs (#10 and its predecessor) actually merged, only becomingDoneat that point, perAGENTS.md's "the prior phase's status/PR-merge state should be current before the next phase starts." PR #11 is still open, so Phase 2 staysIn Progressuntil it merges -Donenow would contradict that established pattern, not fix a real inconsistency. - A thirteenth round of PR #11 review found three real issues, the largest of this whole review cycle. Nullable custom struct silently never composed at runtime (P1).
LeafTypeClassifiertreated everyNullable<T>as a provider-resolved leaf regardless ofT, butNullableValueProvideronly ever composes a primitive/enum/recognized BCL underlying type - a custom struct'sNullable<T>(e.g.Money?) compiled with zero diagnostics and then always threw at runtime (confirmed directly: "no registration, provider, or generated plan could satisfy the request"), for both a rootComposer.Create<Money?>()and a constructor-parameter member. Fixed by makingIsNullableValueTypecheck the underlying type: only primitive/enum/recognized-BCLNullable<T>stays a leaf; any otherTnow falls through to ordinary composable-type handling, exactly like any other concrete type. This does not add a new nullable-composition mechanism -Nullable<T>has two accessible constructors to Roslyn's symbol model (the implicit parameterless one andNullable(T value)), so the existingConstructorSelectorambiguity check reports the sameCMP0001ambiguous-construction diagnostic any other multi-constructor type gets, converting the silent runtime failure into a real compile-time one for free, without new diagnostic code or a parallel dispatch architecture (rejected as disproportionate to this finding, and as "building ahead of the milestone" perAGENTS.md, givendocs/mvp.md's "Nullable value types" Initial Built-in Type sits directly alongside the primitive/enum list, not as its own arbitrary-custom-struct feature). Added regression coverage:CompositionPlanVerifyTests.NullableCustomStructRootType_ReportsDiagnostic_NotASilentRuntimeFailureandNullableCustomStructConstructorParameter_ReportsDiagnostic_NotASilentRuntimeFailure(bothCMP0001, root and member); confirmed no regression forNullable<int>/Nullable<Priority>(the existingNullableValueTypeRootType_GeneratesNoPlan/NullableValueTypeConstructorParameter_LeftAsResolveCallNotRecursedIntotests still pass unchanged). Accepted-ADR immutability violated by this PR's own amendments (P1). ADR-0010's "Amendment 3," ADR-0012's "Amendment 3," and ADR-0013's "Amendment 2" - all three added earlier in this PR (round 8/the original ADR-0014 extraction), not inherited from an earlier merged PR - edited already-AcceptedADRs' content in place, whichdesign-decisions.mdexplicitly prohibits ("don't edit [an accepted ADR's] Decision/Rationale/Consequences... write a new ADR"), regardless of ADR-0010/0012/0013's own earlier, pre-existing Amendments (½, predating this PR) having already established that same drift from the documented rule. Fixed by removing all three sections' prose entirely and replacing each with aLinks-section entry pointing to ADR-0014 instead (metadata/cross-reference, not decision content) - ADR-0014's own Context and Negative Consequences wording updated to match (it previously described these as "reduced to a pointer" rather than removed). ADR-0010/0012/0013's pre-existing Amendments (½) are untouched, since they predate this PR and are a separate, pre-existing concern out of this PR's scope. SpoofableInternalsVisibleTogrant (P2, found against round 12's own fix). Round 12'sInternalsVisibleTo Include="TestsAssembly"grant authenticates only a simple assembly name (unsigned) - any real consumer could name their own assemblyTestsAssemblyand gain the identical internal access to the shippedComponopackage. Fixed by reverting that grant entirely and switchingGeneratedCollectionPlanExecutionTests's fixed-seed test to reachComposer.CreateRootForTesting/CompositionSeedvia reflection from within its compiled test source instead - already howGeneratorTestHelpers.CompileAndExecuteinvokes every test's entry point, so this adds no new public/friend surface to the shipped package at all. - A fourteenth round of PR #11 review found two real issues, both small. A rejected array-shaped member silently produced no diagnostic (P2). A root
dynamic[]/pointer-element array already reportedCMP0006correctly viaComposedTypeAnalyzer(confirmed directly:composer.Create<dynamic[]>()does report it), but the equivalent member case didn't:TransitiveClosureWalker.EnqueueMembercorrectly declines to classify a rejected array as a collection (same reason as always - buildingICompositionPlan<dynamic[]>would hitCS1966), but its fallback for a non-INamedTypeSymbolmember (if (memberType is not INamedTypeSymbol) return;) silently dropped it instead of reporting anything - unlike a rejected generic collection shape (HashSet<dynamic>as a member), which is still anINamedTypeSymboland so still reachesConstructorSelector's ownCMP0001via the ordinary ("ambiguous constructor") path. Confirmed directly before fixing: adynamic[]constructor parameter compiled with zero diagnostics, emittingcontext.Resolve<dynamic[]>()for a type nothing would ever register a plan for. Fixed by threading the walker'sresultslist intoEnqueueMember(andEnqueueRoot, for the recursive element/key calls it makes) and reporting the sameCMP0006ComposedTypeAnalyzeralready uses for the root case, reusing itsTypeArgumentFailurehelper rather than duplicating the diagnostic-construction logic. Added regression coverage:CollectionPlanVerifyTests.DynamicArrayConstructorParameter_ReportsDiagnostic_NotACompilerErrorInGeneratedCode.docs/architecture.md'sCompositionRequestKindcontract was stale (P2). ItsCompositionRequestDescriptorcode sample still commentedkindasConstructorParameter | RequiredMemberonly, missing the three collection-plan cases (CollectionElement/DictionaryKey/DictionaryValue) ADR-0014 added, and its public-vs-internal-visibility discussion didn't mentionCollectionPlanCache<T>/UniqueValueResolverjoiningCompositionRequestDescriptor/ICompositionContextas public generated-code call surface. Fixed by updating both spots. Also self-caught (not a review finding) while replying to this thread: seven source-code doc comments across both projects still cited "ADR-0010's third amendment" - the section round 13 had just removed entirely - repointed all seven to ADR-0014 directly. - A fifteenth round of PR #11 review found one real issue: a regression introduced by round 14's own
CMP0006-for-rejected-array-members fix, not a pre-existing gap. ReportingCMP0006for every non-INamedTypeSymbolmember unconditionally also caught plaindynamic(notdynamic[]) -dynamicis likewise not anINamedTypeSymbol, but unlike an array/pointer/function-pointer it isn't a permanently-unsatisfiable shape:Resolve<dynamic>()is legal and is the established, intentional way to leave a member for a future registration/semantic provider (dynamic erases toobject, so virtually anything could satisfy it). Confirmed directly before fixing: a plaindynamicconstructor parameter, previously compiling clean withcontext.Resolve<dynamic>()left for a provider, started reportingCMP0006after round 14's fix - a real regression, not a restatement of round 14's finding. Fixed by narrowing the diagnostic to onlyIArrayTypeSymbol/IPointerTypeSymbol/IFunctionPointerTypeSymbolspecifically, leavingdynamic(and anything else non-named) to fall through silently exactly as it did before round 14. Added regression coverage:CollectionPlanVerifyTests.PlainDynamicConstructorParameter_GeneratesNoDiagnostic_StaysProviderResolved, alongside the existingDynamicArrayConstructorParameter_ReportsDiagnostic_NotACompilerErrorInGeneratedCodeproving the array case still correctly reportsCMP0006.
Phase 3 — Scope, shared values, exact registrations, and recursion detection (Done, PR #12 merged)¶
-
CompositionScope(type-keyed, one per root composition operation) - Wire
IsSharedrequests through the scope-check pipeline stage (stage 2) - Internal test-only exact-registration store satisfying pipeline stage 3, exercised via
InternalsVisibleTofromCompono.Tests(no public API yet, per ADR-0011's scope note) - Authoritative null/type validation for stages 2 and 3 (ADR-0011's second amendment): a shared/scoped or registration-produced value that is
nullfor a non-nullable request, or whose runtime type isn't assignable toCompositionRequest.RequestedType, is an authoritativeFailureat that stage — never passed through asNotHandledto a later stage - Active-construction-frame stack (
internal, distinct fromCompositionPath): pushed only immediately before stage 8 invokes a generated plan, popped infinally; a request for a type whose frame is already active is an authoritative recursionFailurecarrying the chain of active frames - Unit tests: a shared request resolves once and reuses the value for a second shared request of the same type in the same scope; a non-shared request never reads from scope even if the same type was already shared; a registered/shared value legitimately terminates a self-referencing type without tripping recursion detection at all; a
nullshared/registered value against a non-nullable request, and a type-mismatched registered value, both fail authoritatively at their own stage rather than falling through; an actual construction cycle usingpublic sealed record Node(List<Node> Children);(no terminating registration/shared value anywhere in the loop) fails with a diagnostic whose cycle-edge chain explicitly includes the collection-index segment (Node → Children[0] → Node, not justNode → Node) — moved here from Phase 2 since the active-construction-frame stack this test depends on doesn't exist until now
Notes on what actually happened, versus what was scoped:
- No public entry point can mark a request
IsShared: trueyet (the[Shared]attribute is Milestone 4) andCompositionRequestDescriptordeliberately wasn't widened to carry that flag early — so stage 2 is exercised through a new internal test seam,CompositionContext.ResolveSharedForTesting<TValue>(ordinal, name), mirroringComposer.CreateRootForTesting's existing "explicitly-named test seam per unimplemented public surface" pattern rather than overloadingResolve<TValue>. - The exact-registration store is a small dedicated type,
CompositionRegistrations(wrapping a type-keyedIReadOnlyDictionary<Type, object?>), rather than a bare dictionary field onCompositionContext— kept symmetrical withCompositionScopeas its own named concept per the ADR's wording, and gives stage 3 a singleTryGetcall site. CompositionResultgained theFailurecase ADR-0010 always reserved for context-owned authoritative stages but that no code had constructed yet — used by stages ⅔'s null/type validation and stage 8's recursion check. A new privateCompositionContext.Authoritative<TValue>helper converts aFailureinto the same thrownCompositionExceptionstage 9's terminal case already uses, keeping exactly one result-to-exception conversion point.- The recursion diagnostic reuses
CompositionPath.ToDisplayString()(already tracking every request edge, including collection-index segments) rather than building a separate message from the active-construction-frame stack directly — the frame stack only needed to answer "is this type already under construction," not carry its own display representation. - A PR #12 review round (Codex) found one real gap: a shared value's first population (from a provider, a collection plan, or a generated plan) skipped
ValidateAuthoritativeValueentirely — a null-for- non-nullable or type-mismatched value got cached into_scopeunvalidated, so the initial request threwInvalidCastException/NullReferenceExceptioninstead of the documentedCompositionException, and a later shared read of the same type hit the already-poisoned cache. Fixed by routing both shared-store paths (StoreSharedAndReturn, andResolveViaGeneratedPlan's shared branch) through the same validate-then-cache sequence stages ⅔ already use, scoped toIsSharedrequests only (an ordinary, non-shared provider's output is still validated only by its own contract, not the context — widening validation to every provider result wasn't part of this finding). Added regression coverage inCompositionScopeTests: the null case, the type-mismatch case, the same through a generated plan, and confirming an invalid first value doesn't poison the scope for a subsequent shared request.
Phase 4 — CreateMany<T>() and diagnostics polish (Done, PR #13 merged)¶
-
Composer.CreateMany<T>(int count)—countindependent root composition operations (Execution Flow section, above), each item's root seed forked from the batch root via"CreateMany"+ index (ADR-0012) — no cross-item scope reuse. Contract, per ADR-0011's second amendment:count < 0throws viaArgumentOutOfRangeException.ThrowIfNegative(count);count == 0returns an empty, materialized, non-nullIReadOnlyList<T>; return type isIReadOnlyList<T>(notIEnumerable<T>— the batch is always fully, eagerly materialized, never deferred) - The allocation-free-on-success trace buffer (ADR-0010): checkpoint on
Resolve<T>/ResolveRoot<T>entry, append a compactProviderAttemptper stage/provider tried, rewind on success, materialize into the durable diagnostic on failure before the buffer unwinds further. Per ADR-0010's amendment: because a sibling's checkpoint-rewind happens on its own success (before the next sibling is even attempted), the materialized trace on failure naturally contains only the active failing branch's own attempts — never an already-succeeded, already-rewound sibling's - Structured diagnostic type surfaces root request, full path (as a display string derived from
CompositionPath), the materialized trace, seed, and a human-readable remediation-oriented message, matchingdocs/architecture.md's example format - Unit test asserting the trace-retention property directly: a type with two successfully-resolved constructor parameters followed by a third that fails produces a diagnostic trace containing only the failing parameter's attempts (and its ancestors' own attempts) — not the two already-succeeded siblings'
- Benchmark (extending
Compono.Benchmarksfrom Milestone 1, widened mid-plan per PR #11 review — see this section's note below): confirm the trace buffer is actually allocation-free on the success path; if it measurably harms the hot path, fall back to shallow diagnostics by default with full tracing behind an explicit diagnostic-mode opt-in (ADR-0010's stated fallback). Milestone 1'sArchitectureBenchmarks/EcosystemBenchmarksonly cover generated construction dispatch versus reflection — nothing inCompono.Benchmarksyet exercises the resolution pipeline itself (provider dispatch, random forking, collection generation), and Phase 4 is the first point a representative end-to-end graph (nested composable type + every Phase 2 built-in + a collection) actually exists to benchmark. Add that end-to-end coverage here rather than deferring it further:Create<T>()andCreateMany<T>(count)throughput for a representative graph (the Execution Flow section'sCustomer/Addressshape, extended with a collection member), alongside the existing reflection/generated-construction baseline — establishes Compono's actual per-call cost, not just its construction-dispatch cost- Allocations for that same graph, success path only — the trace buffer's allocation-free claim (above) is one contributor to this number, not the whole story once collections/providers are in the mix
CreateMany<T>(count)scaling behavior across a couple ofcountvalues, to catch anything unexpectedly super-linear (fork-key derivation, scope allocation) before it ships
- Exit-criteria pass: representative object graph (nested composable type + built-ins + a collection) composes deterministically end to end via
Create<T>()andCreateMany<T>(), matching the Execution Flow section above exactly -
CreateManystability test: items 0–2 ofCreateMany<T>(3)andCreateMany<T>(10)(same explicit root seed) are byte-for-byte identical -
CreateManyargument/return contract tests:count < 0throwsArgumentOutOfRangeException;count == 0returns an empty, non-nullIReadOnlyList<T>; a standaloneCreate<T>()(viaCreateRootForTesting) called twice with the same explicit seed produces identical output, confirming no hidden per-call state beyond the seed itself
Notes on what actually happened, versus what was scoped:
ProviderAttemptended up(PipelineStage Stage, CompositionAttemptOutcome Outcome)— no distinct "provider id" field, despite ADR-0010's own text describing one. A PR #13 review (Codex) flagged this as contradicting the accepted ADR; formalized as its own record rather than an in-place ADR-0010 edit (this repo's own established "extract a sub-decision into a new ADR" pattern, per ADR-0014's precedent) — ADR-0015.PipelineStageandCompositionAttemptOutcomearepublic(notinternal), and so, transitively, isProviderAttempt— a publicCompositionDiagnostic.Tracecan't expose an internal element type. This is a deliberate, narrow widening of the public surface (coding-standards.md's "as narrow as the actual callers require"), justified bydocs/public-api.md's own pre-existing Diagnostics API section already showingexception.Diagnosticas consumer-facing.CompositionException's existingMessage(the short, single-line reason - e.g."The shared value for 'Widget' is null...") is left exactly as-is; the newCompositionDiagnostic.Messagecarries the same string.CompositionDiagnostic.ToString()(whatConsole.WriteLine(exception.Diagnostic)renders, perdocs/public-api.md) is the only place the full tree + seed text lives- keeping
Exception.Messageshort avoids relitigating every existingWithMessage("*...*")test's substring assumption, and matchesdocs/public-api.md's own example, which readsexception.Diagnosticseparately from the base exception. - The materialized
Traceslice is always taken from checkpoint0(the whole operation), not the checkpoint of theResolve<T>call that actually threw - the per-callcheckpointlocal is still used for each call's ownRewindon success, but a failing call reports the entire surviving buffer, which by construction (every already-succeeded sibling already rewound itself out) is exactly "the failing branch's own attempts and its ancestors'," per the task list wording above. Threading the local checkpoint intoBuildExceptioninstead would have produced only the failing leaf's own attempts, silently dropping every ancestor's - caught while writingCompositionDiagnosticsTests.ResolveRoot_DiagnosticTrace_RetainsOnlyTheFailingBranch_NotAlreadySucceededSiblingsbefore it was ever committed as a bug. CompositionPathgainedRootTypeandToTreeString()alongside the existingToDisplayString()(kept unchanged, still used by the recursion-cycle message) rather than replacing it - the tree format is specific toCompositionDiagnostic.Path, and the dotted form's callers (BuildCycleMessage) have their own established test coverage (CompositionRecursionTests) that doesn't need to move.Compono.Testsdoesn't referenceCompono.Generatorsas an analyzer (Phase 0's note, still true) - every Phase 4 test (CreateManycontract/stability, the diagnostics trace-retention test, and theCustomer/Addressend-to-end exit-criteria test) uses a hand-writtenICompositionPlan<T>test double viaPlanCache<T>.Instance, the same pattern every earlier phase's tests already established.- Real manual verification (
tasks/implement.mdstep 7, this phase's own new public API surface):dotnet pack'dComponointo a clean local feed and referenced it from a genuinely separate throwaway console project (same pattern Phase 2's round 5 established) exercisingCreate<T>(),CreateMany<T>(count)(positive/zero/negative), and a real recursion failure'sexception.Diagnosticthrough the real generator - notCompono.Tests' hand-writtenPlanCache<T>fakes. This caught one real gap:CompositionPath.ToTreeString()'s node labels usedType.Namedirectly, which renders a closed generic in its raw CLR form (List\1) instead of a readable one -Console.WriteLine(exception.Diagnostic)for aListmember printed `` List1 Children` instead ofListChildren . Fixed by addingCompositionPath.FriendlyTypeName(recurses through nested generic type arguments, e.g.Dictionary> ) and using it in bothToTreeString()call sites;CompositionDiagnosticsTests.ResolveRoot_DiagnosticPath_RendersClosedGenericTypes_InCSharpStyleNotRawClrFormis the regression guard.ToDisplayString()(the dotted form the recursion-cycle *message* still uses) was deliberately left alone - it already only readsNamevalues offPathSegment, neverRequestedType.Name`, so it was never affected by this gap. - Benchmark result (real, not extrapolated,
DefaultJob— not a quick--job shortsmoke test): three newCompono.Benchmarksclasses, all against the sameCustomer/Addressgraph (including aList<string>collection member), run through the real generator viadotnet run -c Release -- --filter "*Resolution*", mirroringArchitectureBenchmarks/EcosystemBenchmarks' split - a review round flagged that the first version of this benchmark had nonew()/ reflection/AutoFixture comparison at all, unlike Milestone 1's benchmarks.ResolutionBenchmarks(Create/CreateMany) measuredCreate<Customer>()at ~2.73 KB allocated per call regardless ofCreateMany's batch size, andCreateMany<T>(count)scaling linearly withcount(10.18× allocation atcount=10, 101.47× atcount=100, against thecount=1baseline - no super-linear growth). This confirms the checkpoint/rewind trace buffer doesn't measurably harm the hot path; the shallow-diagnostics fallback ADR-0010 reserved for that case was not needed.ResolutionEcosystemBenchmarks(GeneratedvsAutoFixture) measured Generated at ~90.9x faster and ~2.75% of AutoFixture's allocation -Customeractually gives AutoFixture real randomized-value-generation work, unlikeLeaf.ResolutionArchitectureBenchmarks(Direct/Generated/Reflection, the newReflectionComposer.ComposeRecursive<T>()) is not a clean win/loss read like theLeafcomparison: the reflection baseline fills every field with a fixed placeholder value rather than replicating Compono's real random-value generation, so it's faster thanGeneratedfor doing categorically less work, not because reflection dispatch beats source-generated dispatch - documented explicitly in both the class' XML doc<remarks>anddocs/performance.md, the mirror image of the AutoFixture caveat. Full tables and reproduction steps recorded permanently indocs/performance.md(docs/architecture.md's Diagnostics section links there too, rather than duplicating the table). - A PR #13 review round (Codex) found three real issues, all fixed in the same PR. Trace buffer's own allocation was asserted, not measured (P1). The Phase 4 benchmark result above reported total allocation without isolating what fraction the trace buffer itself contributes, so "allocation-free on success" read as a stronger claim than actually verified. Fixed by measuring
CompositionTraceBufferin isolation (GC.GetAllocatedBytesForCurrentThread()around 1,000,000 direct constructions, Release, .NET 10.0.3 arm64): 184 B/instance, ~6.6% of a realCustomercomposition's 2.73 KB - real, consistent with ADR-0010's "near-zero, not zero-cost" framing, not a violation of it.docs/performance.mdanddocs/architecture.mdnow state this precisely instead of the unqualified claim; a true zero-allocation design (poolingCompositionTraceBufferacross root operations) is recorded as a new deferred item indocs/architecture.md's Open Architectural Decisions, not attempted as a same-PR fix.Successrecorded before composition actually completed (P2, worse than reported). The review flagged one site (CollectionPlanCache<TValue>.Compose(this)recordingBuiltInProvider: SuccessbeforeComposeran, so an element failure inside the collection left a falseSuccessin the materialized diagnostic) - confirmed, and the identical bug pattern was present in five more places: the profile/semantic/test-double/ built-in provider branches (recordingSuccessbeforeStoreSharedAndReturn's authoritative shared-value validation, which can still throw) andResolveViaGeneratedPlan's shared path (same issue, afterComposebut beforeStoreSharedAndReturn). Fixed at all six sites by moving each_trace.Record(..., Success)to strictly after the corresponding value has actually been returned/stored without throwing. Verified as a real regression, not just a theoretical one: temporarily reverted the fix and confirmed the new regression tests actually fail without it, then restored the fix -CompositionDiagnosticsTests.ResolveRoot_DiagnosticTrace_DoesNotRecordSuccess_WhenACollectionPlanElementFails(the exact collection scenario reported) and..._WhenASharedProviderValueFailsValidation(the broader pattern, at the cheapest site to exercise via the injectable-provider test seam). Provider identity dropped from the public trace record (P1). Already covered above and in ADR-0015 - a real, but already-intentional and now-formally-recorded, deviation from ADR-0010's literal text, not a gap. - The same PR #13 review round separately prompted rebuilding the resolution benchmark's reflection baseline, once its flawed comparison was noticed on inspection rather than via a specific finding:
ReflectionComposer.ComposeRecursive<T>()originally filled every field with a fixed placeholder value ("value", etc.), makingResolutionArchitectureBenchmarks'Reflectioncolumn faster thanGeneratedfor doing categorically less work - a comparison that invited exactly the wrong conclusion ("why not just use reflection"). Rewritten to generate real random values matching Compono's own defaults (an 8-character alphanumeric string perPrimitiveValueProvider'sStringLength, a 3-element collection per ADR-0013's default collection size) viaSystem.Random.Shared- the actual reflection-based alternative someone would reach for, not a dispatch-cost-only strawman. Re-run atDefaultJobafter the rewrite;docs/performance.md's Resolution architecture benchmark table reflects the new, honest numbers. - A second PR #13 review round (
@codex review, triggered after the first round's fixes landed) found three more real issues, all fixed in the same PR, plus a separately-requested optimization landed alongside them.CompositionRequest: class →readonly record struct(the "easy win," requested directly rather than found by review). It's never stored beyond the synchronousResolveCorecall that builds it - never captured, never crosses an async boundary - so the heap allocation was pure waste. Prototyped and measured before touching production code: 40 B → 0 B, 14.9 ns → 5.5 ns, constructed and consumed the same wayResolveCoreactually does (passed byin, not boxed). Applied for real:CompositionRequestis nowinternal readonly record struct,ICompositionProvider.TryComposeand every internal consumer (TryProviders,StoreSharedAndReturn,ValidateAuthoritativeValue,ResolveViaGeneratedPlan) take it byin, matchingCompositionRequestDescriptor's existing convention. This is what moved every benchmark table indocs/performance.mdto new, lower numbers - re-measured, not estimated. A generic root type rendered in raw CLR form at two more sites (P2).CompositionDiagnostic.ToString()'s heading (RootType.Name) and the stage-9 failureMessagetext (requestedType.Name) both had the identical bugCompositionPath.FriendlyTypeNamewas added to fix for the path tree in the first review round - just missed at these two call sites. Fixed by makingFriendlyTypeNameinternal(notprivate) onCompositionPathand reusing it at both sites. An ancestor's in-flight dispatch was silently absent from the trace on a nested failure (P2). The first round'sSuccess-after-not-before fix was correct but incomplete: an ancestor whose ownComposewas still running when a descendant failed never got any trace entry for its stage-8/collection-plan attempt, sinceSuccessis (correctly) only recorded afterComposereturns, which never happens for an ancestor still waiting on a failing descendant. Fixed by addingCompositionAttemptOutcome.Pending, recorded immediately beforeComposeruns at both dispatch sites - rewound away on success like every other entry, but surviving (correctly) when the eventualSuccessnever gets recorded either. The trace buffer's growth path was never benchmarked (P2). Only the shallow, 2-level-deepCustomergraph had been measured, which never gets deep enough to triggerCompositionTraceBuffer's ownArray.Resize(each active ancestor frame retains ~6 entries with thePendingfix above, so a 16-entry buffer resizes past depth ~2-3 - already true ofdocs/architecture.md's own 4-level Diagnostics example). Fixed in two parts: bumped the initial capacity from 16 to 32 (covers ~5 levels without resizing) and addedDeepGraphBenchmarks(an 8-level-deep chain,DeepLevel1throughDeepLevel8, deep enough to guarantee a resize) so the growth cost is a real measured number indocs/performance.mdrather than assumed away. All four changes verified against regression tests reverted-and-reran (same discipline as the first review round) before being restored — seeCompositionDiagnosticsTests.ResolveRoot_Throws_WithADiagnosticTree_MatchingTheNestedFailurePath'sPending-count assertion andResolveRoot_Throws_WithAFriendlyGenericRootName_NotTheRawClrForm. - A third PR #13 review round (
@codex reviewagain) found two more real gaps, plus surfaced a factual error in ADR-0015's own Context that needed correcting. Stage 7's three built-in providers collapsed into one aggregate trace entry (P2).TryProviderslet its caller record a singleNotHandledfor the whole stage regardless of how many providers actually declined -BuiltInProviders.Defaultgenuinely has three registered today (PrimitiveValueProvider,EnumValueProvider,NullableValueProvider), so a failing request's trace showed 1 aggregate entry instead of the real count. This directly falsified ADR-0015's own Decision Drivers claim ("no stage in Milestone 2 ... registers more than one competing provider yet") - stage 7 already does, today, not hypothetically. Fixed by moving theNotHandledrecording intoTryProvidersitself, one entry per provider actually tried (a newPipelineStage stageparameter); the winning provider's own outcome is still left for the caller to record once, afterStoreSharedAndReturnvalidates it - unaffected by the earlier Success-ordering fix. Perdesign-decisions.md's "an accepted ADR is a historical record" rule, ADR-0015's own text is left as originally written (an accurate record of the reasoning at the time, even though one of its cited facts turned out to be wrong) - the correction instead lands where it actually matters going forward:docs/architecture.md's identical claim (a living doc, meant to be kept current) is fixed in place, and this note records the correction for anyone reading the plan forward. ADR-0015's actual Decision Outcome (defer provider identity, not attempt counting) still holds - this fix only restores accurate attempt counts, it doesn't add a way to tell which provider each entry belongs to, so the ADR's core deferral is unaffected by the correction.FriendlyTypeNamedidn't recurse into array element types (P2).Type.IsGenericTypeisfalsefor an array type itself (array-ness and generic-ness are orthogonal in reflection), soList<Missing>[]fell straight through to the raw CLR form (List\1[]) - the same defect classFriendlyTypeNameexists to prevent, just not handled for arrays. Fixed by checkingIsArrayfirst and recursing intoGetElementType(), reapplying the array's own rank. Both fixes verified against regression tests reverted-and-rerun before being restored, matching prior rounds' discipline; neither changed any benchmark number (re-measured to confirm -docs/performance.md`'s tables are unchanged within normal noise, since both are trace-fidelity/ formatting fixes, not allocation or timing changes). - A fourth PR #13 review round found the most severe gap of the whole review cycle (P1):
CreateMany<T>()was never wired into the source generator's discovery, so the single most-advertisedCreateMany<T>usage threwCompositionExceptionat runtime. A consumer calling onlycomposer.CreateMany<Customer>(3)- neverCreate<Customer>(), no[Composable]attribute - got no generated plan forCustomerat all:CreateInvocationDiscovery.IsCandidate/Transform(src/Compono.Generators/Discovery/CreateInvocationDiscovery.cs) still hardcoded the method name"Create"only. Notably, this was not a design gap - ADR-0004 already specified "call-site driven (findCreate<T>()/CreateMany<T>()invocations..." anddocs/architecture.md's Discovery and Dispatch section already said the same - the generator's implementation simply never caught up once Phase 4 shipped the realCreateMany<T>()method, sinceCreateInvocationDiscoverypredates it (Milestone 1). Fixed by widening both the syntax-levelIsCandidatepattern and the symbol-levelTransformcheck to accept"Create"or"CreateMany"- a two-line change once located, per the ADR's already-correct design. Verified three ways, matching the bar for source-generator-facing changes (tasks/implement.mdstep 7): (1) a new generator snapshot test,CompositionPlanVerifyTests.CreateManyOnlyInvocation_GeneratesCompositionPlan, compiling source with aCreateMany<Customer>(3)call site and noCreate<Customer>()/[Composable]anywhere, confirming a real plan gets generated; (2) reverted the fix and confirmed that test fails, restored and confirmed it passes; (3)dotnet pack'dComponointo a clean local feed and ran a genuinely separate throwaway console project calling onlycomposer.CreateMany<Customer>(3), confirming real end-to-end output (three distinct composedCustomers) through the actual published package, not just the in-memory test harness. - A fifth PR #13 review round found four more real issues, the largest being a genuine reversal of ADR-0015's own decision. Provider identity restored in
ProviderAttempt(P1) - the user directed this explicitly ("Codex is pushing back on that provider identity decision. I'm inclined to agree. Let's add it back in") after independently agreeing with the review finding. ADR-0015's deferral rested on "no stage has more than one competing provider yet" - already false for stage 7's three real built-in providers when ADR-0015 was written, and the round-3 per-provider-trace fix only made the gap visible (three indistinguishable(BuiltInProvider, NotHandled)entries) rather than closing it. Perdesign-decisions.md's ADR-immutability rule, this is a genuine decision reversal, not a factual correction like round 3's ADR-0015 issue - so it gets its own ADR (ADR-0016) superseding ADR-0015 (Status flipped, body left unedited), rather than editing ADR-0015 in place.ProviderAttemptgains aType? Providerfield - the provider's own concrete CLR type (nullfor a context-owned stage, which isn't anICompositionProviderinstance), chosen over a provider index (unstable once Milestone 3 allows reordering, per ADR-0015's own still-valid Option 2 reasoning) or an opaque token (needs a newICompositionProvidermember, per ADR-0015's own Option 3 reasoning) -Typeidentity is already established elsewhere in this engine (PlanCache<T>, the active-construction-frame stack,EnumValueProvider's cache), so this isn't a new pattern. Fixing this meant redesigning where outcomes get recorded:TryProvidersnow hands back the winning provider'sTypevia anoutparameter, andStoreSharedAndReturnrecords the stage's real outcome (SuccessorFailure) itself, beforeAuthoritativecan throw - which also fixed finding two of this round (P2): a shared request's validation failure left no trace entry for the provider that produced the bad value at all, the same "recorded after, not before" ordering bug as earlier rounds, just at a site those rounds hadn't reached yet. Finding three (P1): aLinksentry I'd added to the already-Accepted ADR-0010 in an earlier round was itself a rule violation - reviewed and agreed; removed entirely, restoring ADR-0010 to its pre-Phase-4 text. The cross-reference lives only in ADR-0015/ADR-0016's ownLinkssections and the ADR index now, never in ADR-0010 itself. Finding four (P2):CompositionException(CompositionDiagnostic)dereferenceddiagnostic.Messagein itsbase(...)initializer before the constructor body could guard it, so anullargument surfaced asNullReferenceExceptioninstead ofArgumentNullException- fixed by routing the null check through a static helper called from the initializer itself (base(RequireDiagnostic(diagnostic).Message)), since a guard clause in the body runs too late. All four fixes verified against regression tests reverted-and-rerun before being restored, matching every prior round's discipline. Re-measured benchmarks honestly this time: unlike round 3, this round does move every number indocs/performance.md- wideningProviderAttemptroughly doubles each trace entry's size, movingCreate<Customer>()from 2.46 KB to 2.71 KB (+256 B, ~10%), confirmed attributable to the struct width by the isolatedCompositionTraceBuffermeasurement moving by almost exactly the same amount. Recorded as an accepted tradeoff (a real "which provider" answer for ~10% more allocation on the failure-adjacent path), not chased back down. - A sixth PR #13 review round found one more real gap, the same ancestor-visibility class as the
GeneratedPlan/collection-planPendingfixes above, at a third dispatch site (P2).TryProviders(stages 4-7) didn't record an in-flight attempt before callingcandidate.TryCompose(...).ICompositionProvider.TryComposeis handed the liveICompositionContextspecifically so a provider can recursively resolve part of its own value (no built-in provider does this today, but nothing about the contract forbids it, and it's exactly the extension point Milestone ⅗/6 profile/semantic/test-double providers are expected to use) - if that nestedResolve<T>()call throws,candidatewas never recorded anywhere, sinceTryProvidersonly recordedNotHandledafterTryComposereturned normally. The materialized diagnostic then permanently omitted the provider and stage that led to the failing child. Fixed by applying the same checkpoint/Pending/rewind pattern already used for stage 8 and the collection-plan branch: record(stage, candidate.GetType(), Pending)immediately beforeTryComposeruns, then rewind that marker away onceTryComposeactually returns (success or decline) - if it throws instead, the rewind never executes and thePendingmarker survives into the failing trace. Verified with a new regression test (ResolveRoot_DiagnosticTrace_RecordsAPendingEntry_WhenAProviderRecursivelyResolvesAFailingNestedRequestinCompositionDiagnosticsTests.cs) using the injectable-provider test seam and a fake provider whoseTryComposecallscontext.Resolve<T>()for an unsatisfiable nested type; reverted the fix and confirmed the test fails, restored and confirmed it and the full suite (152Compono.Tests, 146Compono.Generators.Tests) pass. Re-measured the allocation-per-Create<T>()benchmark before and after this specific fix in isolation (a throwaway test, deleted after measuring): identical 1384.00 B/op both times - the extra checkpoint/record/rewind calls stay well under the trace buffer's 32-entry initial capacity for an ordinary graph, so they cost stack work only, no heap allocation.docs/performance.mdis unchanged.
Critical Files¶
src/Compono/CompositionRequestDescriptor.cs(Kind,Ordinal,Name,Nullability),src/Compono/CompositionRequestKind.cs— new (public) — Done (Phase 0)src/Compono/CompositionRequest.cs— new (internal) — Done (Phase 0). Converted fromsealed record(class) tointernal readonly record struct, all consumers taking it byin— Done (Phase 4, PR #13 review round 2)src/Compono/CompositionResult.cs— new (internal) — Done (Phase 0).Failurecase added — Done (Phase 3)src/Compono/ICompositionProvider.cs— new (internal) — Done (Phase 0)src/Compono/CompositionException.cs— new (public); minimal message-only exception for now — Done (Phase 0). Second constructor accepting aCompositionDiagnostic(the shape every pipeline-thrown instance uses) and theDiagnosticproperty — Done (Phase 4). Null-diagnosticguard routed through a static helper called from thebase(...)initializer itself, since a body-level guard clause runs too late — Done (Phase 4, PR #13 review round 5)src/Compono/CompositionDiagnostic.cs,src/Compono/ProviderAttempt.cs,src/Compono/PipelineStage.cs,src/Compono/CompositionAttemptOutcome.cs— new (public);src/Compono/CompositionTraceBuffer.cs— new (internal) — Done (Phase 4).CompositionAttemptOutcome.Pendingadded, recorded beforeComposeat both generated-plan/collection-plan dispatch sites;CompositionTraceBuffer's initial capacity bumped 16 → 32;CompositionDiagnostic.ToString()rendersRootTypeviaCompositionPath.FriendlyTypeNameinstead of raw.Name— Done (Phase 4, PR #13 review round 2).ProviderAttemptgained aType? Providerfield (ADR-0016, reversing ADR-0015);CompositionTraceBuffer.Recordtakes the provider type alongside stage and outcome — Done (Phase 4, PR #13 review round 5)src/Compono/PathSegment.cs(Ordinal/Index-keyed,Namefor segments that have one),src/Compono/CompositionPath.cs— new (internal) — Done (Phase 0, pulled forward from Phase 1's original scope; structural chain only, no FNV-1a hashing yet).ToDisplayString()(diagnostics-only, derived from segmentNames) — Done (Phase 1).RootTypeandToTreeString()(theCompositionDiagnostic.Pathtree format) — Done (Phase 4).FriendlyTypeNamewidened fromprivatetointernalsoCompositionDiagnosticandCompositionContextcan reuse it for the heading/failure-message sites the first review round missed — Done (Phase 4, PR #13 review round 2)src/Compono/CompositionContext.cs— new (replaces the inlinePlaceholderCompositionContextinComposer.cs); implements the public descriptor-basedResolve<T>and the internalResolveRoot<T>; the internal test-seam constructor accepting explicit per-stage provider collections — Done (Phase 0). Seed-aware constructors, the_randomfield forked/restored alongside_pathinResolveCore, and the internalRandomtest-observability property — Done (Phase 1). Stages ⅔ (shared/scoped values, exact registrations), the active-construction-frame stack, and theResolveSharedForTesting<T>test seam — Done (Phase 3). TheCompositionTraceBuffercheckpoint/record/rewind wiring andBuildException(materializes aCompositionDiagnosticfrom the current path/trace/seed) — Done (Phase 4).TryProvidersrecords aPendingmarker for each candidate provider immediately before callingTryCompose, rewound on a normal return — closes the same ancestor-visibility gap already fixed for stage 8/collection-plan dispatch, for a provider that recursively resolves a nested request that fails — Done (Phase 4, PR #13 review round 6)src/Compono/CompositionScope.cs,src/Compono/CompositionRegistrations.cs— new (internal) — Done (Phase 3)src/Compono/CompositionSeed.cs,src/Compono/IRandomSource.cs,src/Compono/RandomSource.cs,src/Compono/Fnv1a.cs,src/Compono/SplitMix64.cs— new (internal) — Done (Phase 1)src/Compono/Providers/*.cs— new (internal, one file per built-in primitive/enum/nullable provider, percoding-standards.md's one-public-type-per-file rule — applies tointernaltypes too) — Done (Phase 2)src/Compono/CollectionPlanCache.cs— new (public); mirrorsPlanCache<T>'s shape exactly (ADR-0010 Amendment 3) — Done (Phase 2)src/Compono/UniqueValueResolver.cs— new (public); the bounded duplicate-value retry helper generatedHashSet<T>/Dictionary<TKey, ...>collection plans call (ADR-0010 Amendment 3) — Done (Phase 2)src/Compono/CompositionRequestKind.cs— modified:CollectionElement/DictionaryKey/DictionaryValuecases added (ADR-0010 Amendment 3) — Done (Phase 2)src/Compono.Generators/Discovery/TransitiveClosureWalker.cs— modified: recognizes the five ADR-0013 collection shapes, recursing into element/key type(s) instead of walking the collection itself as a composable type — Done (Phase 2)src/Compono.Generators/Discovery/CollectionWellKnownTypes.cs— new; classifies a symbol as one of the five supported closed collection shapes (or not), extracting element/key type(s) — Done (Phase 2)src/Compono.Generators/Models/DiscoveredCollectionInfo.cs,src/Compono.Generators/Models/TransitiveClosureResult.cs— new — Done (Phase 2)src/Compono.Generators/Emitters/CollectionPlanEmitter.cs,src/Compono.Generators/Emitters/GeneratedFileNaming.cs(hint-naming logic extracted out ofCompositionPlanEmitterfor reuse),src/Compono.Generators/Templates/CollectionPlan.scriban— new — Done (Phase 2)src/Compono.Generators/ComponoIncrementalGenerator.cs,src/Compono.Generators/Discovery/{CreateInvocationDiscovery,ComposableAttributeDiscovery,ComposedTypeAnalyzer}.cs— modified: returnTransitiveClosureResultinstead of a bareEquatableArray<DiscoveredTypeInfo>throughout, and the generator collects/dedupes/emits discovered collections alongside types — Done (Phase 2)test/Compono.Tests/Providers/*.cs,test/Compono.Tests/UniqueValueResolverTests.cs,test/Compono.Tests/CollectionPlanCacheDispatchTests.cs,test/Compono.Generators.Tests/CollectionPlanVerifyTests.cs— new — Done (Phase 2)src/Compono/ICompositionContext.cs— modified (Resolve<T>signature changed toin CompositionRequestDescriptor) — Done (Phase 0)src/Compono/Composer.cs— modified:Create<T>()rewired ontoResolveRoot<T>()— Done (Phase 0). The internalCreateRootForTesting<T>(CompositionSeed)/CreateManyForTesting<T>(int, CompositionSeed)test-seam factories — Done (Phase 1). PublicCreateMany<T>(int count)(negative-count/ zero-count/IReadOnlyList<T>contract), sharing its seed-derivation logic withCreateManyForTestingvia a privateComposeMany<T>helper — Done (Phase 4)src/Compono.Generators/Templates/CompositionPlan.scriban,src/Compono.Generators/Emitters/CompositionPlanEmitter.cs— modified: emitCompositionRequestDescriptor(constructor-parameter name added to the emitter's model; ordinal is the emission-order index for both parameters and required members) instead of Milestone 1's bareNullabilityargument — Done (Phase 0, unscoped fix — see Phase 0 Notes)src/Compono.Generators/Discovery/RequiredMemberCollector.cs— modified: base-to-derived required-member ordering (was derived-to-base), per ADR-0012's amendment 2 — Done (Phase 0, unscoped fix — see Phase 0 Notes; confirmed against ADR-0012 amendment 2's canonical algorithm in Phase 1, no further changes needed)src/Compono.Generators/Discovery/CreateInvocationDiscovery.cs— modified:IsCandidate/TransformrecognizeCreateMany<T>()call sites, not justCreate<T>()— closes a real gap against ADR-0004's already-correct design, discovered onceCreateMany<T>()shipped as a real method — Done (Phase 4, PR #13 review round 4).test/Compono.Generators.Tests/CompositionPlanVerifyTests.cs'sCreateManyOnlyInvocation_GeneratesCompositionPlan(+ its snapshot) — new — Done (Phase 4, PR #13 review round 4)test/Compono.Tests/CompositionContextTests.cs,test/Compono.Tests/ComposerTests.cs— new — Done (Phase 0)test/Compono.Tests/RandomSourceTests.cs,test/Compono.Tests/CompositionRandomIntegrationTests.cs— new — Done (Phase 1)test/Compono.Generators.Tests/CompositionPlanVerifyTests.cs+ regeneratedSnapshots/*.verified.cs— modified — Done (Phase 0, unscoped fix)test/Compono.Tests/CompositionScopeTests.cs,test/Compono.Tests/CompositionRegistrationTests.cs,test/Compono.Tests/CompositionRecursionTests.cs— new — Done (Phase 3)test/Compono.Tests/CompositionTraceBufferTests.cs,test/Compono.Tests/CompositionDiagnosticsTests.cs,test/Compono.Tests/ComposerCreateManyTests.cs,test/Compono.Tests/CompositionEndToEndTests.cs— new — Done (Phase 4).test/Compono.Tests/CompositionExceptionTests.cs— new (round 5's null-diagnosticguard) — Done (Phase 4, PR #13 review round 5)benchmarks/Compono.Benchmarks/ResolutionBenchmarkTypes.cs(Customer/Address),benchmarks/Compono.Benchmarks/ResolutionBenchmarks.cs(Create/CreateManyscaling),benchmarks/Compono.Benchmarks/ResolutionArchitectureBenchmarks.cs(Direct/Generated/Reflection),benchmarks/Compono.Benchmarks/ResolutionEcosystemBenchmarks.cs(Generated/AutoFixture),benchmarks/Compono.Benchmarks/DeepGraphBenchmarks.cs(DeepLevel1-DeepLevel8, an 8-level-deep chain exercisingCompositionTraceBuffer'sArray.Resizegrowth path) — new;benchmarks/Compono.Benchmarks/ReflectionComposer.cs— modified:ComposeRecursive<T>()added (fixed-placeholder-value recursive reflection composer, alongside the existing parameterless-onlyCompose<T>()) — Done (Phase 4)test/Compono.Tests/**— new tests per phase above
Test Plan¶
Per references/testing.md's established pattern in test/Compono.Tests:
- Pipeline-order tests confirming the fixed 9-stage precedence, and the context-owned-vs-provider-collection split, are both honored (Phase 0), using
CompositionContext's internal test-seam constructor to inject fake providers at specific stages — no public configuration surface involved. - Determinism tests (Phase 1) — via
Composer.CreateRootForTesting, the internal seam that exercises the realComposer/CompositionContextflow with an explicit seed before Milestone 3's public builder exists — asserting byte-for-byte reproducibility across independent resolutions with the same seed, structural independence across differently-Ordinaled forks (same-typed sibling parameters, collection elements, dictionary keys vs. values), that renaming (without reordering) a constructor parameter/required member doesn't change its derived value, and that all fivePathSegmentkinds fork to distinct output at ordinal/index 0 (the tag-collision test). - Provider-level unit tests for every built-in provider (Phase 2), keeping each provider narrowly testable per
coding-standards.md's "no God classes" guidance, plus collection-specific edge cases (duplicate-value retry exhaustion for bothDictionary<TKey, ...>keys andHashSet<T>elements; a non-recursive collection-index path-construction test). - Scope/sharing/registration/recursion tests (Phase 3) exercising the internal registration seam via
InternalsVisibleTo, including the "registered value legitimately terminates a cycle" case the original plan's recursion design would have rejected, authoritative null/type validation for shared/registered values, and the concreteNode(List<Node> Children)recursive-element case (moved from Phase 2) asserting the cycle diagnostic's edges include the collection index. CreateManysemantics, argument/return contract, and seed-stability tests (viaComposer.CreateRootForTesting/CreateManyForTesting), a diagnostics-trace-retention test (active failing branch only, not completed siblings), and a benchmark suite (Phase 4) covering both the trace buffer's allocation-free claim and end-to-endCreate<T>()/CreateMany<T>()throughput/allocations/scaling for a representative graph — the first point in this plan a graph exists that's representative enough to be worth benchmarking end to end, not just at the construction-dispatch layer Milestone 1's benchmarks already cover.- An end-to-end
Create<T>()/CreateMany<T>()test against theCustomer/Addressshape from the Execution Flow section (Phase 4), matching the Milestone 1 plan's "representative record or class" bar.
Notes¶
Anything discovered mid-implementation that changes this plan's shape from what's scoped above goes here, per design-decisions.md's "a plan being wrong about how doesn't require superseding anything."
This plan's first draft (context-owned-vs-provider mixing, unrestricted Failure, type-only CompositionPath, before-every-request recursion checking, Phase 3 randomness rewiring built-ins written in Phase 1) went through a deep design review before any implementation started — see ADR-0010 through ADR-0013's Context sections for exactly what each gap was and why the revised design closes it.
A second review before Phase 0 began found ten more pre-implementation gaps (root/descriptor mismatch, ordinal-vs-name identity, type-identity hashing, diagnostics-trace retention scope, HashSet<T> uniqueness, a vague recursion test, missing internal seams for seed injection and pipeline-stage testing, incorrect Phase 0 wording about what already works from Milestone 1, and the collection generic-dispatch bridge) — see ADR-0010/0011/0012/0013's ## Amendment (2026-07-28) sections.
A third review, still before any code was written, found ten further refinements: CompositionRequestDescriptor as a plain struct (not a record struct); the full canonical required-member ordinal algorithm (partial declarations, base members, generator-produced members); an explicit tag-collision test proving the five PathSegment kinds don't collide at ordinal/index 0; the structural-position reproducibility contract stated as a guarantee, not an implementation detail; strongly- typed per-shape delegate caches for the collection-dispatch bridge instead of an untyped Delegate cache; an explicit Native AOT/trimming position for that bridge; the CreateWithSeed→CreateRootForTesting/ CreateManyForTesting rename for clarity; CreateMany's negative/zero/return-type contract; authoritative null/type validation for stages ⅔; and moving the Node(List<Node> Children) recursion test to Phase 3, where the recursion detector it depends on actually exists — see ADR-0010/0011/0012's ## Amendment 2 (2026-07-28) sections for the full detail behind each.
A fourth review, mid-Phase-2 and before any Phase 2 code was written, caught that the collection-dispatch bridge scoped by the third review (MakeGenericMethod/CreateDelegate, cached per closed collection type) is a genuine violation of ADR-0001's no-reflection-by-default rule, not an acceptable bounded exception. Replaced with generator-emitted, strongly typed collection plans dispatched via a new CollectionPlanCache<T> — see ADR-0010's ## Amendment 3 (2026-07-28) section, and this plan's Phase 2 section above, for the corrected shape.