diff --git a/docs/design/sysml2-tools-core/layout/internal/expose-scope-resolver.md b/docs/design/sysml2-tools-core/layout/internal/expose-scope-resolver.md index c150715f..de655c4b 100644 --- a/docs/design/sysml2-tools-core/layout/internal/expose-scope-resolver.md +++ b/docs/design/sysml2-tools-core/layout/internal/expose-scope-resolver.md @@ -90,6 +90,29 @@ Returns `true` when `qualifiedName` matches one of `scope.Subjects` per that sub or is an exact match of one of `scope.ExplicitMembers` (a bracket-filter-matched definition or usage). Used by every strategy to decide whether a candidate element belongs in a scoped diagram. +###### `MatchesUnlimitedSubject(string qualifiedName, ExposedScope scope)` + +A narrower sibling of `IsInSubjectScope`, added for `InterconnectionViewLayoutStrategy`'s per-branch +depth-limited recursion (see +`docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md`'s +*Per-Branch Depth-Limited Recursion* section). Returns `true` only when `qualifiedName` matches one +of `scope.Subjects` (by the same equals-or-subtree-prefix / direct-child rule `IsInSubjectScope` +uses per recursion kind) **and** that specific matched subject's own `ExposeRecursionKind` is +`MembershipRecursive` or `NamespaceRecursive` — i.e. only ever `true` for a match against an +unlimited-depth (`expose X::**;` / `expose X::*::**;`) subject. It never returns `true` via +`scope.ExplicitMembers` (a bracket-filter match is always exact-only, regardless of any subject's +recursion kind elsewhere in the same scope) and never returns `true` merely because *some other*, +unrelated subject in `scope.Subjects` happens to be recursive — only the specific subject that +`qualifiedName` itself matches is considered. Unlike `IsInSubjectScope` (which answers "is this +qualified name in scope at all, considering every subject's own recursion kind together"), this +method deliberately does **not** re-derive its answer from `IsInSubjectScope`'s combined `bool` +result: the caller needs to know not just *whether* `qualifiedName` is in scope, but specifically +*which* subject matched it and *that* subject's own recursion kind — information `IsInSubjectScope` +does not expose. When `qualifiedName` matches more than one subject with different recursion kinds +(possible when two overlapping `expose` statements both name an ancestor of the same feature), this +method returns `true` if *any* matched subject is recursive — most-permissive-wins, the same +inclusive-OR semantics `IsInSubjectScope` itself already uses across subjects. + ###### `IsRootRelevantToScope(string candidateQualifiedName, ExposedScope scope)` Returns `true` when `candidateQualifiedName` (a candidate single-root diagram root, e.g. the diff --git a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index 9e115bf6..9aebb1a8 100644 --- a/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md +++ b/docs/design/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md @@ -12,9 +12,12 @@ container box for the host definition. `InterconnectionViewLayoutStrategy` has no instance state; all input arrives through the `BuildLayout` parameters. Layout constants (`MinPartWidth`, `CharWidthFactor`) are declared as -`private const double` fields. Three private records carry intermediate data: `PartItem` (a nested +`private const double` fields. Four private records carry intermediate data: `PartItem` (a nested part usage with its computed box size, typing, and — when the part is a container — its -pre-laid-out `InnerContent`), `ConnPair` (a resolved binary connection between two nested-part +pre-laid-out `InnerContent`), `TopLevelPart` (pairs a top-level-fallback `PartItem` with its owning +definition's qualified name, `OwnerQualifiedName`, so `ResolveTopLevelConnections` can scope +endpoint resolution correctly per owning definition — see _No-single-root scoped fallback_ below), +`ConnPair` (a resolved binary connection between two nested-part indices, together with the optional port-name label for each end), and `InteriorLayout` (the full container size and content produced by laying out one definition's interior). @@ -26,15 +29,79 @@ Entry point. Resolves the view's `expose` scope via `ExposeScopeResolver.Resolve selects the root part definition via `FindRoot(workspace, scope)`, builds the container-definition index via `BuildDefinitionIndex`, lays out the root's interior via `LayOutInterior` (applying `scope`'s namespace-prefix filter only at the root's own direct children, depth 0; every deeper -recursive call passes `scope: null` so a nested container's own interior always shows its full -composition structure regardless of which namespace it — or the view's exposed subject — happens to -be declared in, since composition structure and namespace/file organization are independent in -SysML v2), and assembles the root container box with the interior -content nested as that box's own `Children` (mirroring the nesting `MakePartBox` already uses for a -container part, so the root box is never a bare sibling of its own content) into the `LayoutTree`. -When `FindRoot` selects no root, falls back to the **no-single-root scoped fallback** described -below when the resolved scope directly includes one or more top-level `part` feature usages; -otherwise returns a minimal 200×100 empty `LayoutTree`. +recursive call passes `scope: null`, since a nested container's own interior's membership in the +exposed scope is never re-derived from its own qualified name — composition structure and +namespace/file organization are independent in SysML v2 — but whether that deeper interior is +expanded **at all** is gated by a per-branch `ancestorUnlimitedRecursion` flag, decided once per +depth-0 feature and threaded unchanged through every recursive call beneath it; see +_Per-Branch Depth-Limited Recursion_ below; the true root call passes `ancestorUnlimitedRecursion: +null`, since no branch decision exists yet at that point), and assembles the root container box +with the interior content nested as that box's own `Children` (mirroring the nesting `MakePartBox` +already uses for a container part, so the root box is never a bare sibling of its own content) into +the `LayoutTree`. When `FindRoot` selects no root, falls back to the **no-single-root scoped +fallback** described below when the resolved scope directly includes one or more top-level `part` +feature usages; otherwise returns a minimal 200×100 empty `LayoutTree`. + +###### Per-Branch Depth-Limited Recursion (`DetermineBranchUnlimitedRecursion`, `MatchesUnlimitedSubject`) + +Recursion depth is decided **once per composition branch**, not once for the whole diagram. At +depth 0 — the only point where a feature's own qualified name is checked against the resolved +scope at all (see _Cross-boundary resolution_ / `CollectParts` below) — `CollectParts` (and, +identically, each independently-scoped feature visited by `CollectTopLevelScopedParts`) computes +`ancestorUnlimitedRecursion ?? DetermineBranchUnlimitedRecursion(feature, scope)`: when an ancestor +branch decision has already been made (always `null` only at the true root), it is reused unchanged +(a branch decision, once made, is final for every descendant); otherwise a fresh decision is made +for this depth-0 feature. `DetermineBranchUnlimitedRecursion` returns `true` unconditionally when +`scope` is `null` (no `expose` statement resolved for this view, including the synthesized `--auto` +view — unchanged from before this fix); otherwise, when the feature carries a qualified name, it +delegates to `ExposeScopeResolver.MatchesUnlimitedSubject(qualifiedName, scope)` — a narrower +sibling of `IsInSubjectScope` that returns `true` only when the feature matches a subject whose +recursion kind is `ExposeRecursionKind.MembershipRecursive` or `ExposeRecursionKind.NamespaceRecursive` +(an `expose X::**;` or `expose X::*::**;` form), never via `ExplicitMembers` (which is always +exact-only, regardless of any subject's recursion kind) and never merely because some _other_, +unrelated subject in the same scope happens to be recursive. When a feature matches both a +recursive and a non-recursive subject simultaneously (e.g. two overlapping `expose` statements both +naming an ancestor of the same feature with different recursion kinds), the branch is still decided +`true` — most-permissive-wins, mirroring `IsInSubjectScope`'s own inclusive-OR semantics across +subjects. As a rare fallback — a depth-0 feature with no qualified name at all — the decision falls +back to the diagram-wide `HasUnlimitedRecursion(scope)` predicate (see below), which cannot +distinguish branches but is the best available signal when no qualified name exists to match +against a specific subject. + +Once decided at depth 0 (or independently, per top-level feature, in `CollectTopLevelScopedParts`), +the resulting `bool` is threaded **unchanged** through every descendant call — `BuildPartItem`'s own +recursive `LayOutInterior` call passes its own already-decided `unlimitedRecursion` value straight +through as the child's `ancestorUnlimitedRecursion` — exactly as the pre-fix diagram-wide boolean +was threaded, except the value is now decided **per branch** rather than **once per diagram**. +Re-deriving the decision by qualified-name matching at any depth past 0 would be fundamentally +unsound: composition-path depth (how many `part` containment levels deep a box is nested inside its +rendered parent) and namespace/file-declaration depth (`::`-qualified-name nesting, what +`IsInSubjectScope`/`MatchesUnlimitedSubject` actually match against) are independent axes in +SysML v2 — a deeply nested composition part can have a namespace-shallow qualified name and vice +versa — so only the depth-0 decision (where the two axes still coincide, by construction of how +`CollectParts` is first invoked) is ever qualified-name-matched; every deeper level inherits it +verbatim. + +When a branch's decision is `false`, `BuildPartItem` still includes every part reached at that +branch's depth 0 as its own node, but never recurses into a container part's own interior past that +point: a deeper container renders as an intrinsic-sized leaf box (its own `«part» name : Type` box, +with no nested children drawn), rather than always expanding fully. When a branch's decision is +`true` (including every `scope is null` case), behavior for that branch is completely unchanged +from before per-branch tracking was introduced — recursion is unconditional at every depth within +that branch. + +**Supersedes the earlier global-boolean simplification.** An earlier iteration of this feature +computed a single diagram-wide `unlimitedRecursion` boolean via `HasUnlimitedRecursion(scope)` once +in `BuildLayout`, then threaded that one value unchanged through every branch of the diagram. That +approach had a known, documented limitation: a scope combining a recursive subject with a +non-recursive subject — e.g. `expose SystemDef::**; expose OtherDef;` in the same view — applied +**unlimited recursion to the entire diagram**, even to parts reached only via the non-recursive +subject, which should have stayed depth-limited to themselves. The per-branch tracking described +above resolves that limitation: each top-level branch now gets its own independently-decided +recursion flag, so `SystemDef`'s branch fully recurses while `OtherDef`'s branch stays depth-limited +to its own direct children, in the same diagram. `HasUnlimitedRecursion` itself is retained, but +demoted to the rare depth-0-no-qualified-name fallback described above; it is no longer the primary +recursion gate for any feature that carries a qualified name. ###### No-single-root scoped fallback (`CollectTopLevelScopedParts`, `ResolveTopLevelConnections`) @@ -55,26 +122,70 @@ excludes any matched feature that is itself nested (`"::"`-prefixed) under anoth feature's own qualified name (it is already reachable as that ancestor's own nested part, so must not also be duplicated as a separate top-level node), and builds a `PartItem` for each survivor via `BuildPartItem` — the same container-vs-leaf recursion `CollectParts` uses for every other nested -part, extracted so the logic is never duplicated. When at least one top-level part is found, -`BuildPartIndex` and `ResolveTopLevelConnections` (a `ResolveConnections` analogue that scans every -definition's own connections, since a top-level connection may be declared inside any definition in -the workspace, keeping only a connection whose own qualified name is itself in scope and whose -endpoints both resolve into the top-level part set) resolve any connections between the top-level -parts, and `LayOutInteriorWithConnections` is called directly with `boxDepth: 0` and -`reserveTitleArea: false` — producing boxless top-level nodes placed side by side by the same -`LayeredPlacement.PlaceWithPorts` containment-packing algorithm used everywhere else, with no -enclosing frame/title reserved. The resulting `LayoutTree.Nodes` therefore holds the placed part -boxes (and any ports/lines) directly as top-level siblings, instead of the usual single root -container box. When `CollectTopLevelScopedParts` returns no parts (no scope, or a scope matching no -`part` feature), the original minimal 200×100 empty canvas is preserved unchanged. - -###### `BuildPartItem(feature, theme, depth, defsByName, visited)` +part, extracted so the logic is never duplicated, gated by that same feature's own independently +decided `DetermineBranchUnlimitedRecursion(feature, scope)` (see _Per-Branch Depth-Limited +Recursion_ above) so this boxless fallback path applies the identical per-branch depth-limiting +decision as the normal container-rooted path — each top-level feature here is its own branch root, +exactly as a depth-0 feature is in `CollectParts`. Each survivor is returned as a `TopLevelPart(Part, +OwnerQualifiedName)` — pairing the built `PartItem` with its owning definition's qualified name +(everything before the feature's own qualified name's final `"::"` segment) — rather than a bare +`PartItem`, so `ResolveTopLevelConnections` (below) can later scope endpoint resolution correctly +per owning definition. When at least one top-level part is found, `ResolveTopLevelConnections` (a +`ResolveConnections` analogue that scans every definition's own connections, since a top-level +connection may be declared inside any definition in the workspace, keeping only a connection whose +own qualified name is itself in scope and whose endpoints both resolve into that connection's own +owning definition's top-level parts) resolves any connections between the top-level parts, and +`LayOutInteriorWithConnections` is called directly with `boxDepth: 0` and `reserveTitleArea: false` +— producing boxless top-level nodes placed side by side by the same `LayeredPlacement.PlaceWithPorts` +containment-packing algorithm used everywhere else, with no enclosing frame/title reserved. The +resulting `LayoutTree.Nodes` therefore holds the placed part boxes (and any ports/lines) directly as +top-level siblings, instead of the usual single root container box. When `CollectTopLevelScopedParts` +returns no parts (no scope, or a scope matching no `part` feature), the original minimal 200×100 +empty canvas is preserved unchanged. + +**Owner-scoped connection resolution (`ResolveTopLevelConnections`, `BuildPartIndex` vs. a flat +cross-workspace index).** `BuildPartIndex(parts)` — used by the single-root `ResolveConnections` +path — indexes by simple name only, which is safe there because one containing `part def` can never +have two direct children sharing a simple name. That assumption does **not** hold for the top-level +fallback: `CollectTopLevelScopedParts` can pull matching top-level features from many different +containing definitions across the entire workspace scan, and two different definitions can each +happen to declare a same-named part (e.g. both `part logger : Logger;`). A single flat +`Dictionary` built from every collected top-level part would let `TryAdd` silently keep +only the first same-named occurrence, so a connection declared inside the _other_ definition would +incorrectly resolve against the first definition's part index — producing a bogus edge (or a +silently dropped connection, if the mis-resolved indices happened to coincide). `ResolveTopLevelConnections` +avoids this by grouping the received `IReadOnlyList` into a `Dictionary>` keyed first by each part's own `OwnerQualifiedName` (from +`CollectTopLevelScopedParts`, not by re-deriving ownership from simple-name matching), then, for +each definition in `defsByName.Values.Distinct()`, resolving that definition's own +`SysmlConnectionNode` children's endpoints against **only** the name→index sub-map for that +definition's own qualified name — never the flat union across all definitions. A definition that +owns no collected top-level part (its own qualified name has no entry in the owner grouping) is +skipped entirely, since none of its connections could possibly resolve. This keeps every endpoint +resolution scoped to "this connection's own containing definition's direct children", matching the +same meaning `ResolveConnections`'s per-root `BuildPartIndex` already has for the single-root path, +without needing to change that unrelated, unaffected code path at all. + +###### `BuildPartItem(feature, theme, depth, defsByName, visited, unlimitedRecursion)` Extracted from `CollectParts`'s per-feature loop body: resolves the feature's `FeatureTyping` against `defsByName` via `TryResolveContainer`, recursing into `LayOutInterior` at `depth + 1` for a -container part (with the resolved child's qualified name added to a copy of `visited`, and -`scope: null` — nested composition structure is never re-scoped, matching `CollectParts`'s own -documented recursion behavior) or computing an intrinsic leaf size via `ComputePartSize` otherwise. +container part **only when `unlimitedRecursion` is `true`** (with the resolved child's qualified +name added to a copy of `visited`, `scope: null` — nested composition structure's own membership is +never re-derived from its own qualified name, matching `CollectParts`'s own documented recursion +behavior — and `ancestorUnlimitedRecursion: unlimitedRecursion`, propagating this branch's +already-decided flag unchanged to every descendant; see _Per-Branch Depth-Limited Recursion_ above) +or computing an intrinsic leaf size via `ComputePartSize` otherwise — which happens both +when the feature's type does not resolve to a container, and, unconditionally, whenever +`unlimitedRecursion` is `false` (a container part still renders as its own node; only its interior +expansion is suppressed). Here, `unlimitedRecursion` is always the concrete, already-decided +per-branch `bool` computed once at that branch's own depth 0 (never the ambient `bool?` — by the +time `CollectParts`/`CollectTopLevelScopedParts` call this method, the branch decision has always +already been resolved to a concrete value). This method is only ever invoked at `depth == 0` +directly (from `CollectParts` or `CollectTopLevelScopedParts`) or via its own +`unlimitedRecursion`-gated recursive `LayOutInterior` call, so gating solely on `unlimitedRecursion` +(without separately re-checking `depth`) is sufficient to limit expansion to "depth 0 only" under a +non-recursive branch decision. Both `CollectParts` (depth > 0 or an already-scope-filtered depth-0 feature) and `CollectTopLevelScopedParts` (a scope-matched top-level feature, always at `depth: 0`) call this one method, so the container-vs-leaf recursion is defined exactly once. @@ -105,10 +216,13 @@ node by its parent, which is laid out with the **same** flat placement. (`TryResolveContainer`); a part whose type resolves to a container, and whose type is not already on the recursion path, is a container, and every other part is a leaf. - **Recursion.** A container part is laid out by calling `LayOutInterior` on its type definition at - `depth + 1`, with the type's qualified name added to a `visited` set. The returned interior size - becomes the part's atomic box size, and the returned interior content becomes its - `InnerContent`. A `visited` qualified-name set guards against self- or mutually-referential types - (cycle parts are treated as leaves), guaranteeing termination. + `depth + 1`, with the type's qualified name added to a `visited` set — but only when this + branch's own decided `unlimitedRecursion` value is `true` (see _Per-Branch Depth-Limited + Recursion_ above); when `false`, expansion never proceeds past that branch's own depth 0 and a + deeper container is instead sized as an intrinsic leaf. The + returned interior size becomes the part's atomic box size, and the returned interior content + becomes its `InnerContent`. A `visited` qualified-name set guards against self- or + mutually-referential types (cycle parts are treated as leaves), guaranteeing termination. - **Sizing.** Each level reserves the same title area and insets used by the root: `offsetX = LabelPadding × 2`, `offsetY = TitleAreaHeight(hasLabel, hasKeyword) + LabelPadding × 2`, `containerWidth = TotalWidth + offsetX × 2`, and @@ -190,15 +304,15 @@ relevant), the most specific (deepest/longest qualified name) relevant candidate `ExposeScopeResolver.IsMoreSpecificCandidate`, with the connections/parts tie-break used only to break ties among equally specific candidates; this ordering does not apply when `scope` is `null`. -###### `CollectParts(root, theme)` and `ResolveConnections(root, partIndex)` +###### `CollectParts` and `ResolveConnections(root, partIndex)` `CollectParts` gathers the root's nested `part` usages, sizing each box from its `name : Type` -label, additionally excluding — when a scope is resolved — any part feature whose qualified name -fails `ExposeScopeResolver.IsInSubjectScope`. `ResolveConnections` maps each binary connection's -dotted endpoint references to nested-part indices and port-name labels via `ResolveEndpoint` -(matching the first dotted segment against the (possibly narrowed) part names and capturing any -remaining segment as the port label — see _Cross-boundary resolution_ above), keeping only -distinct, resolvable pairs; a connection whose endpoint was excluded by scoping simply fails to +label, additionally excluding — when a scope is resolved and `depth == 0` — any part feature whose +qualified name fails `ExposeScopeResolver.IsInSubjectScope`. `ResolveConnections` maps each binary +connection's dotted endpoint references to nested-part indices and port-name labels via +`ResolveEndpoint` (matching the first dotted segment against the (possibly narrowed) part names and +capturing any remaining segment as the port label — see _Cross-boundary resolution_ above), keeping +only distinct, resolvable pairs; a connection whose endpoint was excluded by scoping simply fails to resolve and is dropped by this existing endpoint-lookup logic — no separate edge-side scoping is needed. @@ -206,8 +320,11 @@ needed. Because this strategy renders exactly one selected root's interior, scoping cannot narrow a workspace-wide collection the way `GridViewLayoutStrategy` and `BrowserViewLayoutStrategy` do; -instead it restricts **which root is selected** and then narrows **which of that root's parts are -shown**. `FindRoot` only considers candidates `ExposeScopeResolver.IsRootRelevantToScope` accepts, +instead it restricts **which root is selected**, then narrows **which of that root's parts are +shown**, and finally — via `DetermineBranchUnlimitedRecursion`/`MatchesUnlimitedSubject` (see +_Per-Branch Depth-Limited Recursion_ above) — governs **how deep into each shown part's own +interior the diagram recurses, decided independently per branch**. +`FindRoot` only considers candidates `ExposeScopeResolver.IsRootRelevantToScope` accepts, so exposing the current heuristic root itself, an inner part of it, or a definition that itself contains the heuristic default all correctly select a root, while exposing an unrelated definition yields no root and thus the minimal empty canvas. When more than one candidate is @@ -220,7 +337,19 @@ the selected root's own part features to those within the resolved scope (via whose endpoint did not resolve" behavior transparently drops any connection touching an excluded part — no new edge-side logic was required. A view with no `expose` statement (including the synthesized `--auto` view, whose `ViewNode` is `null`) resolves no scope, so `FindRoot` considers -every candidate and `CollectParts` keeps every part, unchanged from the pre-scoping behavior. +every candidate, `CollectParts` keeps every part, and every branch's decided recursion flag is +always `true` — completely unchanged from the pre-scoping behavior. + +Once a root is selected and its direct parts are narrowed, each depth-0 feature's own independently +decided recursion flag determines whether the diagram continues to recurse into that specific +feature's own composed interior at all: when the feature matches a subject with unlimited-depth +recursion (`expose X::**;` / `expose X::*::**;`), or there is no scope at all, that feature's +branch fully expands its own nested parts, unbounded in depth — identical to today's behavior +before this depth-limiting was introduced. When the feature matches only non-recursive subjects +(`expose X;` and/or `expose X::*;` forms), that specific branch's expansion stops after its own +direct part children: a deeper container part within that branch still renders as its own box, but +with no nested children of its own drawn inside it — while a sibling branch matched by a different, +recursive subject in the very same scope fully recurses, independently of this one. ###### Placement and routing @@ -256,8 +385,12 @@ diagnostics, so the returned `LayoutTree` carries no layout-quality warnings. `DemaConsulting.Rendering.Layout`. - `StdlibFilter` (Rendering Internal subsystem) — standard-library exclusion. - `ExposeScopeResolver` (Layout Internal subsystem) — `ResolveExposedScope`, - `IsRootRelevantToScope`, and `IsInSubjectScope` supply the shared `expose`-scoping used by - `BuildLayout`, `FindRoot`, and `CollectParts`. + `IsRootRelevantToScope`, `IsInSubjectScope`, and `IsMoreSpecificCandidate` supply the shared + `expose`-scoping used by `BuildLayout`, `FindRoot`, and `CollectParts`; `MatchesUnlimitedSubject` + (via `DetermineBranchUnlimitedRecursion`) is the primary per-branch depth-limiting gate, deciding + once per depth-0/top-level branch whether that branch's specific matching subject is recursive, + with `HasUnlimitedRecursion` retained only as the depth-0 fallback when no qualified name is + available to match against a subject. - `SysmlWorkspace`, `SysmlDefinitionNode`, `SysmlFeatureNode`, `SysmlConnectionNode` (Semantic subsystem) — model input. - The `LayoutTree`, `LayoutBox`, `LayoutPort`, and `LayoutLine` data types (`DemaConsulting.Rendering`). diff --git a/docs/reqstream/sysml2-tools-core/layout/internal/expose-scope-resolver.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/expose-scope-resolver.yaml index 5c8e4c9c..961c8fd8 100644 --- a/docs/reqstream/sysml2-tools-core/layout/internal/expose-scope-resolver.yaml +++ b/docs/reqstream/sysml2-tools-core/layout/internal/expose-scope-resolver.yaml @@ -185,3 +185,29 @@ sections: usages, not just definitions, and a definitions-only candidate set could never match one. tests: - ResolveExposedScope_BracketFilterMetaclassKind_MatchesUsageLevelCandidate + + - id: SysML2Tools-Core-Layout-Internal-ExposeScopeResolver-UnlimitedSubjectMatch + title: >- + ExposeScopeResolver.MatchesUnlimitedSubject shall return true for a qualified name that + matches a scope subject (by IsInSubjectScope's own equals-or-subtree-prefix or + direct-child rule per that subject's recursion kind) whose own ExposeRecursionKind is + MembershipRecursive or NamespaceRecursive, and false for a match against any other + recursion kind, a match found only via ExplicitMembers, or an unrelated qualified name. + justification: | + InterconnectionViewLayoutStrategy's per-branch depth-limited recursion (see that unit's + ExposeScopingPerBranchRecursionIndependence requirement) needs to know, for a specific + matched feature, not just whether it is in scope at all (IsInSubjectScope's combined + answer across every subject) but specifically whether the subject that matched it is + itself recursive — information IsInSubjectScope does not expose, since it deliberately + answers only a single combined in/out-of-scope boolean. Without this narrower predicate, + a per-branch recursion decision could only be approximated by re-deriving from + IsInSubjectScope, which cannot distinguish "matched a recursive subject" from "matched a + non-recursive subject", the exact distinction per-branch recursion depends on. + tests: + - MatchesUnlimitedSubject_MembershipRecursiveSelf_ReturnsTrue + - MatchesUnlimitedSubject_MembershipRecursiveSubtree_ReturnsTrue + - MatchesUnlimitedSubject_NamespaceRecursiveSubtree_ReturnsTrue + - MatchesUnlimitedSubject_MembershipExact_ReturnsFalse + - MatchesUnlimitedSubject_NamespaceDirectChildren_ReturnsFalse + - MatchesUnlimitedSubject_ExplicitMembersOnly_ReturnsFalse + - MatchesUnlimitedSubject_UnrelatedName_ReturnsFalse diff --git a/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml index 539f09ab..b14d6b0c 100644 --- a/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml +++ b/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml @@ -165,6 +165,58 @@ sections: - InterconnectionView_BuildLayout_ExposeSinglePart_NarrowsToThatPart - InterconnectionView_BuildLayout_ExposeMultipleParts_UnionsBothSubtrees + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ExposeScopingRecursionDepthLimit + title: >- + For a resolved expose scope subject with no unlimited-depth recursion (MembershipExact + or NamespaceDirectChildren), InterconnectionViewLayoutStrategy shall limit interior + expansion of the branch matched by that subject to its own direct part children, + rendering any deeper container part in that branch as an intrinsic-sized leaf box + instead of recursing into its own interior. + justification: | + A non-recursive `expose` form (`expose X;` / `expose X::*;`) asks for only the named + scope's own direct children — not its entire composition subtree. Before this + depth-limiting existed, the resolved scope's filter was applied only at depth 0 and every + included part's interior was then expanded unconditionally regardless of whether the + `expose` statement itself was recursive, silently over-rendering one extra level of nested + parts the user never asked for. This decision is made independently per composition + branch (see the design documentation's "Per-Branch Depth-Limited Recursion" section, and + the ExposeScopingPerBranchRecursionIndependence requirement below for the mixed-subject + case): a feature matching only non-recursive subjects has its own branch depth-limited, + regardless of whether some *other* subject in the same scope is recursive. A branch whose + feature matches at least one recursive subject, or the whole diagram when there is no + scope at all (including the `--auto` view), continues to recurse fully at every depth, + unchanged. + tests: + - InterconnectionView_BuildLayout_ExposeNonRecursiveSubjects_DoesNotRecurseIntoNestedPartsInterior + - InterconnectionView_BuildLayout_ExposeRootOnly_NestedSubsystemInDifferentNamespace_RendersItsOwnUnits + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ContainerFeature_RendersAsLeaf + - InterconnectionView_BuildLayout_NullViewNode_PicksHeuristicRootUnchanged + + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-PerBranchRecursionIndependence + title: >- + When a resolved expose scope combines a subject with unlimited-depth recursion + (MembershipRecursive or NamespaceRecursive) and a subject with no recursion + (MembershipExact or NamespaceDirectChildren), InterconnectionViewLayoutStrategy shall + decide interior-expansion depth independently for each top-level branch — fully + recursing the branch matched by the recursive subject while depth-limiting the branch + matched only by the non-recursive subject — rather than applying one diagram-wide + decision to every branch. + justification: | + An earlier implementation of the ExposeScopingRecursionDepthLimit requirement computed a + single diagram-wide `unlimitedRecursion` boolean from the resolved scope's `Subjects` as a + whole, so a scope such as `expose SystemDef::**; expose OtherDef;` applied unlimited + recursion to the *entire* diagram, including parts reached only via the non-recursive + `OtherDef` subject — over-collapsing a branch the user explicitly asked to keep + depth-limited. This requirement replaces that diagram-wide simplification with a + per-branch decision (see `ExposeScopeResolver.MatchesUnlimitedSubject` and the design + documentation's "Per-Branch Depth-Limited Recursion" section): each top-level feature's + own matched subject (not the scope's `Subjects` collection as a whole) determines that + feature's own branch recursion depth, and that decision is threaded unchanged to every + descendant of the branch, never re-derived by qualified-name matching past depth 0 (since + composition-path depth and namespace-declaration depth are independent axes in SysML v2). + tests: + - InterconnectionView_BuildLayout_ExposeMixedRecursion_OnlyRecursiveBranchRecurses + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-NoExposeFallback title: >- When a view's ViewContext carries no resolved Expose edge — including a null ViewNode @@ -257,8 +309,8 @@ sections: When no candidate root definition is scope-relevant but the resolved expose scope directly includes one or more top-level part feature usages, InterconnectionViewLayoutStrategy shall render those features as boxless nodes side by side instead of an empty canvas, - recursing into each feature's own interior exactly as a normal nested container part - would. + applying the same depth-limited interior expansion (see the + ExposeScopingRecursionDepthLimit requirement) as the normal container-rooted path. justification: | Per SysML v2 spec §8.3.26.11 and §9.2.20.2.6, an InterconnectionView's exposed content need not be a single definition — a scope such as `expose PublishingSubsystem::*;` where @@ -269,11 +321,14 @@ sections: side-by-side placement (reusing the same `LayeredPlacement.PlaceWithPorts` containment packing every other case already uses) and reusing the same container-vs-leaf recursion `CollectParts` uses for every other nested part (via the shared `BuildPartItem` helper) - avoids duplicating any layout or recursion logic for this fallback. + avoids duplicating any layout or recursion logic for this fallback — including whether + that recursion is depth-limited: a top-level feature exposed by a purely non-recursive + scope renders as a leaf with no nested interior drawn, exactly like a depth-limited nested + container part would. tests: - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoRootDef_RendersTopLevelFeatureWithoutFrame - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTopLevelParts_ArrangesSideBySideNoFrame - - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior + - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ContainerFeature_RendersAsLeaf - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NestedTopLevelFeatureExcluded_NotDuplicated - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ScopedTopLevelFeatureFallbackConnections @@ -304,3 +359,26 @@ sections: tests: - InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoMatchingFeature_ReturnsMinimalCanvas - InterconnectionView_BuildLayout_EmptyWorkspace_ReturnsMinimalCanvas + + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-TopLevelOwnerScopedConnections + title: >- + When the no-single-root scoped fallback collects top-level parts from two or more + different containing definitions that happen to share a simple part name, + InterconnectionViewLayoutStrategy shall resolve each definition's own connections only + against that definition's own top-level parts, never against a same-named top-level part + collected from a different containing definition. + justification: | + A connection's endpoints are only meaningful relative to its own containing definition's + direct children. Before this fix, `ResolveTopLevelConnections` built one flat + `Dictionary` simple-name index from every top-level part collected across + the entire workspace scan; when two different containing definitions each declared a + same-named part (e.g. both `part logger : Logger;`), `TryAdd` silently kept only the + first occurrence, so a connection declared inside the *other* definition resolved against + the wrong definition's part — producing a bogus edge, or silently dropping the intended + one. Grouping collected top-level parts by their own `OwnerQualifiedName` (tracked by + `CollectTopLevelScopedParts` from each matched feature's own containing definition, not + re-derived by simple-name matching) and resolving each definition's own connections only + against its own owner-restricted index eliminates this cross-owner name collision, without + changing the unrelated single-root `BuildPartIndex`/`ResolveConnections` path at all. + tests: + - InterconnectionView_BuildLayout_TopLevelNameCollisionAcrossOwners_ResolvesOwnConnection diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 6f11fb85..68772cb1 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -245,6 +245,28 @@ single `part def` qualifies as "the" root — those feature usages render direct with no enclosing frame around them. A scope that matches neither a root definition nor any top-level `part` feature usage still renders the empty diagram described above. +Also specifically for the Interconnection View: when a nested part's own type is itself a +container (a `part def` with its own nested parts), how deep the diagram recurses into that +part's own interior depends on whether that specific part's own branch is matched by a recursive +`expose` subject — decided **independently for each top-level part**, not once for the whole +diagram. A part matched by at least one recursive form (`expose X::**;` or `expose X::*::**;`), +or any part at all when there is no `expose` statement, has its own branch recurse fully — every +nested container's own interior is shown within that branch, at any depth. A part matched **only** +by non-recursive forms (`expose X;` and/or `expose X::*;`) has its own branch limited to that +part's own direct part children: a deeper nested part still renders as its own box within that +branch, but its own interior is not drawn. Because this decision is per-branch, a single view can +mix both outcomes: `expose SystemA::**; expose SystemB;` fully recurses into `SystemA`'s own +composed structure while stopping `SystemB`'s branch at its own direct children, in the very same +diagram — a sibling part matched by a different subject is never affected by another part's own +recursion kind. For example, `expose System; expose System::*;` shows `System`'s direct part +children as boxes, but does not expand into any of those parts' own nested structure, even if +their types have one — whereas `expose System::**;` shows every level. + +When the no-single-root scoped fallback (above) collects top-level parts from more than one +containing definition, a connection declared inside one of those definitions always resolves +against that definition's own top-level parts only — even if a different exposed definition +happens to declare a same-named part. + Named `view Name { ... }` usages (not just `view def` declarations) are also now recognized as their own renderable declarations: a workspace containing both `view def` declarations and named `view` usages surfaces both kinds as views the `render` command discovers and renders. diff --git a/docs/verification/sysml2-tools-core/layout/internal/expose-scope-resolver.md b/docs/verification/sysml2-tools-core/layout/internal/expose-scope-resolver.md index 29c26590..ae0e16d5 100644 --- a/docs/verification/sysml2-tools-core/layout/internal/expose-scope-resolver.md +++ b/docs/verification/sysml2-tools-core/layout/internal/expose-scope-resolver.md @@ -3,8 +3,8 @@ ##### Verification Approach `ExposeScopeResolver` is verified through direct unit tests in `ExposeScopeResolverTests` that -call `ResolveExposedScope`, `IsInSubjectScope`, `IsRootRelevantToScope`, and -`IsMoreSpecificCandidate` directly with synthetic `SysmlWorkspace`/`SysmlViewNode` inputs and +call `ResolveExposedScope`, `IsInSubjectScope`, `MatchesUnlimitedSubject`, `IsRootRelevantToScope`, +and `IsMoreSpecificCandidate` directly with synthetic `SysmlWorkspace`/`SysmlViewNode` inputs and assert on the returned `ExposedScope` (Phase 2a: `Subjects`/`ExplicitMembers`/`Failures`, replacing the earlier flat qualified-name-list shape) or boolean result. No mocking is required; every method is a pure function over its parameters. @@ -62,6 +62,15 @@ configuration are required beyond a standard .NET SDK installation. - (Phase 2a) `IsInSubjectScope` returns `true` for an exact qualified-name match against an `ExplicitMembers` entry, and `false` for one of that entry's own descendants — an `ExplicitMembers` match is exact-only, not a subtree match. +- `MatchesUnlimitedSubject` returns `true` for a qualified name matching a `Subjects` entry (exact + or subtree) whose own recursion kind is `MembershipRecursive`, and likewise for a subtree match + against a `NamespaceRecursive` entry. +- `MatchesUnlimitedSubject` returns `false` for a qualified name matching a `Subjects` entry whose + own recursion kind is `MembershipExact` or `NamespaceDirectChildren` — matching a non-recursive + subject never signals unlimited recursion, even though the qualified name is otherwise in scope. +- `MatchesUnlimitedSubject` returns `false` for a qualified name that matches only an + `ExplicitMembers` entry (never via explicit members, regardless of any subject's recursion kind + elsewhere in the same scope) and for an unrelated qualified name. - `IsRootRelevantToScope` returns `true` when the candidate equals a `Subjects` entry. - `IsRootRelevantToScope` returns `true` when the candidate is nested within a `Subjects` entry. @@ -141,6 +150,21 @@ configuration are required beyond a standard .NET SDK installation. Exact qualified-name match against an `ExplicitMembers` entry is in scope - `IsInSubjectScope_ExplicitMemberDescendant_ReturnsFalse`: A descendant of an `ExplicitMembers` entry is not automatically in scope (exact match only) +- `MatchesUnlimitedSubject_MembershipRecursiveSelf_ReturnsTrue`: + Exact match against a `MembershipRecursive` subject matches the unlimited-recursion predicate +- `MatchesUnlimitedSubject_MembershipRecursiveSubtree_ReturnsTrue`: + Subtree match against a `MembershipRecursive` subject matches the unlimited-recursion predicate +- `MatchesUnlimitedSubject_NamespaceRecursiveSubtree_ReturnsTrue`: + Subtree match against a `NamespaceRecursive` subject matches the unlimited-recursion predicate +- `MatchesUnlimitedSubject_MembershipExact_ReturnsFalse`: + Match against a `MembershipExact` subject does not match the unlimited-recursion predicate +- `MatchesUnlimitedSubject_NamespaceDirectChildren_ReturnsFalse`: + Match against a `NamespaceDirectChildren` subject does not match the unlimited-recursion + predicate +- `MatchesUnlimitedSubject_ExplicitMembersOnly_ReturnsFalse`: + A match found only via `ExplicitMembers` never matches the unlimited-recursion predicate +- `MatchesUnlimitedSubject_UnrelatedName_ReturnsFalse`: + An unrelated qualified name does not match the unlimited-recursion predicate - `IsRootRelevantToScope_CandidateEqualsSubject_ReturnsTrue`: Candidate equal to a `Subjects` entry is relevant - `IsRootRelevantToScope_CandidateNestedInSubject_ReturnsTrue`: diff --git a/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md b/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index fa7583ff..5e3d8690 100644 --- a/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md +++ b/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md @@ -32,13 +32,42 @@ like the reported bug (a namespace-like `part def` whose only nested content is feature usage, exposed via `expose Namespace::*;`) asserts the previously-empty canvas now renders that feature directly, with no frame for the enclosing namespace anywhere in the tree; a two-sibling variant asserts two independent top-level parts render as two non-overlapping top-level boxes; a -container-typed top-level feature variant asserts the recursion into its own nested parts still -fires (proving `BuildPartItem` is shared, not duplicated); a connection-between-top-level-parts -variant asserts a connector is drawn when the connection's own qualified name is itself in scope; -and a nested-qualified-name variant asserts a matched feature nested under another matched -feature's own qualified name is excluded from the top-level set (not duplicated). A final -regression test pins down the preserved empty-canvas fallback when the scope matches no `part` -feature at all. +container-typed top-level feature variant asserts the same depth-limiting gate as the normal +container-rooted path applies here too (the top-level feature renders as a leaf box with no nested +children, since the scope exercised is purely non-recursive), proving `BuildPartItem` is shared, not +duplicated; a connection-between-top-level-parts variant asserts a connector is drawn when the +connection's own qualified name is itself in scope; and a nested-qualified-name variant asserts a +matched feature nested under another matched feature's own qualified name is excluded from the +top-level set (not duplicated). A final regression test pins down the preserved empty-canvas +fallback when the scope matches no `part` feature at all. + +A further pair of tests exercises the depth-limiting recursion gate directly on a two-tier +cross-namespace composition (a root `part def` composing a subsystem `part def` which itself +composes two nested units): exposing the root with a purely non-recursive scope (one +`MembershipExact` subject and one `NamespaceDirectChildren` subject, no recursive subject) confirms +the subsystem still renders as its own box but with no nested unit boxes drawn inside it — the +fix-targeting scenario; exposing the same shape with a `MembershipRecursive` subject confirms full +recursion is unchanged, a regression guard proving the depth-limiting gate never engages when the +resolved scope has an unlimited-depth subject. The pre-existing null-`ViewNode` test additionally +pins down that `HasUnlimitedRecursion` evaluates to `true` whenever no scope is resolved at all. + +A dedicated per-branch test exercises a scope combining a recursive subject with a non-recursive +sibling subject in the same view (two sibling `part` features under one root, `a` exposed via a +`MembershipRecursive` subject and `b` exposed via a `MembershipExact` subject, each typed by its own +two-nested-part container definition): it asserts `a`'s branch fully recurses (both of its nested +parts appear as boxes) while `b`'s branch, in the very same diagram, renders as a leaf with no +nested boxes — proving the recursion decision is made independently per top-level branch rather +than once for the whole diagram, replacing the earlier global-boolean simplification's behavior for +this exact shape. + +A dedicated fallback test exercises the owner-scoped top-level connection resolution fix: two +different `part def`s (each exposed as independent top-level scoped features via their own +`NamespaceDirectChildren` subject, so `FindRoot` selects no root and the boxless fallback fires) +each declare a same-named `part logger : ...;` plus one other own part, and each declares its own +connection between its own `logger` and its own other part; the test asserts the connector +belonging to the first definition's own connection is geometrically anchored to that definition's +own `logger`/other boxes and never to the second definition's same-named `logger` box — the +pre-fix collision this owner-scoped grouping resolves. ##### Test Environment @@ -104,14 +133,31 @@ configuration are required beyond a standard .NET SDK installation. one box, with no frame labeled for the enclosing namespace anywhere in the tree. - Two independent top-level scoped parts render as two non-overlapping top-level boxes with no wrapping frame. -- A top-level scoped feature that is itself typed by a container definition still recurses into its - own nested parts, which appear as that top-level box's own `Children`. +- A top-level scoped feature that is itself typed by a container definition, exposed by a purely + non-recursive scope, renders as an intrinsic-sized leaf box — its own nested parts are not drawn + — applying the same depth-limiting gate as the normal container-rooted path. - A connection between two top-level scoped parts is drawn as a connector line when the connection's own qualified name is itself within the resolved scope. - A matched top-level feature whose qualified name is nested under another matched feature's own qualified name is excluded from the top-level set (not duplicated as its own separate node). - When the resolved `expose` scope matches no `part` feature usage at all, the pre-existing minimal empty canvas is returned unchanged. +- When the resolved `expose` scope has no subject with unlimited-depth recursion (only + `MembershipExact` and/or `NamespaceDirectChildren` subjects), a nested container part beyond the + root's own direct children renders as an intrinsic-sized leaf box with no nested children of its + own, even though its type has its own composed interior. +- When the resolved `expose` scope has at least one subject with unlimited-depth recursion + (`MembershipRecursive` or `NamespaceRecursive`), or there is no scope at all, interior recursion + remains fully unbounded at every depth, unchanged from before this depth-limiting was introduced. +- When a resolved `expose` scope combines a recursive subject with a non-recursive sibling subject + in the same view, the branch matched by the recursive subject fully recurses while the branch + matched only by the non-recursive subject stays depth-limited to its own direct children, in the + same diagram — the recursion decision is made independently per top-level branch, not once for + the whole diagram. +- When the no-single-root scoped fallback collects top-level parts from two different containing + definitions that happen to declare a same-named part, each definition's own connection resolves + against that definition's own top-level parts only — never against a same-named top-level part + collected from the other containing definition. ##### Test Scenarios @@ -143,7 +189,11 @@ configuration are required beyond a standard .NET SDK installation. | `InterconnectionView_BuildLayout_ExposeBothSameDepthSiblings_ScoreBreaksTieNotLength` | Score breaks the tie | | `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoRootDef_RendersTopLevelFeatureWithoutFrame` | Alone | | `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTopLevelParts_ArrangesSideBySideNoFrame` | 2 boxes | -| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior` | Nested | +| `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ContainerFeature_RendersAsLeaf` | Leaf | | `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NoMatchingFeature_ReturnsMinimalCanvas` | No match | | `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ConnectionBetweenTopLevelFeatures_DrawsEdge` | Linked | | `InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_NestedTopLevelFeatureExcluded_NotDuplicated` | Dedup | +| `InterconnectionView_BuildLayout_ExposeNonRecursiveSubjects_DoesNotRecurseIntoNestedPartsInterior` | Depth limit | +| `InterconnectionView_BuildLayout_ExposeRootOnly_NestedSubsystemInDifferentNamespace_RendersItsOwnUnits` | Unchanged | +| `InterconnectionView_BuildLayout_ExposeMixedRecursion_OnlyRecursiveBranchRecurses` | Per-branch | +| `InterconnectionView_BuildLayout_TopLevelNameCollisionAcrossOwners_ResolvesOwnConnection` | Owner-scoped | diff --git a/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj b/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj index e26aa014..8d11a533 100644 --- a/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj +++ b/src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj @@ -56,7 +56,7 @@ - + diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs index c6003f05..af992731 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs @@ -218,6 +218,42 @@ public static bool IsInSubjectScope(string qualifiedName, ExposedScope scope) => }) || scope.ExplicitMembers.Contains(qualifiedName, StringComparer.Ordinal); + /// + /// Returns when matches one of + /// 's whose own + /// kind carries unlimited-depth recursion — + /// or + /// — using that subject's same + /// per-kind containment rule as . This is a distinct, narrower + /// predicate from : the latter decides whether a + /// qualified name is in scope at all (any subject kind, plus bracket-filtered + /// ), while this method decides how far a + /// branch that is already known to be in scope should recurse — a caller + /// () needs to know not just that a feature + /// matched, but specifically which subject matched it and whether that subject's own recursion + /// kind means the matched branch's interior should expand without a depth limit. A + /// match (a bracket-filter result) is never + /// unlimited — bracket filters name exact matched declarations only, never a containment + /// subtree, so they never contribute to this predicate. + /// + /// The candidate qualified name. + /// The resolved expose scope. + /// + /// when matches a subject whose + /// recursion kind is or + /// ; otherwise . + /// + public static bool MatchesUnlimitedSubject(string qualifiedName, ExposedScope scope) => + scope.Subjects.Any(subject => subject.Recursion switch + { + ExposeRecursionKind.MembershipRecursive => + qualifiedName == subject.QualifiedName || + qualifiedName.StartsWith(subject.QualifiedName + "::", StringComparison.Ordinal), + ExposeRecursionKind.NamespaceRecursive => + qualifiedName.StartsWith(subject.QualifiedName + "::", StringComparison.Ordinal), + _ => false, + }); + /// /// Returns when is a direct (one-level) /// child of — starts with "{container}::" and the diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs index 11174d79..3bdc5606 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs @@ -39,7 +39,15 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// . A single-level model (no part typed by a definition with internal /// parts) is a strict no-op: the recursion never fires and the output is identical to the /// non-recursive layout. Recursion is driven here, at the strategy level, because container detection -/// is a semantic-model concern the model-independent algorithm cannot see. +/// is a semantic-model concern the model-independent algorithm cannot see. This recursion is +/// unconditional only for a part's own branch when that branch was matched into the resolved +/// expose scope by a subject with no depth limit — i.e. a subject with unlimited-depth +/// recursion (see ) — or when there is no +/// scope at all. This decision is made per branch, not once for the whole diagram: a scope +/// combining a recursive subject (e.g. expose X::**;) with a non-recursive subject (e.g. +/// expose Y;) recurses fully into X's branch while Y's branch stops expanding +/// after its own direct part children (depth 0), rendering any deeper container in Y's branch +/// as an intrinsic-sized leaf box instead of recursing further. /// /// /// Not every resolved expose scope names a single part def worth treating as "the" @@ -49,7 +57,12 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// part feature usages, those features are rendered as boxless nodes side by side (via /// ) instead of the diagram falling back to an empty canvas /// — each one recursing into its own interior exactly as a normal nested container part would, -/// reusing so the container-vs-leaf logic is never duplicated. +/// reusing so the container-vs-leaf logic is never duplicated. Because +/// these boxless top-level parts may be declared inside different containing definitions, any +/// connection between them is resolved against only its own declaring definition's own children +/// (see 's remarks) — never a single flat cross-workspace +/// name index, which would silently collide when two different definitions each declare a +/// same-simple-named part. /// /// internal sealed class InterconnectionViewLayoutStrategy : ILayoutStrategy @@ -80,6 +93,26 @@ private sealed record PartItem( double Height, IReadOnlyList? InnerContent); + /// + /// One top-level scoped feature collected by , pairing + /// its laid-out with the qualified name of the definition that directly owns + /// it (its containing part def), so can group + /// top-level parts by owner and resolve each definition's own connections against only its own + /// children — never a flat cross-workspace name index, which would silently collide when two + /// different definitions each own a same-simple-named part (see ). + /// + /// The collected part's laid-out box data. + /// + /// The qualified name of the part def that directly declares this feature — the feature's + /// own qualified name with its last "::"-separated segment removed — or + /// when the feature's qualified name has no "::" separator at all + /// (declared at the absolute root namespace with no containing definition; such a feature cannot + /// be the endpoint of any connection resolved by , since + /// connections are only ever children of an actual , matching + /// pre-existing behavior for this shape). + /// + private sealed record TopLevelPart(PartItem Part, string? OwnerQualifiedName); + /// /// A resolved binary connection between two nested-part indices, together with the port-name /// label for each end (the dotted-reference remainder after the resolved part, e.g. @@ -123,11 +156,11 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) // preserved unchanged. if (scope is not null) { - var topLevelParts = CollectTopLevelScopedParts(context.Workspace, scope, theme, defsByName); - if (topLevelParts.Count > 0) + var topLevelEntries = CollectTopLevelScopedParts(context.Workspace, scope, theme, defsByName); + if (topLevelEntries.Count > 0) { - var partIndex = BuildPartIndex(topLevelParts); - var pairs = ResolveTopLevelConnections(defsByName, partIndex, scope); + var topLevelParts = topLevelEntries.Select(e => e.Part).ToList(); + var pairs = ResolveTopLevelConnections(defsByName, topLevelEntries, scope); var layout = LayOutInteriorWithConnections( topLevelParts, pairs, theme, boxDepth: 0, reserveTitleArea: false); return new LayoutTree(layout.Width, layout.Height, layout.Content); @@ -151,7 +184,7 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) visited.Add(root.QualifiedName); } - var interior = LayOutInterior(root, theme, depth: 0, defsByName, visited, scope); + var interior = LayOutInterior(root, theme, depth: 0, defsByName, visited, scope, ancestorUnlimitedRecursion: null); // Container box for the root part definition. The root sits at the same origin (0, 0) // that interior.Content is already positioned relative to, so the interior content is @@ -188,6 +221,14 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) /// /// The view's resolved expose containment-subtree scope, or null when no scoping applies. /// + /// + /// The already-decided per-branch unlimited-recursion flag inherited from the ancestor call that + /// recursed into this definition, or only for the true top-of-recursion + /// call from 's root path ( is always 0 in that + /// case). When non-null, it is propagated unchanged — the decision is made once, per branch, at + /// depth 0 (see ), and every deeper recursive call simply inherits it; + /// see 's remarks for how this actually gates recursion. + /// /// The laid-out interior size and content. private static InteriorLayout LayOutInterior( SysmlDefinitionNode def, @@ -195,9 +236,10 @@ private static InteriorLayout LayOutInterior( int depth, IReadOnlyDictionary defsByName, ISet visited, - ExposedScope? scope) + ExposedScope? scope, + bool? ancestorUnlimitedRecursion) { - var parts = CollectParts(def, theme, depth, defsByName, visited, scope); + var parts = CollectParts(def, theme, depth, defsByName, visited, scope, ancestorUnlimitedRecursion); var partIndex = BuildPartIndex(parts); var pairs = ResolveConnections(def, partIndex); @@ -334,6 +376,38 @@ private static InteriorLayout LayOutInteriorWithConnections( return new InteriorLayout(containerWidth, containerHeight, content); } + /// + /// Returns when interior recursion should be unconditionally unlimited for + /// an entire diagram — either because is (no + /// expose statement resolved for this view / the synthetic --auto view, unchanged + /// pre-scoping behavior), or because at least one of its + /// carries a recursion kind meaning "unlimited depth" + /// ( or + /// ). + /// + /// + /// This diagram-wide check is no longer the primary recursion gate — that decision is now made + /// per branch, at depth 0, by via + /// , which knows specifically which + /// subject matched each feature into scope (so a scope combining a recursive subject, e.g. + /// expose X::**;, with a non-recursive subject, e.g. expose Y;, correctly recurses + /// only X's branch while Y's branch stays depth-limited to itself — superseding the + /// earlier "known limitation" where this method's diagram-wide answer was applied to the whole + /// diagram regardless of which subject actually matched a given part). This method now serves + /// only as the conservative fallback uses for the rare edge case of a + /// depth-0 feature that carries no qualified name at all — a shape with no specific subject to + /// attribute the decision to, so the diagram-wide answer is used instead, matching the + /// pre-per-branch behavior for that one edge case. + /// + /// The view's resolved expose containment-subtree scope, or null. + /// + /// when interior recursion is unlimited for the whole diagram; + /// when the resolved scope has no unlimited-depth subject. + /// + private static bool HasUnlimitedRecursion(ExposedScope? scope) => + scope is null || + scope.Subjects.Any(s => s.Recursion is ExposeRecursionKind.MembershipRecursive or ExposeRecursionKind.NamespaceRecursive); + /// /// Finds the part definition whose interior to render: among scope-relevant candidates, prefers /// the composition-graph root — a candidate that is not itself referenced as another candidate's @@ -449,22 +523,44 @@ private static InteriorLayout LayOutInteriorWithConnections( /// and is 0 (the root definition's own direct part usages), a /// part feature whose own QualifiedName fails /// is skipped — this lets a narrow expose (e.g. one specific subsystem, not the whole - /// system) select which of the root's own branches to draw. Scope is not re-applied to - /// any deeper recursive call ( > 0): per the InterconnectionView - /// definition (SysML v2 spec §9.2.20.2.6), an interconnection diagram presents "exposed features - /// as nodes, nested features as nested nodes" — once a part has been included as a node, its own - /// composed structure must always be shown recursively, regardless of whether that structure - /// happens to be declared in a different namespace than the one named in the view's expose - /// statement (namespace location and composition structure are independent in SysML v2 — a part - /// def's own nested parts are not themselves separate members of the exposed namespace). + /// system) select which of the root's own branches to draw. Scope is never re-applied to any + /// deeper recursive call ( > 0) by re-checking a deeper feature's own + /// qualified name — a part def's own nested parts are not themselves separate members of the + /// exposed namespace (namespace-declaration location and composition/usage depth are + /// independent axes in SysML v2), so re-matching a deeper qualified name against the scope would + /// be incorrect. Instead, whether a container part's interior is expanded at all past depth 0 is + /// decided per branch: at depth 0, each feature's own branch-unlimited-recursion decision is + /// computed once (via , from + /// when already known — i.e. this is itself a + /// recursive call from an ancestor branch — or freshly derived from which specific subject + /// matched this feature otherwise) and then propagated unchanged to every one of that feature's + /// own descendants (see ). A scope combining a recursive subject + /// (e.g. expose X::**;) with a non-recursive subject (e.g. expose Y;) therefore + /// correctly recurses only the branch matched by the recursive subject, while the branch matched + /// only by the non-recursive subject stays depth-limited to itself — superseding the earlier + /// diagram-wide simplification. /// + /// The definition whose direct part children to collect. + /// The active rendering theme. + /// Nesting depth of this call (0 for the root definition's own children). + /// Container-definition index keyed by qualified and simple name. + /// Qualified names already on the recursion path, guarding against cycles. + /// + /// The view's resolved expose containment-subtree scope, or null when no scoping applies. + /// + /// + /// The already-decided per-branch unlimited-recursion flag inherited from an ancestor call, or + /// only at the true top of recursion ( is always 0 + /// in that case). See 's remarks. + /// private static IReadOnlyList CollectParts( SysmlDefinitionNode root, Theme theme, int depth, IReadOnlyDictionary defsByName, ISet visited, - ExposedScope? scope) + ExposedScope? scope, + bool? ancestorUnlimitedRecursion) { var result = new List(); foreach (var feature in root.Children.OfType()) @@ -480,48 +576,92 @@ private static IReadOnlyList CollectParts( continue; } - result.Add(BuildPartItem(feature, theme, depth, defsByName, visited)); + var unlimitedRecursion = ancestorUnlimitedRecursion ?? DetermineBranchUnlimitedRecursion(feature, scope); + result.Add(BuildPartItem(feature, theme, depth, defsByName, visited, unlimitedRecursion)); } return result; } + /// + /// Decides, for one depth-0 feature, whether its own branch's interior recursion is + /// unconditionally unlimited: when is + /// (no expose scoping at all — unchanged unconditional recursion); + /// otherwise, when the feature carries a qualified name, whether it matches a subject with + /// unlimited-depth recursion via — the + /// specific subject that matched this feature into scope, not the scope as a whole. Falls back to + /// the conservative diagram-wide check only for the rare edge + /// case of a feature with no qualified name at all, since there is then no specific subject to + /// attribute the decision to. + /// + /// The depth-0 part feature usage being collected. + /// The view's resolved expose containment-subtree scope, or null. + /// Whether this feature's own branch should recurse without a depth limit. + private static bool DetermineBranchUnlimitedRecursion(SysmlFeatureNode feature, ExposedScope? scope) + { + if (scope is null) + { + return true; + } + + return feature.QualifiedName is { Length: > 0 } fqn + ? ExposeScopeResolver.MatchesUnlimitedSubject(fqn, scope) + : HasUnlimitedRecursion(scope); + } + /// /// Builds a single nested part usage's , recursing into its interior when /// the part's type resolves to a container definition (a non-stdlib part def with its own - /// internal parts, not already on the recursion path) and computing an intrinsic leaf size - /// otherwise. Extracted from so the same container-vs-leaf recursion - /// is reused verbatim by — the boxless top-level - /// fallback path must lay out each top-level feature exactly as a normal nested part would be, - /// per the "no duplicated logic" coding principle. + /// internal parts, not already on the recursion path) and + /// is , and computing an intrinsic leaf size otherwise. Extracted from + /// so the same container-vs-leaf recursion is reused verbatim by + /// — the boxless top-level fallback path must lay out + /// each top-level feature exactly as a normal nested part would be, per the "no duplicated + /// logic" coding principle. /// /// The part feature usage to build a for. /// The active rendering theme. /// Nesting depth of the feature's own container box (0 for a top-level part). /// Container-definition index keyed by qualified and simple name. /// Qualified names already on the recursion path, guarding against cycles. + /// + /// This branch's already-decided unlimited-recursion flag — computed once at depth 0 by + /// (via ) or + /// , from which specific subject matched this branch's + /// own top-of-branch feature into scope, and propagated unchanged to every descendant of that + /// feature (this method's own recursive call below passes it on as + /// ancestorUnlimitedRecursion, never re-deriving it). When , a + /// container part's own interior is never expanded — it always renders as an intrinsic-sized + /// leaf box, regardless of . + /// /// The built , container or leaf. private static PartItem BuildPartItem( SysmlFeatureNode feature, Theme theme, int depth, IReadOnlyDictionary defsByName, - ISet visited) + ISet visited, + bool unlimitedRecursion) { var name = feature.Name ?? feature.FeatureTyping ?? "part"; - if (TryResolveContainer(feature.FeatureTyping, defsByName, visited, out var childDef)) + if (unlimitedRecursion && TryResolveContainer(feature.FeatureTyping, defsByName, visited, out var childDef)) { // Container part: lay out its interior bottom-up and treat it as an atomic node. // Scope is intentionally not carried into this recursive call — see the remarks on - // CollectParts for why nested composition structure is always shown once its owning - // part has been included, regardless of the exposed namespace scope. + // CollectParts for why nested composition structure's own qualified name is never + // re-checked against scope; unlimitedRecursion (already decided for this branch) is + // threaded through unchanged as the ancestor's decision so every descendant of this + // branch shares the same depth-limiting decision. var childVisited = new HashSet(visited, StringComparer.Ordinal) { childDef.QualifiedName! }; - var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope: null); + var inner = LayOutInterior(childDef, theme, depth + 1, defsByName, childVisited, scope: null, ancestorUnlimitedRecursion: unlimitedRecursion); return new PartItem(name, "part", feature.FeatureTyping, inner.Width, inner.Height, inner.Content); } - // Leaf part: intrinsic size, no nested content. + // Leaf part: intrinsic size, no nested content. Reached either because the type does not + // resolve to a container definition, or because unlimitedRecursion is false and interior + // expansion is intentionally stopped here (the part still renders as its own node — its + // own nested parts are simply not drawn). var (width, height) = ComputePartSize(name, feature.FeatureTyping, theme); return new PartItem(name, "part", feature.FeatureTyping, width, height, null); } @@ -540,11 +680,12 @@ private static PartItem BuildPartItem( /// The active rendering theme. /// Container-definition index keyed by qualified and simple name. /// - /// The matched top-level parts, each recursively laid out via exactly - /// as a normal nested container part would be, in 's declaration - /// order. Empty when no non-stdlib part feature satisfies the scope. + /// The matched top-level parts paired with their owning definition's qualified name, each + /// recursively laid out via exactly as a normal nested container part + /// would be, in 's declaration order. Empty when no non-stdlib + /// part feature satisfies the scope. /// - private static IReadOnlyList CollectTopLevelScopedParts( + private static IReadOnlyList CollectTopLevelScopedParts( SysmlWorkspace workspace, ExposedScope scope, Theme theme, @@ -581,16 +722,36 @@ private static IReadOnlyList CollectTopLevelScopedParts( !matchedNames.Any(other => other != fqn && fqn.StartsWith(other + "::", StringComparison.Ordinal))) .ToList(); - var result = new List(); + var result = new List(); foreach (var feature in topLevel) { var visited = new HashSet(StringComparer.Ordinal); - result.Add(BuildPartItem(feature, theme, depth: 0, defsByName, visited)); + var unlimitedRecursion = DetermineBranchUnlimitedRecursion(feature, scope); + var part = BuildPartItem(feature, theme, depth: 0, defsByName, visited, unlimitedRecursion); + result.Add(new TopLevelPart(part, OwnerQualifiedNameOf(feature.QualifiedName))); } return result; } + /// + /// Derives a feature's owning definition's qualified name by stripping the last + /// "::"-separated segment of its own qualified name — see 's + /// remarks for why this string-derivation is sufficient without a dedicated parent pointer. + /// + /// The feature's own qualified name, or null. + /// The owning definition's qualified name, or null when it cannot be derived. + private static string? OwnerQualifiedNameOf(string? qualifiedName) + { + if (qualifiedName is not { Length: > 0 }) + { + return null; + } + + var sep = qualifiedName.LastIndexOf("::", StringComparison.Ordinal); + return sep >= 0 ? qualifiedName[..sep] : null; + } + /// /// Resolves connections between the boxless top-level parts collected by /// , reusing verbatim. @@ -601,18 +762,62 @@ private static IReadOnlyList CollectTopLevelScopedParts( /// part set — an incidental connection between unrelated parts must never be surfaced just /// because it happens to share a name with one of the rendered top-level features. /// + /// + /// A connection's endpoints are only meaningful relative to its own containing definition's + /// direct children — two different containing definitions may each declare a same-simple-named + /// part (e.g. both have part logger : Logger;). A single flat cross-workspace name index + /// built from every collected top-level part (as a naive call would) + /// would silently keep only the first such part under that name, causing bogus or dropped + /// connections for every other definition sharing the name. Instead, this groups + /// by into a + /// per-owner name → index map (byOwner, keyed against the overall + /// list position, matching the part ordering the caller lays out), then resolves each definition's + /// own connections only against its own restricted, unambiguous index — never the flat union. + /// /// Container-definition index keyed by qualified and simple name. - /// Name → index lookup for the collected top-level parts. + /// + /// The top-level parts collected by , each paired with + /// its owning definition's qualified name. + /// /// The view's resolved expose containment-subtree scope. /// The resolved connection pairs between top-level parts, if any. private static IReadOnlyList ResolveTopLevelConnections( IReadOnlyDictionary defsByName, - Dictionary partIndex, + IReadOnlyList topLevelParts, ExposedScope scope) { + // Group the collected top-level parts by their owning definition, building each owner's own + // restricted simple-name → overall-list-index map so same-simple-named parts owned by + // different definitions can never collide. + var byOwner = new Dictionary>(StringComparer.Ordinal); + for (var i = 0; i < topLevelParts.Count; i++) + { + var owner = topLevelParts[i].OwnerQualifiedName; + if (owner is null) + { + continue; + } + + if (!byOwner.TryGetValue(owner, out var ownerIndex)) + { + ownerIndex = new Dictionary(StringComparer.Ordinal); + byOwner[owner] = ownerIndex; + } + + ownerIndex.TryAdd(topLevelParts[i].Part.Name, i); + } + var pairs = new List(); foreach (var def in defsByName.Values.Distinct()) { + if (def.QualifiedName is not { Length: > 0 } ownerName || !byOwner.TryGetValue(ownerName, out var partIndex)) + { + // This definition owns no collected top-level part, so none of its own connections + // could possibly resolve to a top-level part; skip it rather than resolving against + // an unrelated (or the wrong) owner's index. + continue; + } + foreach (var conn in def.Children.OfType()) { if (conn.QualifiedName is not { Length: > 0 } fqn || !ExposeScopeResolver.IsInSubjectScope(fqn, scope)) diff --git a/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj b/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj index 15ca0a84..d6a71483 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj +++ b/src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj @@ -98,8 +98,8 @@ - - + + diff --git a/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj b/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj index 9a628c1a..4b1a14c4 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj +++ b/test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj @@ -56,8 +56,8 @@ - - + + diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs index 27dc5376..5a6473b5 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs @@ -180,6 +180,80 @@ public void IsInSubjectScope_ExplicitMemberDescendant_ReturnsFalse() Assert.False(ExposeScopeResolver.IsInSubjectScope("Root::A::Matched::Nested", new ExposedScope([], ["Root::A::Matched"]))); } + /// + /// A feature exactly matching a subject + /// matches the unlimited-recursion predicate. + /// + [Fact] + public void MatchesUnlimitedSubject_MembershipRecursiveSelf_ReturnsTrue() + { + Assert.True(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.MembershipRecursive)], []))); + } + + /// + /// A feature nested under a subject's + /// containment subtree matches the unlimited-recursion predicate. + /// + [Fact] + public void MatchesUnlimitedSubject_MembershipRecursiveSubtree_ReturnsTrue() + { + Assert.True(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A::Child", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.MembershipRecursive)], []))); + } + + /// + /// A feature nested under a subject's + /// containment subtree matches the unlimited-recursion predicate. + /// + [Fact] + public void MatchesUnlimitedSubject_NamespaceRecursiveSubtree_ReturnsTrue() + { + Assert.True(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A::Child", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.NamespaceRecursive)], []))); + } + + /// + /// A feature matching only a subject does + /// not match the unlimited-recursion predicate (exact/non-recursive expose). + /// + [Fact] + public void MatchesUnlimitedSubject_MembershipExact_ReturnsFalse() + { + Assert.False(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.MembershipExact)], []))); + } + + /// + /// A feature matching only a subject + /// does not match the unlimited-recursion predicate (only direct children, not recursive). + /// + [Fact] + public void MatchesUnlimitedSubject_NamespaceDirectChildren_ReturnsFalse() + { + Assert.False(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A::Child", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.NamespaceDirectChildren)], []))); + } + + /// + /// An explicit (bracket-filter-matched) member is never considered an unlimited-recursion + /// match, even if it happens to equal an explicit-members entry — MatchesUnlimitedSubject + /// only ever considers , never . + /// + [Fact] + public void MatchesUnlimitedSubject_ExplicitMembersOnly_ReturnsFalse() + { + Assert.False(ExposeScopeResolver.MatchesUnlimitedSubject("Root::A::Matched", new ExposedScope([], ["Root::A::Matched"]))); + } + + /// An unrelated qualified name does not match the unlimited-recursion predicate. + [Fact] + public void MatchesUnlimitedSubject_UnrelatedName_ReturnsFalse() + { + Assert.False(ExposeScopeResolver.MatchesUnlimitedSubject("Root::B", + new ExposedScope([new ExposeSubject("Root::A", ExposeRecursionKind.MembershipRecursive)], []))); + } + /// A candidate root that is itself an exposed subject is relevant to the scope. [Fact] public void IsRootRelevantToScope_CandidateEqualsSubject_ReturnsTrue() diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs index ef6f1d33..634f387b 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs @@ -1271,6 +1271,149 @@ public void InterconnectionView_BuildLayout_ExposeRootOnly_NestedSubsystemInDiff Assert.Contains(nestedLabels, l => l is not null && l.Contains("unit2", StringComparison.Ordinal)); } + /// + /// When the resolved expose scope is purely non-recursive — here expose + /// Root::NsA::System; plus expose Root::NsA::System::*;, i.e. one + /// subject and one + /// subject, with no subject + /// carrying unlimited-depth recursion — HasUnlimitedRecursion evaluates to + /// , so interior expansion stops at depth 0: the root's own direct part + /// child (sub : Sub) still renders as its own box, but that box carries no nested + /// unit1/unit2 boxes of its own, even though Sub has its own nested parts. + /// This is the fix-targeting scenario: contrast with + /// , + /// which uses a recursive subject on the same workspace shape and must retain full recursion. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeNonRecursiveSubjects_DoesNotRecurseIntoNestedPartsInterior() + { + var strategy = new InterconnectionViewLayoutStrategy(); + var workspace = BuildCrossNamespaceCompositionWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + ExposeMembers = + [ + new ExposeMember("Root::NsA::System", null, ExposeRecursionKind.MembershipExact), + new ExposeMember("Root::NsA::System", null, ExposeRecursionKind.NamespaceDirectChildren) + ], + ResolvedEdges = + [ + new SysmlEdge("Root::V", "Root::NsA::System", SysmlEdgeKind.Expose), + new SysmlEdge("Root::V", "Root::NsA::System", SysmlEdgeKind.Expose) + ] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var container = CollectBoxes(layout.Nodes).First(b => b.Keyword == "part def"); + Assert.Equal("System", container.Label); + + // The subsystem itself still renders as a box, but as a leaf — its own unit1/unit2 interior + // is never drawn under a non-recursive scope. + var subBox = FindPartBox(layout, "sub : Sub"); + Assert.Empty(CollectBoxes(subBox.Children)); + } + + /// + /// Regression test for the per-branch recursion fix: when the resolved expose scope + /// combines a subject with unlimited-depth recursion (expose System::a::**;, i.e. + /// on the root's own a feature) + /// with a subject that has no depth limit at all (expose System::b;, i.e. + /// on the root's own b feature), only + /// the branch matched by the recursive subject fully recurses — a's own nested parts + /// (a1/a2) render as nested boxes — while the branch matched only by the + /// non-recursive subject stays depth-limited to itself: b still renders as its own box, + /// but as a leaf, even though its type (SubB) has its own nested parts (b1/ + /// b2). Before the fix, a single diagram-wide unlimitedRecursion boolean meant + /// the presence of the recursive subject on a would have also (incorrectly) unlocked + /// full recursion for b's branch. + /// + [Fact] + public void InterconnectionView_BuildLayout_ExposeMixedRecursion_OnlyRecursiveBranchRecurses() + { + // Arrange: System { a : SubA { a1, a2 }, b : SubB { b1, b2 } }, exposed via one recursive + // subject on "a" and one exact (non-recursive) subject on "b". + var strategy = new InterconnectionViewLayoutStrategy(); + var subA = new SysmlDefinitionNode + { + Name = "SubA", + QualifiedName = "SubA", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "a1", QualifiedName = "SubA::a1", FeatureKeyword = "part", FeatureTyping = "UnitA1" }, + new SysmlFeatureNode { Name = "a2", QualifiedName = "SubA::a2", FeatureKeyword = "part", FeatureTyping = "UnitA2" } + ] + }; + var subB = new SysmlDefinitionNode + { + Name = "SubB", + QualifiedName = "SubB", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "b1", QualifiedName = "SubB::b1", FeatureKeyword = "part", FeatureTyping = "UnitB1" }, + new SysmlFeatureNode { Name = "b2", QualifiedName = "SubB::b2", FeatureKeyword = "part", FeatureTyping = "UnitB2" } + ] + }; + var system = new SysmlDefinitionNode + { + Name = "System", + QualifiedName = "System", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "a", QualifiedName = "System::a", FeatureKeyword = "part", FeatureTyping = "SubA" }, + new SysmlFeatureNode { Name = "b", QualifiedName = "System::b", FeatureKeyword = "part", FeatureTyping = "SubB" } + ] + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["System"] = system, + ["SubA"] = subA, + ["SubB"] = subB + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "System::V", + ExposeMembers = + [ + new ExposeMember("System::a", null, ExposeRecursionKind.MembershipRecursive), + new ExposeMember("System::b", null, ExposeRecursionKind.MembershipExact) + ], + ResolvedEdges = + [ + new SysmlEdge("System::V", "System::a", SysmlEdgeKind.Expose), + new SysmlEdge("System::V", "System::b", SysmlEdgeKind.Expose) + ] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: the root's own two direct children both render, but only "a"'s branch recurses. + var container = CollectBoxes(layout.Nodes).First(b => b.Keyword == "part def"); + Assert.Equal("System", container.Label); + + var aBox = FindPartBox(layout, "a : SubA"); + var aNestedLabels = CollectBoxes(aBox.Children).Select(b => b.Label).ToList(); + Assert.Contains(aNestedLabels, l => l is not null && l.Contains("a1", StringComparison.Ordinal)); + Assert.Contains(aNestedLabels, l => l is not null && l.Contains("a2", StringComparison.Ordinal)); + + var bBox = FindPartBox(layout, "b : SubB"); + Assert.Empty(CollectBoxes(bBox.Children)); + } + /// /// Builds a workspace with a genuine two-tier composition root (Root::NsA::System /// composes Root::NsB::Sub via a typed part feature) alongside an unrelated @@ -1508,16 +1651,20 @@ public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TwoTop /// /// When the exposed namespace's direct-child part is itself typed by a part def that - /// has its own nested parts, the single top-level box's own Children contain the - /// nested part boxes — proving the boxless top-level fallback reuses the same recursive - /// container detection (BuildPartItem) as every other nested part, rather than - /// duplicating a shallow copy of that logic. + /// has its own nested parts, the resolved scope here is purely non-recursive + /// (NamespaceDirectChildren, i.e. expose NsRoot::*;, with no recursive subject), + /// so HasUnlimitedRecursion is and interior expansion must stop + /// at depth 0: the single top-level box renders as an intrinsic-sized leaf with no nested part + /// boxes of its own, even though its type (Motherboard) has its own cpu/ + /// chipset parts. This also proves the boxless top-level fallback + /// (CollectTopLevelScopedParts) shares the exact same depth-limiting gate as the normal + /// container-rooted path, since both funnel through the same BuildPartItem. /// [Fact] - public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLevelFeatureIsContainer_RecursesInterior() + public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_ContainerFeature_RendersAsLeaf() { // Arrange: NsRoot::board : Motherboard { cpu, chipset, connect cpu to chipset } — two levels - // deep — exposed via "expose NsRoot::*;" (NamespaceDirectChildren). + // deep — exposed via "expose NsRoot::*;" (NamespaceDirectChildren, non-recursive). var strategy = new InterconnectionViewLayoutStrategy(); var motherboard = new SysmlDefinitionNode { @@ -1559,13 +1706,13 @@ public void InterconnectionView_BuildLayout_ExposeNamespaceDirectChildren_TopLev // Act var layout = strategy.BuildLayout(context, options); - // Assert: exactly one top-level box (no frame), and its own Children hold the nested - // cpu/chipset part boxes recursed via the shared BuildPartItem logic. + // Assert: exactly one top-level box (no frame), and it carries no nested part boxes — the + // non-recursive scope limits expansion to depth 0, so board's own cpu/chipset interior is + // never drawn. var topBox = Assert.Single(layout.Nodes.OfType()); Assert.Contains("board", topBox.Label, StringComparison.Ordinal); - var nestedLabels = CollectBoxes(topBox.Children).Select(b => b.Label).ToList(); - Assert.Contains(nestedLabels, l => l is not null && l.Contains("cpu", StringComparison.Ordinal)); - Assert.Contains(nestedLabels, l => l is not null && l.Contains("chipset", StringComparison.Ordinal)); + var nestedBoxes = CollectBoxes(topBox.Children).ToList(); + Assert.Empty(nestedBoxes); } /// @@ -1770,4 +1917,132 @@ private static SysmlWorkspace BuildTwoTopLevelPartsWithConnectionWorkspace() } }; } + + /// + /// Regression test for the owner-scoped top-level connection resolution fix: two different + /// containing part defs (SubsystemA, SubsystemB) each declare their own + /// logger-simple-named part feature, and both subsystems' own direct children are + /// exposed as separate top-level scoped features (via one NamespaceDirectChildren + /// subject per subsystem, so neither subsystem itself qualifies as the diagram's single + /// root — see + /// for the same self-exclusion mechanism). SubsystemA also declares a connection + /// between its own logger and sensor parts. SubsystemB's own logger + /// is declared first in enumeration order, so a flat + /// simple-name index built across all four collected top-level parts (the pre-fix + /// BuildPartIndex-of-everything approach) would resolve SubsystemA's connection + /// endpoint "logger" to SubsystemB's logger instead of its own — a bogus + /// cross-owner connector. The fix must resolve the connection against only SubsystemA's + /// own restricted index, drawing exactly one connector, between SubsystemA's own + /// logger and sensor boxes. + /// + [Fact] + public void InterconnectionView_BuildLayout_TopLevelNameCollisionAcrossOwners_ResolvesOwnConnection() + { + // Arrange: SubsystemB { logger : LoggerB, other : OtherB } declared first, then + // SubsystemA { logger : LoggerA, sensor : SensorA, connect logger to sensor }. Both exposed + // via "expose SubsystemA::*;" and "expose SubsystemB::*;" (NamespaceDirectChildren), so + // neither subsystem def itself is root-relevant (self-excluded by that recursion kind) and + // FindRoot selects no root, falling to the boxless top-level path. + var strategy = new InterconnectionViewLayoutStrategy(); + var subsystemB = new SysmlDefinitionNode + { + Name = "SubsystemB", + QualifiedName = "SubsystemB", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "logger", QualifiedName = "SubsystemB::logger", FeatureKeyword = "part", FeatureTyping = "LoggerB" }, + new SysmlFeatureNode { Name = "other", QualifiedName = "SubsystemB::other", FeatureKeyword = "part", FeatureTyping = "OtherB" }, + new SysmlConnectionNode + { + Name = "connB", + QualifiedName = "SubsystemB::connB", + ConnectionKeyword = "connection", + EndpointA = "logger", + EndpointB = "other" + } + ] + }; + var subsystemA = new SysmlDefinitionNode + { + Name = "SubsystemA", + QualifiedName = "SubsystemA", + DefinitionKeyword = "part def", + Children = + [ + new SysmlFeatureNode { Name = "logger", QualifiedName = "SubsystemA::logger", FeatureKeyword = "part", FeatureTyping = "LoggerA" }, + new SysmlFeatureNode { Name = "sensor", QualifiedName = "SubsystemA::sensor", FeatureKeyword = "part", FeatureTyping = "SensorA" }, + new SysmlConnectionNode + { + Name = "conn1", + QualifiedName = "SubsystemA::conn1", + ConnectionKeyword = "connection", + EndpointA = "logger", + EndpointB = "sensor" + } + ] + }; + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["SubsystemB"] = subsystemB, + ["SubsystemB::logger"] = subsystemB.Children[0], + ["SubsystemB::other"] = subsystemB.Children[1], + ["SubsystemA"] = subsystemA, + ["SubsystemA::logger"] = subsystemA.Children[0], + ["SubsystemA::sensor"] = subsystemA.Children[1], + ["SubsystemA::conn1"] = subsystemA.Children[2] + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "M::V", + ExposeMembers = + [ + new ExposeMember("SubsystemA", null, ExposeRecursionKind.NamespaceDirectChildren), + new ExposeMember("SubsystemB", null, ExposeRecursionKind.NamespaceDirectChildren) + ], + ResolvedEdges = + [ + new SysmlEdge("M::V", "SubsystemA", SysmlEdgeKind.Expose), + new SysmlEdge("M::V", "SubsystemB", SysmlEdgeKind.Expose) + ] + }.WithResolvedExposeMembers(); + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + // Act + var layout = strategy.BuildLayout(context, options); + + // Assert: four top-level boxes (both subsystems' own two parts), exactly two connectors (one + // per subsystem's own connection), and SubsystemA's connector connects SubsystemA's own + // logger and sensor boxes — not SubsystemB's logger box. + Assert.Equal(4, layout.Nodes.OfType().Count()); + var lines = CollectLines(layout.Nodes).ToList(); + Assert.Equal(2, lines.Count); + + var loggerABox = FindPartBox(layout, "logger : LoggerA"); + var sensorABox = FindPartBox(layout, "sensor : SensorA"); + var loggerBBox = FindPartBox(layout, "logger : LoggerB"); + var otherBBox = FindPartBox(layout, "other : OtherB"); + + // The connector linking SubsystemA's own logger and sensor boxes must exist, and must not + // instead link SubsystemB's logger box (the pre-fix bogus cross-owner connection). + var subsystemAConnector = lines.SingleOrDefault(l => LineTouchesBox(l, loggerABox) && LineTouchesBox(l, sensorABox)); + Assert.NotNull(subsystemAConnector); + Assert.False(LineTouchesBox(subsystemAConnector, loggerBBox), "connector must not also touch SubsystemB's logger box"); + Assert.False(LineTouchesBox(subsystemAConnector, otherBBox), "connector must not also touch SubsystemB's other box"); + } + + /// + /// Determines whether either end of 's waypoints falls within + /// 's bounds (with a small tolerance for port placement on the box's + /// own border), used to identify which rendered box a connector actually terminates at. + /// + private static bool LineTouchesBox(LayoutLine line, LayoutBox box, double tolerance = 5.0) => + line.Waypoints.Any(p => + p.X >= box.X - tolerance && p.X <= box.X + box.Width + tolerance && + p.Y >= box.Y - tolerance && p.Y <= box.Y + box.Height + tolerance); }