perf: consolidate module initializers into single .cctor via partial class

[thomhurst]

Summary

• Replace per-class and per-method [ModuleInitializer] methods with static field initializers on a shared partial class TUnit_TestRegistration
• The C# compiler merges all field initializers into a single static constructor (.cctor), reducing JIT overhead from O(N) methods to O(1)
• Add SourceRegistrar.RegisterReturn() overloads that return a dummy int for use as field initializers
• InfrastructureGenerator emits the partial class shell and triggers the .cctor via RuntimeHelpers.RunClassConstructor

How it works

Each per-class/per-method source file contributes a static readonly int field to the shared TUnit_TestRegistration partial class:

// Before: N separate [ModuleInitializer] methods → N JIT compilations
[ModuleInitializer]
internal static void Initialize()
{
    SourceRegistrar.Register(typeof(TestClass1), GetTests, EnumerateTestDescriptors);
}

// After: All merged into ONE .cctor → 1 JIT compilation
internal static partial class TUnit_TestRegistration
{
    static readonly int _r_TestClass1 = SourceRegistrar.RegisterReturn(
        typeof(TestClass1), TestSource1.GetTests, TestSource1.EnumerateTestDescriptors);
}

Performance impact

Metric	Before	After
JIT'd methods at startup (10k test classes)	~10,000	2 (.cctor + RegisterReturn)
Estimated JIT time savings	baseline	~500x fewer JIT calls

Key design decisions

• No .Collect() needed in the incremental source generator — each file independently contributes to the partial class, preserving incremental caching (zero IDE performance impact)
• catch (TypeLoadException) guards the RunClassConstructor call for assemblies with no test registrations
• Shared EmitRegistrationField helper eliminates duplication between per-class and per-method code paths

Test plan

• All 464 source generator snapshot tests pass across all 4 frameworks (net8.0, net9.0, net10.0, net472)
• All 12 public API tests pass across all frameworks
• Snapshot files updated to reflect new generated code pattern
• CI passes

[claude]

Code Review: perf: consolidate module initializers into single .cctor via partial class

Overall this is a well-conceived performance optimization. The design correctly leverages C#'s partial class static field initializer consolidation into a single .cctor, and the PR description clearly explains the trade-offs. The 464 passing snapshot tests give good confidence in the generated output.

---

Issue 1: RegisterReturn exposes an internal implementation detail as public API

SourceRegistrar.cs adds two public static int RegisterReturn overloads. These return a dummy 0 solely as a source-generator code-generation trick — users should never call them.

While the class is already annotated with [EditorBrowsable(Never)] (outside DEBUG), these methods still constitute public API that needs versioning, will appear in reflection, and can be called by any code that bypasses IntelliSense.

Better approach: Separate the source-generator-specific interface from the public API. Either:

Option A is cleanest if the source generator and TUnit.Core are in the same assembly or use InternalsVisibleTo. This keeps the public API surface minimal and intentional.

---

Issue 2: EmitRegistrationField has an implicit and fragile semicolon contract

In TestMetadataGenerator.cs:

Both call sites happen to pass registrarCall strings that end in ; (e.g., "RegisterReturn(...);")). But this is an invisible contract on the parameter. A future contributor adding a third call site without the semicolon would generate syntactically broken code that only fails at compile time, not at generation time.

Better approach: Either enforce the convention defensively, or flip the contract so the method owns the semicolon:

This makes the method self-contained and removes the hidden dependency.

---

Issue 3: catch (TypeLoadException) is overly broad

TypeLoadException can be thrown for many reasons beyond the type simply not existing in the assembly — e.g., a missing dependency of a type member, a bad generic constraint, a native AOT issue. Silently swallowing these would make debugging very difficult.

Better approach: Check for the type's existence before attempting RunClassConstructor:

This is more precise and won't silently hide real TypeLoadException failures from type members within TUnit_TestRegistration itself (e.g., if a RegisterReturn call throws during a field initializer, that would currently be swallowed too — the exception thrown from a static constructor is wrapped in a TypeInitializationException, not a TypeLoadException, but this is worth verifying).

---

Positive aspects

• The incremental generator design is preserved — no .Collect() call means IDE performance is unaffected. This is the right call.
• The EmitRegistrationField refactor correctly unifies the per-class and per-method code paths, removing duplication.
• The namespace alignment is correct: the file-scoped namespace TUnit.Generated; in per-class files ensures all partial class contributions land in the same TUnit.Generated namespace as the infrastructure shell.
• The [ExcludeFromCodeCoverage] on the shell class is a nice touch.
• The PR description is thorough and the performance rationale is sound.

[claude]

Code Review: perf: consolidate module initializers into single .cctor via partial class

Overall this is a well-conceived performance optimization. The design correctly leverages C#'s partial class static field initializer consolidation into a single .cctor, and the PR description clearly explains the trade-offs. The 464 passing snapshot tests give good confidence in the generated output.

---

Issue 1: RegisterReturn exposes an internal implementation detail as public API

SourceRegistrar.cs adds two public static int RegisterReturn overloads. These return a dummy 0 solely as a source-generator code-generation trick — users should never call them.

While the class already has [EditorBrowsable(Never)] (outside DEBUG), these methods still constitute public API: they need versioning, appear in reflection, and can be called by any code bypassing IntelliSense.

Better approach: Separate the source-generator-specific interface from the public API. Either mark them internal (if the source generator and TUnit.Core share an assembly or InternalsVisibleTo) or at minimum add [Obsolete("For generated code use only.", error: false)] with the existing [EditorBrowsable(Never)]. Option A keeps the public API surface minimal and intentional.

---

Issue 2: EmitRegistrationField has a fragile implicit semicolon contract

In TestMetadataGenerator.cs:

writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall}");
//                                                           ^ caller must include trailing ';'

Both existing call sites happen to pass registrarCall strings ending in ;. But this is an invisible constraint. A future contributor adding a third call site without the trailing semicolon would produce syntactically broken generated code — caught only at user compile time, not at generator time.

Better approach: Let the method own the semicolon and require callers to pass just the expression:

// Method adds the semicolon
writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall};");

// Callers pass expression only (no trailing ';')
EmitRegistrationField(writer, uniqueClassName,
    $"SourceRegistrar.RegisterReturn({GenerateTypeReference(...)}, new {uniqueClassName}())");

This makes the contract self-documenting and eliminates the hidden dependency.

---

Issue 3: catch (TypeLoadException) is overly broad

catch (global::System.TypeLoadException) { /* Type not emitted - no test registrations */ }

TypeLoadException is thrown for many reasons beyond the type not being present — e.g., a missing member dependency or a native AOT issue. Silently swallowing these makes failures in TUnit_TestRegistration itself very hard to diagnose.

Note also: a failure inside a field initializer (i.e., inside RegisterReturn) would be wrapped in TypeInitializationException, not TypeLoadException — so the catch block would not swallow initialization failures, which is good. But it's worth documenting that distinction explicitly.

Better approach: Check existence before triggering:

var regType = typeof(global::TUnit.Generated.TUnit_TestRegistration).Assembly
    .GetType("TUnit.Generated.TUnit_TestRegistration");
if (regType != null)
{
    global::System.Runtime.CompilerServices.RuntimeHelpers.RunClassConstructor(regType.TypeHandle);
}

This is precise: it only skips execution when the type genuinely doesn't exist, rather than silently swallowing any TypeLoadException thrown for any reason.

---

Positive aspects

• Incremental generator design is preserved — no .Collect() means zero IDE performance impact. Right call.
• EmitRegistrationField correctly unifies per-class and per-method code paths, removing duplication.
• Namespace alignment is correct: the file-scoped namespace TUnit.Generated; in per-class generated files means all partial class contributions land in the same namespace as the infrastructure shell — the .cctor consolidation works as intended.
• [ExcludeFromCodeCoverage] on the shell class is a thoughtful detail.
• PR description is thorough and the performance rationale is sound.

[claude]

Code Review

The concept here is excellent — replacing N [ModuleInitializer] methods with a single .cctor is a genuine JIT optimization. The hook side of the implementation looks correct. However, there is a critical namespace mismatch bug that would cause test registrations to silently fail.

---

🔴 Critical Bug: TUnit_TestRegistration is in the wrong namespace

TestMetadataGenerator.EmitRegistrationField() (line 2420) emits the partial class in the global namespace:

internal static partial class TUnit_TestRegistration  // global::TUnit_TestRegistration
{
    static readonly int _r_... = SourceRegistrar.RegisterReturn(...);
}

InfrastructureGenerator (line 458, 486-490) declares the shell class in namespace TUnit.Generated and triggers it via the fully-qualified name:

namespace TUnit.Generated
{
    internal static partial class TUnit_TestRegistration { }  // TUnit.Generated.TUnit_TestRegistration
}
// ...
RuntimeHelpers.RunClassConstructor(typeof(global::TUnit.Generated.TUnit_TestRegistration).TypeHandle);

These are two completely different types. The .cctor that runs contains only the empty shell — the test registrations in global::TUnit_TestRegistration are never triggered by RunClassConstructor. All test registrations from TestMetadataGenerator would silently not execute.

Contrast with the hooks path which is correct — HookMetadataGenerator (line 405-407) properly wraps in namespace TUnit.Generated { ... }.

Fix: Wrap the test registration partial class in namespace TUnit.Generated:

private static void EmitRegistrationField(CodeWriter writer, string fieldName, string registrarCall)
{
    writer.AppendLine();
    writer.AppendLine("namespace TUnit.Generated");
    writer.AppendLine("{");
    writer.Indent();
    writer.AppendLine("internal static partial class TUnit_TestRegistration");
    writer.AppendLine("{");
    writer.Indent();
    writer.AppendLine($"static readonly int _r_{fieldName} = {registrarCall}");
    writer.Unindent();
    writer.AppendLine("}");
    writer.Unindent();
    writer.AppendLine("}");
}

This would also need updated snapshot tests (TUnit_TestRegistration would appear with a namespace TUnit.Generated { } wrapper).

---

🟡 catch (TypeLoadException) is dead code

The InfrastructureGenerator always emits the TUnit_TestRegistration and TUnit_HookRegistration shell classes, so TypeLoadException can never be thrown by RunClassConstructor for these types. If the concern is about projects with no tests/hooks, the empty partial class shell still exists. More importantly, if a registration throws at runtime, it would be wrapped in a TypeInitializationException, not a TypeLoadException — so the catch block doesn't protect against real errors either.

The comment /* Type not emitted - no test registrations */ suggests this was meant as a guard for when no source was generated, but the shell is always emitted unconditionally.

Consider either removing the try/catch or catching TypeInitializationException if you want to handle registration failures gracefully.

---

🟡 RegisterReturn is public API with an opaque name

RegisterReturn is now part of the public API surface (visible in Tests.Core_Library_Has_No_API_Changes.*.verified.txt). The name doesn't communicate its purpose well to anyone reading it outside the context of generated code. Since this is only meant to be used by generated code, adding [EditorBrowsable(EditorBrowsableState.Never)] would be appropriate to hide it from IDE autocomplete and signal it's infrastructure-only.

---

✅ What's well-done

• The core optimization idea is sound — merging N .cctor entry points into one is a real JIT win
• The hooks implementation (TUnit_HookRegistration) correctly handles the namespace and the Initialize() → field-initializer → .cctor chain
• RegisterReturn itself is a clean minimal API change
• Snapshot tests are properly updated
• The [ExcludeFromCodeCoverage] on the shell classes is a nice touch

[thomhurst]

Review response

Issue 1 (RegisterReturn as public API): Not implementing. The generated code lives in the user's test assembly, not in any InternalsVisibleTo target. internal would cause CS0122 at compile time. The class already has [EditorBrowsable(Never)] which is the established pattern in this codebase for source-generator-consumed APIs (see memory note: "types used by generated code must be public with [EditorBrowsable(Never)] — not internal, CS0281 error with InternalsVisibleTo").

Issue 2 (semicolon contract): Fixed in 4f37d18. EmitRegistrationField now owns the semicolon; callers pass expression only.

Issue 3 (catch TypeLoadException): The try/catch was actually dead code — InfrastructureGenerator always emits both shell classes (TUnit_TestRegistration and TUnit_HookRegistration), so typeof() always resolves. Removed entirely in 4f37d18. The reviewer's suggested Assembly.GetType() approach would use runtime reflection, which violates the AOT compatibility rule (CLAUDE.md rule 5).

[claude]

Follow-up Review (after feedback addressed in commit 4f37d18)

Thanks for the detailed responses in the review comment. This is a well-executed optimization. Here's my assessment of the current state:

---

Previous issues — all resolved ✅

Semicolon contract (Issue 2): Fixed. EmitRegistrationField now owns the trailing semicolon and callers pass expression-only. The contract is now self-documenting.

catch (TypeLoadException) (Issue 3): Correctly removed as dead code. The comment in InfrastructureGenerator.cs now explains why: InfrastructureGenerator always emits both shell classes unconditionally, so TypeLoadException cannot occur.

RegisterReturn as public API (Issue 1): The explanation is technically correct — generated code runs in the user's assembly, so InternalsVisibleTo cannot apply and internal would cause CS0122. [EditorBrowsable(Never)] is the right approach for this codebase.

Namespace mismatch (previous critical bug report): I need to retract this — it was incorrect. TestMetadataGenerator has private const string GeneratedNamespace = "TUnit.Generated" and emits namespace TUnit.Generated; as a file-scoped namespace. Since file-scoped namespaces apply to the entire file, the TUnit_TestRegistration partial class contributions ARE correctly in TUnit.Generated and match typeof(global::TUnit.Generated.TUnit_TestRegistration) in the infrastructure trigger. The original assessment was wrong.

---

Current implementation quality

The design is sound end-to-end:

• Correctness: File-scoped namespace TUnit.Generated; in every generated test source file ensures all TUnit_TestRegistration partial class fragments land in the correct namespace for the RunClassConstructor trigger.
• Incremental safety: No .Collect() call means the IDE incremental pipeline sees no cross-file dependency — each file is independently cached.
• Hook path parity: HookMetadataGenerator uses an explicit block-scoped namespace TUnit.Generated { ... } wrapper for TUnit_HookRegistration, which is equivalent and correct.
• AOT compatibility: RunClassConstructor with typeof(...).TypeHandle (not reflection by name) is AOT-safe.
• New test coverage: DuplicateTypeNameAcrossAssembliesTests is a good addition that covers the edge case of same-named types across assemblies.

One minor observation (no action required): RegisterReturn returning int 0 and being stored in static readonly int _r_* fields means the JIT may optimize these fields away entirely after the .cctor runs (the stored value is never read). This is fine — the side effect of the method call is the goal, not the stored value. The pattern is idiomatic for this use case.

Overall: LGTM. The optimization is genuine, the implementation is correct, and all previous feedback has been addressed.

[claude]

Follow-up Review (after feedback addressed in commit 4f37d18)

Thanks for the detailed responses in the review comment. This is a well-executed optimization. Here's my assessment of the current state:

---

Previous issues — all resolved ✅

Semicolon contract (Issue 2): Fixed. EmitRegistrationField now owns the trailing semicolon and callers pass expression-only. The contract is now self-documenting.

catch (TypeLoadException) (Issue 3): Correctly removed as dead code. The comment in InfrastructureGenerator.cs now explains why: InfrastructureGenerator always emits both shell classes unconditionally, so TypeLoadException cannot occur.

RegisterReturn as public API (Issue 1): The explanation is technically correct — generated code runs in the user's assembly, so InternalsVisibleTo cannot apply and internal would cause CS0122. [EditorBrowsable(Never)] is the right approach for this codebase.

Namespace mismatch (critical bug from previous review): I need to retract this — it was incorrect. TestMetadataGenerator has private const string GeneratedNamespace = "TUnit.Generated" and emits namespace TUnit.Generated; as a file-scoped namespace. Since file-scoped namespaces apply to the entire file, the TUnit_TestRegistration partial class contributions ARE correctly in TUnit.Generated and match typeof(global::TUnit.Generated.TUnit_TestRegistration) in the infrastructure trigger. My prior analysis was wrong.

---

Current implementation — LGTM

The design is sound end-to-end:

• Correctness: File-scoped namespace TUnit.Generated; in every generated test source file ensures all TUnit_TestRegistration partial class fragments land in the correct namespace for the RunClassConstructor trigger.
• Incremental safety: No .Collect() means the IDE incremental pipeline sees no cross-file dependency — each generated file is independently cached.
• Hook path parity: HookMetadataGenerator uses an explicit block-scoped namespace TUnit.Generated { ... } wrapper for TUnit_HookRegistration, which is equivalent and correct.
• AOT compatibility: RunClassConstructor with typeof(...).TypeHandle (not reflection by name) is AOT-safe.
• New test coverage: DuplicateTypeNameAcrossAssembliesTests covers the cross-assembly duplicate type name edge case.

One observation (no action needed): RegisterReturn returning int 0 stored in static readonly int _r_* fields means the JIT may optimize those fields away entirely after the .cctor runs — the stored value is never read back. This is intentional and idiomatic; the side effect of the call is the goal.

Overall: the optimization is genuine, the implementation is correct, and all previous feedback has been addressed.

[claude]

Follow-up Review (after feedback addressed in commit 4f37d18)

All previous feedback has been addressed. Here is the current assessment:

Previous issues — all resolved ✅

Semicolon contract: Fixed in 4f37d18. EmitRegistrationField now owns the trailing semicolon; callers pass expression-only. Self-documenting.

catch (TypeLoadException): Correctly removed as dead code. InfrastructureGenerator always emits both shell classes unconditionally, so TypeLoadException cannot occur. The explanatory comment left in place is helpful.

RegisterReturn as public API: The reasoning holds — generated code runs in the user's assembly, so InternalsVisibleTo cannot apply and internal would cause CS0122. [EditorBrowsable(Never)] is the correct established pattern.

Namespace mismatch (critical bug from previous review): I need to retract this — it was incorrect. TestMetadataGenerator has private const string GeneratedNamespace = "TUnit.Generated" and emits namespace TUnit.Generated; as a file-scoped namespace declaration. Since file-scoped namespaces apply to the entire file, the TUnit_TestRegistration partial class contributions ARE correctly in TUnit.Generated and match typeof(global::TUnit.Generated.TUnit_TestRegistration) in the infrastructure trigger.

---

Current implementation quality

The design is sound end-to-end:

• Correctness: File-scoped namespace TUnit.Generated; in every generated test source file ensures all TUnit_TestRegistration partial class fragments land in the correct namespace for the RunClassConstructor trigger.
• Incremental safety: No .Collect() call means the IDE incremental pipeline sees no cross-file dependency — each file is independently cached.
• Hook parity: HookMetadataGenerator uses an explicit block-scoped namespace TUnit.Generated { ... } wrapper for TUnit_HookRegistration, which is equivalent and correct.
• AOT compatibility: RunClassConstructor with typeof(...).TypeHandle (not reflection by name) is AOT-safe.
• New test coverage: DuplicateTypeNameAcrossAssembliesTests covers the cross-assembly duplicate type name edge case — good addition.

One minor observation (no action needed): RegisterReturn returning int 0 stored in static readonly int _r_* fields means the JIT may optimize those fields away entirely after the .cctor runs, since the stored value is never read back. This is intentional and idiomatic — the side effect is the goal.

LGTM. The optimization is genuine, the implementation is correct, and all previous feedback has been addressed.

[claude]

Follow-up Review (latest commit: 3c77e66 — "perf: inline hook registration into field initializers")

This final commit completes the optimization arc: hook registrations are now fully inlined into field initializers as single expressions, meaning the .cctor calls only RegisterHook (one JIT method) rather than N per-hook Initialize() methods. The direction is right and the GenerateInlineHookRegistration / GetHookCollectionAndKey logic is correct — I verified that all hook type/kind combinations match the behavior of the old GenerateHookRegistration.

---

🔴 Dead Code: ~172 lines that should be removed

After 3c77e66, GenerateHookRegistration is never called anywhere — the Initialize() method it used to populate no longer exists. This orphans the entire call tree beneath it:

• GenerateHookRegistration (lines 516–617)
• GenerateInstanceHookRegistration (lines 619–628)
• GenerateTypeHookRegistration (lines 630–639)
• GenerateAssemblyHookRegistration (lines 641–651)
• GenerateGlobalHookRegistration (lines 653–660)
• GetConcreteHookType (lines 662–687)

That's ~172 lines of dead code. These were the implementation helpers for the intermediate "Initialize() per hook" approach from commit cb97b83, but they were never cleaned up after the inline approach replaced them.

Why this matters: Dead code isn't free — it confuses future contributors who may not realize it's unreachable, increases the maintenance surface, and creates risk of accidentally re-introducing the old approach. Since these are private static methods in a generator class, the compiler won't warn about them.

Fix: Delete methods on lines 516–687 in HookMetadataGenerator.cs.

---

🟡 Minor: Naming asymmetry between RegisterReturn and RegisterHook

Both are generated-code-only APIs with [EditorBrowsable(Never)] and return 0 as a dummy value for the field initializer pattern. But they use different naming conventions:

• Test registrations: RegisterReturn(Type, ITestSource) — named after the mechanism
• Hook registrations: RegisterHook<T>(dictionary, key, hook) — named after the intent

RegisterHook is the better pattern since it describes what the method actually does. No action required if the asymmetry is intentional, but worth flagging in case a rename of RegisterReturn → RegisterTest or RegisterTestSource is considered.

---

✅ Correctness confirmed

• GetHookCollectionAndKey switch covers all 16 hook type/kind combinations and correctly matches the old GenerateHookRegistration dispatch logic.
• RegisterHook<T> for Assembly keys uses typeof({typeDisplay}).Assembly passed as a single argument — cleaner than the old approach which evaluated .Assembly twice (once for GetOrAdd, once for the indexer).
• The {safeFileName}Initializer class is still generated (to house the {delegateKey}_Body delegate), just without the Initialize() method — the delegate reference in GenerateHookObject via delegatePrefix is correct.
• GenerateHookObject's optional delegatePrefix parameter is a clean API extension.

---

Summary: LGTM with one actionable cleanup item: remove the ~172 lines of dead code from the superseded GenerateHookRegistration call tree. The optimization is genuine and the correctness is solid.

[claude]

Follow-up Review (latest commit: 7f23193 — "chore: remove ~170 lines of dead hook registration code")

All previously raised issues have been addressed across the commit history. Here is the final assessment.

---

Previous issues — all resolved ✅

Dead code (~172 lines): Removed in 7f231930. GenerateHookRegistration and its entire call tree are gone. The file is now clean.

Semicolon contract: Fixed in 4f37d18. EmitRegistrationField owns the trailing semicolon; callers pass expression-only.

catch (TypeLoadException): Correctly removed as dead code. InfrastructureGenerator always emits both shell classes unconditionally.

Namespace mismatch (retracted): File-scoped namespace TUnit.Generated; in TestMetadataGenerator-generated files means all TUnit_TestRegistration partial class contributions ARE in TUnit.Generated. Confirmed correct.

RegisterReturn as public API: Accepted reasoning — generated code runs in the user's assembly, so internal + InternalsVisibleTo is not viable. [EditorBrowsable(Never)] on the class is the correct established pattern.

---

Minor observation (no action required)

In GetHookCollectionAndKey, the arms ("Test", "Before", false) and ("Test", "After", false) are currently unreachable because isInstance is always true when hookKind is "Before" or "After" and hookType == "Test". They map to the correct collection names, so this is not a correctness issue — just defensive code. Could be a _ wildcard simplification:

This is a very minor point — worth noting for future clarity but not blocking.

---

Overall assessment

The optimization is genuine and correctly implemented end-to-end:

• JIT reduction: N methods → 1 per registration class is a real win for large test suites
• Incremental safety: No means per-file independent caching — zero IDE regression
• AOT compatibility: is AOT-safe
• Hook correctness: covers all 16 hook type/kind combinations, verified against
• Unconditional shell emission: Both and shells are always emitted, so never throws

LGTM. All feedback has been addressed. This is a clean, well-executed performance optimization.

[claude]

Follow-up Review (latest commit: 7f23193 — "chore: remove ~170 lines of dead hook registration code")

All previously raised issues have been addressed across the commit history. Here is the final assessment.

---

Previous issues — all resolved ✅

Dead code (~172 lines): Removed in 7f231930. GenerateHookRegistration and its entire call tree are gone. The file is now clean.

Semicolon contract: Fixed in 4f37d18. EmitRegistrationField owns the trailing semicolon; callers pass expression-only.

catch (TypeLoadException): Correctly removed as dead code. InfrastructureGenerator always emits both shell classes unconditionally.

Namespace mismatch (retracted): File-scoped namespace TUnit.Generated; in TestMetadataGenerator-generated files means all TUnit_TestRegistration partial class contributions ARE in TUnit.Generated. Confirmed correct.

RegisterReturn as public API: Accepted reasoning — generated code runs in the user's assembly, so internal + InternalsVisibleTo is not viable. [EditorBrowsable(Never)] on the class is the correct established pattern.

---

Minor observation (no action required)

In GetHookCollectionAndKey, the arms ("Test", "Before", false) and ("Test", "After", false) are currently unreachable because isInstance is always true when hookKind is "Before" or "After" and hookType == "Test". They map to the correct collection names, so this is not a correctness issue — just defensive code. Could be simplified with a _ wildcard:

// Currently (unreachable arms exist):
("Test", "Before", true) => ("BeforeTestHooks", ...),
("Test", "After", true) => ("AfterTestHooks", ...),
("Test", "Before", false) => ("BeforeEveryTestHooks", null),  // unreachable
("Test", "After", false) => ("AfterEveryTestHooks", null),   // unreachable
("Test", "BeforeEvery", _) => ("BeforeEveryTestHooks", null),
("Test", "AfterEvery", _) => ("AfterEveryTestHooks", null),

// Simplified (no behavior change):
("Test", "Before", true) => ("BeforeTestHooks", ...),
("Test", "After", true) => ("AfterTestHooks", ...),
("Test", "BeforeEvery" or "Before", _) => ("BeforeEveryTestHooks", null),
("Test", "AfterEvery" or "After", _) => ("AfterEveryTestHooks", null),

This is a very minor point — not blocking.

---

Overall assessment

The optimization is genuine and correctly implemented end-to-end:

• JIT reduction: N [ModuleInitializer] methods → 1 .cctor per registration class is a real win for large test suites
• Incremental safety: No .Collect() means per-file independent caching — zero IDE regression
• AOT compatibility: RunClassConstructor(typeof(...).TypeHandle) is AOT-safe
• Hook correctness: GetHookCollectionAndKey covers all 16 hook type/kind combinations, verified against Sources.cs
• Unconditional shell emission: Both TUnit_TestRegistration and TUnit_HookRegistration shells are always emitted, so RunClassConstructor never throws

LGTM. All feedback has been addressed. This is a clean, well-executed performance optimization.