Collection validation reports messages the form never shows

[phmatray]

Problem / motivation

A collection field carries two message sets: the flat, human-formatted strings on the collection's
own field identifier (what a ValidationSummary shows), and the nested Items[i].Field messages that
render beside each row (#91). Nothing keeps those two — or either of them and what the form actually
renders — in agreement.

The result is messages the user cannot see or act on. Two verified instances:

1. A hidden collection blocks submit with an invisible error

ValidateModelAsync's collection loop never checks visibility, although the ordinary-field loop
twenty lines above does:

// FormCraft/Forms/Validation/DynamicFormValidator.cs:102 — the ORDINARY field loop
if (!IsFieldVisible(field, model))
{
    continue;   // hidden fields must not block submission with invisible errors
}

IsFieldVisible appears exactly twice in the file — that call at :102 and its own declaration at
:153. The collection loop has no equivalent, while both adapters' render loops do skip invisible
collection fields. So a collection with IsVisible = false and a required item field left empty makes
ValidateModelAsync return false and attach messages for a field that renders nothing: the form
reports invalid, the submit button stops working, and there is no control on screen to correct.

This is precisely the failure the ValidateCollections parameter's own XML remarks already describe:

reports invalid with nothing on screen to explain why … the submit button silently stops working

That parameter exists so an adapter which renders no collection UI can opt out wholesale. This issue
is the same hazard one level down: an adapter that renders collections, with one collection hidden.

2. The flat message goes stale the moment a cell is corrected

The field-changed path clears and refreshes only the nested identifier:

// FormCraft/Forms/Validation/DynamicFormValidator.cs:321
_messageStore!.Clear(fieldIdentifier);   // ← Items[0].ProductName only

The collection's own identifier is never touched outside a full pass. So after the user fixes row 1,
the inline message beside the row disappears while a ValidationSummary keeps showing
Items [1] - Product: Product name is required until the next submit — the form says "fix this" about
something already fixed.

Pre-existing, but #331 restructured this exact method and added a test pinning the flat messages after
a full pass without pinning their refresh, so nothing currently guards it.

Proposed solution

Make the flat set a maintained projection of the same state the nested set reflects, rather than a
parallel set updated on a different schedule:

1. Skip validation for a collection the form does not render, matching how ordinary fields are
   treated. This needs one decision: does VisibilityCondition participate — as it does for ordinary
   fields, via IsFieldVisible — or only the static IsVisible flag? Ordinary-field parity argues for
   the former.
2. Refresh the flat set when a cell changes. Second decision: remove just the flat line belonging
   to the edited cell, or recompute the whole flat set for that collection? Recomputing is simpler to
   reason about and cannot drift, but it re-runs the item-count rules; removing one line is cheaper
   but has to reproduce the exact formatting to find its own line.

Whatever lands must preserve #91's nested attribution and #329's one-invocation-per-pass guarantee —
in particular, a refresh must not reintroduce a second full traversal.

Alternatives considered

• Drop the flat set entirely and let ValidationSummary read the nested messages. Removes the
  coherence problem by removing the duplication — but the flat strings carry context the nested ones
  do not (Items [1] - Product: … names the row and field), and it is public-facing behaviour that
  existing forms display today.
• Fix only the hidden-field case. It is the more severe of the two — a form that cannot be
  submitted beats a form showing a stale line — but both stem from the same missing coherence rule,
  and the second is cheap once the first is being worked.
• Do nothing. The hidden-collection case is silent and total: no error text, no console warning,
  just a submit button that stops responding. That is the hardest class of bug to report, which is the
  argument against leaving it.

Area

FormCraft — core validation pipeline

---

Follow-up from #331. Related: #329, #91, #279

🧠 Brainstorm

Problem / context

#329 fixed how often collection validation runs. This is about what it produces. The two message
sets were introduced at different times — the flat strings first, the nested Items[i].Field
identifiers in #91 — and the rules that should govern both were only ever applied to one.

Visibility is the clearest example: the ordinary-field loop grew an IsFieldVisible guard for exactly
this reason ("hidden fields must not block submission with invisible errors", per its own comment), and
the collection loop, added later, never got one.

Approaches

A. Give the collection loop the same visibility rule, and refresh the flat set on a cell change.
Pros: restores parity with ordinary fields, which is the principle the codebase already states; both
fixes land in the one method pair that owns this. Cons: the refresh needs a decision about scope
(one line vs. recompute), and recomputing risks re-running the item-count rules more often than today.

B. Fix visibility only.
Pros: smallest change for the most severe symptom. Cons: leaves a known stale-message path in code
that was just restructured, with a test pinning the flat messages that does not cover their refresh.

C. Collapse the two message sets into one.
Pros: the coherence problem cannot recur if there is nothing to keep in sync. Cons: changes what
existing forms display, and loses the row/field context the flat strings carry.

Recommendation

A, with the visibility fix first since it is the user-blocking half, and VisibilityCondition
included so collections behave exactly as ordinary fields do. C is the tempting "root cause" answer
but it trades an internal inconsistency for a public behaviour change, which is a worse deal.

📋 Spec

Goal

Every validation message a collection produces corresponds to something the user can see and correct.

Scope

• Skip collection validation for a collection field the form does not render.
• Refresh the collection's flat message set when one of its cells changes.

Non-goals

• Removing or reformatting the flat message strings (that is alternative C).
• Changing nested Items[i].Field attribution (Improve nested field identification for collection fields using Blazor's FieldIdentifier system #91).
• Re-running the per-item traversal more than once per pass (Collection validation runs the whole collection twice per pass, so item validators fire twice #329).

Behaviour

flowchart TB
  subgraph now
    A1[hidden collection] --> B1[validated anyway]
    B1 --> C1["invalid + messages<br/>for a field that renders nothing"]
    D1[user fixes a cell] --> E1[nested message cleared]
    D1 --> F1[flat message untouched → stale]
  end
  subgraph after
    A2[hidden collection] --> B2[skipped, like an ordinary hidden field]
    D2[user fixes a cell] --> E2[nested message cleared]
    D2 --> F2[flat set refreshed for that collection]
  end

Loading

Key files

• FormCraft/Forms/Validation/DynamicFormValidator.cs — :102 (the ordinary-field guard to mirror),
  :153 (IsFieldVisible), the collection loop below it, and :321 (the cell-change clear).
• FormCraft/Forms/Validators/CollectionFieldValidator.cs — where the flat strings are built.

Validation rules

• A collection with IsVisible = false and an invalid item does not make the model invalid and
  adds no messages on any identifier.
• A collection whose VisibilityCondition returns false behaves the same way (decision above).
• A visible collection behaves exactly as today — flat and nested messages unchanged in text and order.
• After correcting a cell, the collection's flat set no longer contains that cell's line.
• A cell correction still invokes only that cell's validators (Collection validation runs the whole collection twice per pass, so item validators fire twice #329).

Edge cases

• Visibility flipping between passes — a collection hidden after producing messages must have them
  cleared, not stranded; a full pass clears the store first, so the risk is the field-changed path.
• Item-count rules on a hidden collection — MinItems/MaxItems are collection-level, not per-item,
  but they are equally invisible; skipping the collection skips these too, which is the intended
  reading of "the user cannot act on it".
• ValidateCollections = false (Move the UI-agnostic adapter machinery into FormCraft core #279) must keep short-circuiting the whole path first.

Assumptions

• Collection fields expose IsVisible and VisibilityCondition comparably to ordinary fields. Verify
  when implementing: ICollectionFieldConfigurationBase may expose only a subset, and if
  VisibilityCondition is absent there, the decision above resolves itself to the static flag.

🛠️ 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: collection validation only reports what the user can see and act on.

Architecture: all changes in FormCraft (core). No UI-framework types. DynamicFormValidator is
shared by both adapters since #279 — both must stay green.

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

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, which MTP ignores). Run the full suite before claiming done.
• Preserve Improve nested field identification for collection fields using Blazor's FieldIdentifier system #91 nested attribution and Collection validation runs the whole collection twice per pass, so item validators fire twice #329's one-invocation-per-pass guarantee.

Task 1: Establish what a hidden collection does today

Files: modify FormCraft.UnitTests/Validation/CollectionValidationPassTests.cs.

Interfaces: none — tests only.

• Step 1: Check what ICollectionFieldConfigurationBase actually exposes for visibility (IsVisible, and whether a VisibilityCondition exists) and record the answer in the test file's header comment — it decides the guard's shape.
• Step 2: Write a failing test: a collection field with IsVisible = false whose item field is required and empty must leave the model valid and add no messages.
• Step 3: Run the suite → FAIL (currently invalid, with messages on both identifiers).
• Step 4: Commit: test(core): pin that a hidden collection is not validated.

Task 2: Skip validation for a collection the form does not render

Files: modify FormCraft/Forms/Validation/DynamicFormValidator.cs.

Interfaces: the collection loop gains the visibility guard the ordinary-field loop already has.

• Step 1: Add the guard to the collection loop, mirroring IsFieldVisible (including VisibilityCondition if the type exposes it, per Task 1 Step 1).
• Step 2: Run the suite → Task 1's test PASSES; every existing suite still does, both adapters included.
• Step 3: Commit: fix(core): do not validate a collection field the form does not render.

Task 3: Refresh the flat message set when a cell changes

Files: modify FormCraft/Forms/Validation/DynamicFormValidator.cs; test as above.

Interfaces: the field-changed path updates the collection's own identifier as well as the nested one.

• Step 1: Write a failing test: validate a two-row collection with row 0 invalid, correct row 0, raise the field-change notification, and assert the collection's flat message set no longer mentions row 1's line.
• Step 2: Run the suite → FAIL (the flat line survives).
• Step 3: Decide between removing the edited cell's flat line and recomputing the collection's flat set, and write the reasoning into the method's XML docs.
• Step 4: Implement it without reintroducing a second per-item traversal (Collection validation runs the whole collection twice per pass, so item validators fire twice #329) — assert with a counting validator that a cell edit still invokes only that cell's validators.
• Step 5: Run the suite → PASS, with the Improve nested field identification for collection fields using Blazor's FieldIdentifier system #91 attribution tests green.
• Step 6: Commit: fix(core): refresh a collection's flat messages when a cell changes.

Task 4: Verify both adapters and release-note it

Files: test FormCraft.ForMudBlazor.UnitTests; modify README.md.

Interfaces: none new.

• Step 1: Add a MudBlazor test rendering a form with a hidden collection field, asserting it submits.
• Step 2: Run dotnet build -c Release and the full dotnet test -c Release → green, no warnings, all three assemblies reporting.
• Step 3: Add a bullet under the README's ## 🎉 Unreleased — call out that a hidden collection no longer blocks submit, since a form relying on the old behaviour would change.
• Step 4: Commit: fix(core): keep collection validation messages consistent with the rendered form.