Skip to content

Validation: report a loaded-but-invalid shared sub-architecture's errors once #2822

Description

@LeighFinegold

Feature Proposal

Follow-up to PR #2805 (validation engine redesign). During review @willosborne noted that a
sub-architecture with faults that is referenced by many nodes could get "very, very noisy" if its
errors are reported once per referencing node, and that ideally such errors should be reported once,
but that this can be deferred to keep #2805 mergeable. This issue captures that follow-up.

Target Project:

shared/ (validation engine), primarily
shared/src/commands/validate/validate-node-details.ts.

Description of Feature:

De-duplicate validation findings for a detailed-architecture sub-architecture that loads
successfully but is itself invalid
and is referenced by multiple nodes, so its intrinsic errors are
reported once rather than repeated at every referencing node's path.

Note the deliberate asymmetry agreed on #2805:

  • A failed-to-load reference is reported at every referencing node (so the user can fix all call
    sites at once), implemented in feat(shared): Validation Engine Redesign with articulated assumptions #2805.
  • A loaded-but-invalid sub-architecture's own errors should be reported once (this issue), because
    repeating potentially many intrinsic errors across many referencing nodes is pure noise.

User Stories:

  • As an architect, I want the intrinsic validation errors of a shared, invalid sub-architecture to be
    reported once so that a widely-referenced faulty sub-architecture does not flood my output with
    duplicate findings.
  • As an architect, I still want a sub-architecture that fails to load to be reported at every
    referencing node (unchanged from feat(shared): Validation Engine Redesign with articulated assumptions #2805), so I can find and fix all the broken references.

Current Limitations:

validate-node-details de-duplicates traversal (each sub-architecture is validated once via the
CachingTrackingResolver seen/cache tracking), so in the common case a successfully-loaded
sub-architecture is only validated (and therefore only reported) once. This issue exists to:

  • confirm and lock that behaviour with explicit tests, and
  • define the intended semantics if/when a future change (e.g. per-node contextual validation, or the
    internal "validate against a specified required-pattern" requirement) causes the same
    sub-architecture to be validated in more than one context, which could reintroduce duplicate
    intrinsic findings.

Proposed Implementation:

  • Add explicit tests asserting that a loaded-but-invalid sub-architecture referenced by N nodes yields
    its intrinsic errors once (attributed to a single, well-defined path), while a failed-to-load
    reference still yields one finding per referencing node.
  • If duplication is reintroduced by later work, de-duplicate intrinsic sub-architecture findings by
    (sub-architecture URL + finding identity), keeping the failed-load per-node behaviour separate.
  • Technical design considerations:
    • Decide the canonical path attribution for once-only intrinsic findings (first referencing node,
      the sub-architecture URL itself, or a synthesised path).
    • Ensure de-duplication keys on stable finding identity (code + path + message) to avoid dropping
      genuinely distinct findings.
  • API changes: none expected (internal to node-details result aggregation).
  • Data model changes: none.
  • Dependencies on other components: interacts with Issue (cycle detection) follow-up and any future
    required-pattern contextual validation.

Alternatives Considered:

Testing Strategy:

Documentation Requirements:

  • Update shared/src/commands/validate/ValidationTechnicalSpecification.md §6/§8 to record the
    agreed asymmetry (failed-load per node vs invalid-content once).

Implementation Checklist:

  • Design reviewed and approved
  • Implementation completed
  • Tests written and passing
  • Documentation updated
  • Relevant workflows updated (if needed)
  • Performance impact assessed

Additional Context:

Split out of PR #2805 to keep that PR mergeable. See @willosborne's review comment on #2805 (point 3)
and ValidationTechnicalSpecification.md §8.

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions