Collection validation dispatches through reflection and re-resolves the collection per pass

[phmatray]

Problem / motivation

DynamicFormValidator<TModel> cannot name the item type of a collection — TItem exists only on
CollectionFieldValidator<TModel, TItem> — so it constructs and calls that validator reflectively.
That one untyped seam produces two costs, neither user-visible today.

1. Every call is still a reflective dispatch

#331 cached the plumbing per configuration, so MakeGenericType, Activator.CreateInstance and
GetMethod now run once. The calls themselves did not change:

// FormCraft/Forms/Validation/DynamicFormValidator.cs — CollectionValidatorInvoker
internal Task<List<CollectionItemError>> ValidateItemFieldAsync(
    object model, int itemIndex, string fieldName, IServiceProvider services)
    => (Task<List<CollectionItemError>>)_validateCell.Invoke(
        _validator, [model, itemIndex, fieldName, services])!;

Every collection keystroke therefore pays a MethodInfo.Invoke, an object[] allocation and a boxed
itemIndex, and the model parameter is widened to object — so a signature change on
CollectionFieldValidator fails at runtime, not at compile time. The invoker's constructor throws on
an unresolvable method precisely because that class of mistake cannot be caught by the compiler here.

2. The collection is resolved more than once per pass

ValidateAllAsync awaits ValidateItemsAsync, which calls _configuration.CollectionAccessor(model),
and then calls BuildMessages, which calls the accessor again for the item count
(FormCraft/Forms/Validators/CollectionFieldValidator.cs).

For an ordinary auto-property that is a wasted call. For a property that materialises per get —

public List<Item> Items => _set.ToList();   // a new list on every access

— the two calls return different lists, so the MinItems/MaxItems rules are measured against a
different snapshot than the items that were validated. The messages then describe two different
collections.

Pre-existing: the old ValidateAsync had the same double resolution. #331 made it explicit by
extracting BuildMessages, without changing it.

Proposed solution

Close the seam rather than cache around it.

Give CollectionFieldValidator<TModel, TItem> a non-generic surface — an interface exposing
ValidateAllAsync(object model, IServiceProvider services) and
ValidateItemFieldAsync(object model, int itemIndex, string fieldName, IServiceProvider services), or
a CreateValidator() factory on ICollectionFieldConfigurationBase that returns it. DynamicFormValidator
then holds something directly callable: both Invokes, both casts, the object[]s, the boxing and the
constructor's defensive throws all disappear, and a signature change becomes a compile error.

Resolve the collection once per pass — have ValidateAllAsync read the accessor once and thread
the resulting list into both the item traversal and the count rules. Note this changes the shape of the
public ValidateItemsAsync, which currently resolves its own; keep that method working for external
callers.

Neither change alters behaviour, so the existing suites are the contract.

Alternatives considered

• Leave it. Defensible: fix(core): validate each collection item field once per pass (#329) #331 already removed the per-call type resolution, which was the bulk of
  the cost, and nothing is user-visible. The argument against is the type safety, not the speed — this
  is the only place in the validation path where a wrong signature reaches production as a runtime
  failure.
• Source-generate a typed dispatcher. Removes reflection entirely with no interface to design, but
  adds a generator to a library that has none, for one call site.
• Fix only the double resolution. The smaller half and independently correct, but it leaves the
  seam that motivates the whole issue.

Area

FormCraft — core validation pipeline

---

Follow-up from #331. Related: #329, #312, #314

🧠 Brainstorm

Problem / context

This is the residue of a sequence: #269 cached the render path's compiled getter, #312 extended that to
validation and unified both onto one cache, #329 stopped collection validation running twice and cached
its reflective plumbing. Each removed the dominant cost of its day. What is left is the mechanism —
reflection as the calling convention between a component that knows TModel and a validator that also
needs TItem.

#314 is the same shape one layer over: RenderField resolving the field type by reflection per render.
Whatever pattern resolves this one is worth reusing there.

Approaches

A. Non-generic interface on the validator.
Pros: deletes every reflective call and restores compile-time checking; small and local; the invoker
class in DynamicFormValidator collapses to a field. Cons: an object-typed interface is itself
mildly untyped — it moves the cast inside the validator rather than removing it.

B. CreateValidator() factory on ICollectionFieldConfigurationBase.
Pros: the configuration already knows its TItem, so it is the natural place to construct the
validator; removes Activator.CreateInstance too. Cons: widens a configuration interface with a
behavioural method, which is a heavier abstraction than the problem needs.

C. Cache more aggressively (compiled delegates over MethodInfo).
Pros: no API change at all. Cons: keeps the runtime-failure mode, and trades reflection for
expression-compilation — the exact cost #269/#312 spent two PRs removing.

Recommendation

A, with the accessor fix folded in since both live in the same two files and neither changes
behaviour. C is rejected on principle: adding Expression.Compile() back into this path to avoid
MethodInfo.Invoke would be undoing #312 to optimise #329.

📋 Spec

Goal

DynamicFormValidator calls the collection validator through a typed surface, and one validation pass
reads the collection once.

Scope

• A non-generic surface on CollectionFieldValidator<TModel, TItem> that DynamicFormValidator can
  hold and call directly.
• One accessor call per pass, threaded into both the item traversal and the count rules.

Non-goals

• Any behaviour change: same messages, identifiers, ordering, and one invocation per item field (Collection validation runs the whole collection twice per pass, so item validators fire twice #329).
• The render path's reflection (GetActualFieldType) — that is RenderField resolves the field type by reflection on every render #314.
• Removing ValidateAsync / ValidateItemsAsync from the public surface.

Behaviour

flowchart LR
  subgraph before
    A1[DynamicFormValidator] -->|MethodInfo.Invoke<br/>object[] + boxing| B1["CollectionFieldValidator&lt;TModel,TItem&gt;"]
    B1 --> C1[accessor call #1<br/>item traversal]
    B1 --> D1[accessor call #2<br/>count rules]
  end
  subgraph after
    A2[DynamicFormValidator] -->|typed interface call| B2["CollectionFieldValidator&lt;TModel,TItem&gt;"]
    B2 --> C2[one accessor call]
    C2 --> D2[traversal + count rules]
  end

Loading

Key files

• FormCraft/Forms/Validation/DynamicFormValidator.cs — CollectionValidatorInvoker and its two call
  sites.
• FormCraft/Forms/Validators/CollectionFieldValidator.cs — ValidateAllAsync, ValidateItemsAsync,
  BuildMessages.
• FormCraft/Forms/Core/ICollectionFieldConfiguration.cs — only if the factory approach is chosen.

Validation rules

• Every existing suite passes unmodified — this changes no behaviour.
• Each item field's validators are still invoked exactly once per pass (Collection validation runs the whole collection twice per pass, so item validators fire twice #329).
• A cell edit still validates only that cell, and still reports every configuration declared for it.
• A collection property that returns a new list per access has its count rules and its item messages
  describing the same snapshot.
• Nested Items[i].Field attribution is unchanged (Improve nested field identification for collection fields using Blazor's FieldIdentifier system #91).

Edge cases

• Two collections of the same item type must still get their own validator — fix(core): validate each collection item field once per pass (#329) #331 keys the cache by
  configuration instance for this reason, and a factory approach must preserve it.
• A null collection must short-circuit as it does today, before any count rule runs.
• ValidateItemsAsync called directly by an external consumer keeps resolving its own collection.

Assumptions

• No external consumer depends on the reflective shape (nobody is calling these through reflection
  themselves), so introducing a typed surface is additive.

🛠️ Implementation plan

For agentic workers: REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (- [ ]) syntax for tracking.

Goal: replace the reflective calling convention with a typed one, and read the collection once per
pass.

Architecture: all changes in FormCraft (core). No UI-framework types. Behaviour-preserving — the
existing suites are the contract.

Tech stack: .NET 8 / 10 multi-target, xUnit + Shouldly.

Global constraints:

• Base branch dev; commit as Philippe Matray <phmatray@gmail.com>; conventional commits.
• TreatWarningsAsErrors=true — the build fails on any warning.
• Per-suite filter: dotnet test <project>.csproj -c Release -- --filter-class <FQN> (never
  dotnet test --filter). Run the full suite before claiming done.
• Preserve Collection validation runs the whole collection twice per pass, so item validators fire twice #329's one-invocation-per-pass and Improve nested field identification for collection fields using Blazor's FieldIdentifier system #91's nested attribution.

Task 1: Pin the snapshot consistency the accessor fix must deliver

Files: modify FormCraft.UnitTests/Validators/CollectionFieldValidatorTests.cs.

Interfaces: none — tests only.

• Step 1: Write a failing test using a model whose collection property returns a new list per access (=> _set.ToList()), with MinItems set so the count rule fires, asserting the count message and the item messages describe the same snapshot.
• Step 2: Run the suite → observe the failure and record in the test what it actually reports today; if the double resolution happens to agree for a stable backing set, make the property's contents differ per access so the inconsistency is forced.
• Step 3: Commit: test(core): pin that collection count and item messages share one snapshot.

Task 2: Resolve the collection once per pass

Files: modify FormCraft/Forms/Validators/CollectionFieldValidator.cs.

Interfaces: an internal traversal taking the resolved list; ValidateItemsAsync keeps its public shape.

• Step 1: Extract the item traversal so it accepts an already-resolved list, and have ValidateAllAsync resolve once and pass it to both the traversal and BuildMessages.
• Step 2: Keep the public ValidateItemsAsync resolving its own collection and delegating, so external callers are unaffected.
• Step 3: Run the suite → Task 1's test PASSES and every existing suite still does.
• Step 4: Commit: fix(core): read a collection once per validation pass.

Task 3: Replace the reflective calls with a typed surface

Files: modify FormCraft/Forms/Validators/CollectionFieldValidator.cs and FormCraft/Forms/Validation/DynamicFormValidator.cs; add the new interface.

Interfaces: a non-generic surface exposing ValidateAllAsync(object, IServiceProvider) and ValidateItemFieldAsync(object, int, string, IServiceProvider).

• Step 1: Decide between the non-generic interface and a CreateValidator() factory on ICollectionFieldConfigurationBase, and record the reasoning in the surviving type's XML docs.
• Step 2: Implement it on CollectionFieldValidator<TModel, TItem>, casting the object model once at the boundary.
• Step 3: Replace CollectionValidatorInvoker's MethodInfo.Invoke calls with direct interface calls, deleting the GetMethod lookups, the casts, the object[]s and the constructor's defensive throws that only existed because the compiler could not check this.
• Step 4: Keep the cache keyed by configuration instance, not item type — two collections of the same item type must not share a validator (fix(core): validate each collection item field once per pass (#329) #331 pins this).
• Step 5: Run dotnet build -c Release and the full dotnet test -c Release → both green, no warnings, all three assemblies reporting.
• Step 6: Commit: refactor(core): call the collection validator through a typed surface.