From f1dcb83a3db193fe6d9cf9d62ed0f69f195e7f15 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 15:21:11 -0400 Subject: [PATCH 1/8] Fix InterconnectionView to depth-limit recursion for non-recursive expose scopes Add HasUnlimitedRecursion helper computed once per BuildLayout call from the resolved expose scope's Subjects, and thread the resulting boolean through LayOutInterior, CollectParts, BuildPartItem, and CollectTopLevelScopedParts. A purely non-recursive scope (MembershipExact and/or NamespaceDirectChildren subjects only) now limits interior expansion to the selected root's own direct part children, rendering any deeper container part as an intrinsic-sized leaf box instead of always recursing fully into its interior. A scope with at least one recursive subject, or no scope at all (--auto), continues to recurse fully at every depth, unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../InterconnectionViewLayoutStrategy.cs | 127 ++++++++++++++---- 1 file changed, 99 insertions(+), 28 deletions(-) diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs index 11174d79..e53c1937 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs @@ -39,7 +39,12 @@ 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 when the resolved expose scope has no depth limit — i.e. it contains at +/// least one subject with unlimited-depth recursion, or there is no scope at all (see +/// ); otherwise, under a purely non-recursive scope, expansion +/// stops after the root's own direct part children (depth 0) and any deeper container renders as an +/// intrinsic-sized leaf box instead of recursing further. /// /// /// Not every resolved expose scope names a single part def worth treating as "the" @@ -104,6 +109,10 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) var scope = ExposeScopeResolver.ResolveExposedScope(context.Workspace, context.ViewNode); + // Whether interior recursion should be unconditionally unlimited for this whole diagram — + // see HasUnlimitedRecursion's own remarks for the rationale and its documented limitation. + var unlimitedRecursion = HasUnlimitedRecursion(scope); + // Index of candidate container definitions (non-stdlib part defs with at least one part child). // Built before FindRoot so root-selection can resolve which candidates are themselves used as // another candidate's nested part type (see FindRoot's composition-graph-root preference). @@ -123,7 +132,7 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) // preserved unchanged. if (scope is not null) { - var topLevelParts = CollectTopLevelScopedParts(context.Workspace, scope, theme, defsByName); + var topLevelParts = CollectTopLevelScopedParts(context.Workspace, scope, theme, defsByName, unlimitedRecursion); if (topLevelParts.Count > 0) { var partIndex = BuildPartIndex(topLevelParts); @@ -151,7 +160,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, unlimitedRecursion); // 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 +197,12 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) /// /// The view's resolved expose containment-subtree scope, or null when no scoping applies. /// + /// + /// Whether interior recursion is unconditionally unlimited for this diagram — computed once by + /// from and threaded unchanged + /// through every recursive call so the whole diagram shares one depth-limiting decision. See + /// 's remarks for how this actually gates recursion. + /// /// The laid-out interior size and content. private static InteriorLayout LayOutInterior( SysmlDefinitionNode def, @@ -195,9 +210,10 @@ private static InteriorLayout LayOutInterior( int depth, IReadOnlyDictionary defsByName, ISet visited, - ExposedScope? scope) + ExposedScope? scope, + bool unlimitedRecursion) { - var parts = CollectParts(def, theme, depth, defsByName, visited, scope); + var parts = CollectParts(def, theme, depth, defsByName, visited, scope, unlimitedRecursion); var partIndex = BuildPartIndex(parts); var pairs = ResolveConnections(def, partIndex); @@ -334,6 +350,33 @@ private static InteriorLayout LayOutInteriorWithConnections( return new InteriorLayout(containerWidth, containerHeight, content); } + /// + /// Returns when interior recursion should be unconditionally unlimited — + /// 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 is a single, diagram-wide decision + /// computed once (in ) and threaded unchanged through every recursive + /// call — it is never re-evaluated per feature or per composition branch. This is a deliberate + /// simplification: a scope that combines a recursive subject (e.g. expose X::**;) with a + /// non-recursive subject (e.g. expose Y;) uses unlimited recursion for the whole + /// diagram, even for parts reached only via the non-recursive subject — precisely attributing + /// recursion depth to "which subject caused this part to be included" would require carrying + /// subject provenance through every part-collection call site, which is a materially larger + /// change reserved for a future iteration if this simplification proves insufficient in + /// practice. + /// + /// 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, meaning + /// interior expansion must stop at depth 0 (see ). + /// + 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,14 +492,16 @@ 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 + /// governed purely by (see ): + /// when the resolved scope has no subject with unlimited-depth recursion, every part is included + /// as a node, but interior expansion stops at depth 0 and each deeper container renders as an + /// intrinsic-sized leaf box instead of recursing further. /// private static IReadOnlyList CollectParts( SysmlDefinitionNode root, @@ -464,7 +509,8 @@ private static IReadOnlyList CollectParts( int depth, IReadOnlyDictionary defsByName, ISet visited, - ExposedScope? scope) + ExposedScope? scope, + bool unlimitedRecursion) { var result = new List(); foreach (var feature in root.Children.OfType()) @@ -480,7 +526,7 @@ private static IReadOnlyList CollectParts( continue; } - result.Add(BuildPartItem(feature, theme, depth, defsByName, visited)); + result.Add(BuildPartItem(feature, theme, depth, defsByName, visited, unlimitedRecursion)); } return result; @@ -489,39 +535,57 @@ private static IReadOnlyList CollectParts( /// /// 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. + /// + /// Whether interior recursion is unconditionally unlimited for this diagram — computed once, in + /// , from the resolved expose scope's + /// via . When + /// , a container part's own interior is never expanded — it always + /// renders as an intrinsic-sized leaf box, regardless of . This method + /// is only ever invoked at depth == 0 (directly, from or + /// ) or via its own recursive + /// call below, which is itself gated by this same flag — so gating solely on + /// (without also re-checking ) is + /// sufficient and equivalent to "stop expanding past depth 0". + /// /// 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 true here) is threaded through + // unchanged so the whole diagram continues to share one 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, 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); } @@ -539,6 +603,12 @@ private static PartItem BuildPartItem( /// The view's resolved expose containment-subtree scope. /// The active rendering theme. /// Container-definition index keyed by qualified and simple name. + /// + /// Whether interior recursion is unconditionally unlimited for this diagram — computed once, in + /// , and forwarded to unchanged so this + /// boxless fallback path applies the exact same depth-limiting decision as the normal + /// container-rooted path. + /// /// /// The matched top-level parts, each recursively laid out via exactly /// as a normal nested container part would be, in 's declaration @@ -548,7 +618,8 @@ private static IReadOnlyList CollectTopLevelScopedParts( SysmlWorkspace workspace, ExposedScope scope, Theme theme, - IReadOnlyDictionary defsByName) + IReadOnlyDictionary defsByName, + bool unlimitedRecursion) { var matched = new List(); foreach (var (qualifiedName, node) in workspace.Declarations) @@ -585,7 +656,7 @@ private static IReadOnlyList CollectTopLevelScopedParts( foreach (var feature in topLevel) { var visited = new HashSet(StringComparer.Ordinal); - result.Add(BuildPartItem(feature, theme, depth: 0, defsByName, visited)); + result.Add(BuildPartItem(feature, theme, depth: 0, defsByName, visited, unlimitedRecursion)); } return result; From 78ba6b586ca2998278f8b122042aa01c36664bf4 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 15:21:20 -0400 Subject: [PATCH 2/8] Add/rename InterconnectionView tests for depth-limited recursion - Rename ..._TopLevelFeatureIsContainer_RecursesInterior to ..._ContainerFeature_RendersAsLeaf and invert its assertion: it previously encoded the pre-fix over-recursion bug for a non-recursive scope; now asserts the top-level container feature renders as a leaf with no nested children, matching the corrected boxless-fallback depth-limiting. - Add ExposeNonRecursiveSubjects_DoesNotRecurseIntoNestedPartsInterior, confirming a purely non-recursive scope (MembershipExact + NamespaceDirectChildren) stops interior expansion at depth 0 on the normal container-rooted path. - The existing ExposeRootOnly_NestedSubsystemInDifferentNamespace_ RendersItsOwnUnits test (MembershipRecursive) now doubles as the regression guard proving full recursion is unchanged when the scope has an unlimited-depth subject. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../InterconnectionViewLayoutStrategyTests.cs | 73 ++++++++++++++++--- 1 file changed, 62 insertions(+), 11 deletions(-) diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs index ef6f1d33..b0ec4f5e 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs @@ -1271,6 +1271,53 @@ 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)); + } + /// /// 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 +1555,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 +1610,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); } /// From c551dafce0c6849ad28868346e828bedff0f5d8d Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 15:21:29 -0400 Subject: [PATCH 3/8] Update companion docs for InterconnectionView depth-limited recursion - design doc: describe HasUnlimitedRecursion, conditional recursion in BuildLayout/CollectParts/BuildPartItem/CollectTopLevelScopedParts, and document the global-boolean simplification and its known limitation (a scope combining a recursive and a non-recursive subject uses unlimited recursion for the whole diagram). - reqstream: revise ScopedTopLevelFeatureFallback's title/tests to match the corrected fallback behavior; add new requirement ExposeScopingRecursionDepthLimit covering both the normal root path and the top-level fallback path. - verification doc: update Verification Approach, Acceptance Criteria, and Test Scenarios for the new/renamed tests. - user guide: clarify that Interconnection View interior recursion depth depends on whether the resolved expose scope is recursive. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../interconnection-view-layout-strategy.md | 113 +++++++++++++----- .../interconnection-view-layout-strategy.yaml | 38 +++++- docs/user_guide/introduction.md | 12 ++ .../interconnection-view-layout-strategy.md | 41 +++++-- 4 files changed, 162 insertions(+), 42 deletions(-) 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..d16a3434 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 @@ -23,19 +23,52 @@ container size and content produced by laying out one definition's interior). ###### `BuildLayout(ViewContext context, RenderOptions options)` Entry point. Resolves the view's `expose` scope via `ExposeScopeResolver.ResolveExposedScope`, -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 +computes a single diagram-wide `unlimitedRecursion` boolean via `HasUnlimitedRecursion(scope)` (see +_Depth-Limited Recursion_ below), 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`, 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 `unlimitedRecursion`, threaded unchanged through every +recursive call), 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`. +###### Depth-Limited Recursion (`HasUnlimitedRecursion`) + +`HasUnlimitedRecursion(scope)` returns `true` — meaning interior recursion is unconditionally +unlimited for the **whole diagram** — when `scope` is `null` (no `expose` statement resolved for +this view, including the synthesized `--auto` view) or when at least one of `scope.Subjects` carries +`ExposeRecursionKind.MembershipRecursive` or `ExposeRecursionKind.NamespaceRecursive` (i.e. an +`expose X::**;` or `expose X::*::**;` form). It returns `false` only when every resolved subject is +non-recursive (`MembershipExact` and/or `NamespaceDirectChildren` — `expose X;` and/or +`expose X::*;` forms, with no recursive subject at all). This single boolean, computed once in +`BuildLayout` from the resolved scope's `Subjects`, is threaded unchanged through +`LayOutInterior`, `CollectParts`, `BuildPartItem`, and `CollectTopLevelScopedParts` — it is never +recomputed per feature or per composition branch. + +When `unlimitedRecursion` is `false`, `BuildPartItem` still includes every part reached at 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 `unlimitedRecursion` is `true` (including +every `scope is null` case), behavior is completely unchanged from before this depth-limiting was +introduced — recursion is unconditional at every depth. + +**Known simplification and its limitation.** This is a deliberate global (diagram-wide), not +per-subject or per-composition-branch, decision. A scope that combines a recursive subject with a +non-recursive subject — e.g. `expose SystemDef::**; expose OtherDef;` in the same view — uses +**unlimited recursion for the entire diagram**, even for parts reached only via the non-recursive +subject. Precisely attributing "which subject caused this part to be included" would require +carrying subject provenance through every part-collection call site (`CollectParts` → +`BuildPartItem` → `TryResolveContainer`), none of which currently track _why_ a part was reached — +a materially larger change than this feature makes. This is a known, sanctioned simplification +rather than a silent gap; it is documented here, in the `HasUnlimitedRecursion` XmlDoc, and in the +corresponding requirement's justification text. + ###### No-single-root scoped fallback (`CollectTopLevelScopedParts`, `ResolveTopLevelConnections`) Not every resolved `expose` scope names a single `part def` worth treating as "the" subject of the @@ -48,14 +81,16 @@ drawing. Before this fallback existed, `FindRoot` returning `null` always produc canvas in this shape, even though the scope named something concrete. When `FindRoot` returns `null` and a scope is resolved, `BuildLayout` calls -`CollectTopLevelScopedParts(workspace, scope, theme, defsByName)`, which scans +`CollectTopLevelScopedParts(workspace, scope, theme, defsByName, unlimitedRecursion)`, which scans `workspace.Declarations` for every non-standard-library `SysmlFeatureNode` with `FeatureKeyword == "part"` whose qualified name satisfies `ExposeScopeResolver.IsInSubjectScope`, excludes any matched feature that is itself nested (`"::"`-prefixed) under another matched 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, +part, extracted so the logic is never duplicated, and gated by the same `unlimitedRecursion` flag +so this boxless fallback path applies the identical depth-limiting decision as the normal +container-rooted path. 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 @@ -68,13 +103,20 @@ boxes (and any ports/lines) directly as top-level siblings, instead of the usual 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)` +###### `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`, and `scope: null` — nested composition structure's own membership +is never re-derived from its own qualified name, matching `CollectParts`'s own documented recursion +behavior) 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). 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 scope. 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 +147,12 @@ 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 + `unlimitedRecursion` is `true` (see _Depth-Limited Recursion_ above); when `false`, expansion + never proceeds past 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 +234,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 +250,10 @@ 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 `HasUnlimitedRecursion`/`unlimitedRecursion` (see _Depth-Limited +Recursion_ above) — governs **how deep into each shown part's own interior the diagram recurses**. +`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 +266,17 @@ 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 `unlimitedRecursion` is always `true` — +completely unchanged from the pre-scoping behavior. + +Once a root is selected and its direct parts are narrowed, `unlimitedRecursion` decides whether the +diagram continues to recurse into each shown part's own composed interior at all: when the resolved +scope contains at least one subject with unlimited-depth recursion (`expose X::**;` / +`expose X::*::**;`), or there is no scope at all, every container part fully expands its own nested +parts, unbounded in depth — identical to today's behavior before this depth-limiting was +introduced. When the resolved scope is purely non-recursive (only `expose X;` and/or +`expose X::*;` forms), expansion stops after the root's own direct part children: a deeper container +part still renders as its own box, but with no nested children of its own drawn inside it. ###### Placement and routing @@ -257,7 +313,8 @@ diagnostics, so the returned `LayoutTree` carries no layout-quality warnings. - `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`. + `BuildLayout`, `FindRoot`, and `CollectParts`; `ExposedScope.Subjects`' recursion kinds also drive + `HasUnlimitedRecursion`'s depth-limiting decision. - `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/interconnection-view-layout-strategy.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.yaml index 539f09ab..6b61503d 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,33 @@ sections: - InterconnectionView_BuildLayout_ExposeSinglePart_NarrowsToThatPart - InterconnectionView_BuildLayout_ExposeMultipleParts_UnionsBothSubtrees + - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ExposeScopingRecursionDepthLimit + title: >- + When the resolved expose scope has no subject with unlimited-depth recursion (i.e. every + subject is MembershipExact and/or NamespaceDirectChildren), InterconnectionViewLayoutStrategy + shall limit interior expansion to the selected root's (or top-level scoped feature's) own + direct part children, rendering any deeper container part 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 requirement is deliberately implemented as a single + diagram-wide decision (`HasUnlimitedRecursion`, computed once from the resolved scope's + `Subjects`), not a per-subject or per-composition-branch decision: a scope that combines a + recursive subject with a non-recursive subject uses unlimited recursion for the whole + diagram, which is a known, documented, and sanctioned simplification (see the design + documentation's "Depth-Limited Recursion" section). A scope with at least one recursive + subject, or 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-NoExposeFallback title: >- When a view's ViewContext carries no resolved Expose edge — including a null ViewNode @@ -257,8 +284,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 +296,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 diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 6f11fb85..1521dde6 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -245,6 +245,18 @@ 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 the resolved `expose` scope is recursive. A scope +containing at least one recursive form (`expose X::**;` or `expose X::*::**;`), or no `expose` +statement at all, recurses fully — every nested container's own interior is shown, at any depth. +A scope containing **only** non-recursive forms (`expose X;` and/or `expose X::*;`, with no +recursive form present anywhere in the view) limits expansion to the selected root's (or +top-level scoped feature's) own direct part children: a deeper nested part still renders as its +own box, but its own interior is not drawn. 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. + 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/interconnection-view-layout-strategy.md b/docs/verification/sysml2-tools-core/layout/internal/interconnection-view-layout-strategy.md index fa7583ff..48e6ee76 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,24 @@ 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. ##### Test Environment @@ -104,14 +115,22 @@ 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. ##### Test Scenarios @@ -143,7 +162,9 @@ 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 | From 641c87db080d4e4b8f448893475701c56608a65c Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 17:10:17 -0400 Subject: [PATCH 4/8] Fix InterconnectionView per-branch recursion depth and top-level connection name collisions Fix 1: Replace the diagram-wide unlimitedRecursion boolean with per-branch tracking. Previously, HasUnlimitedRecursion computed one bool from the whole resolved scope and threaded it unchanged through every branch of the diagram, so a scope combining a recursive subject (expose A::**;) with a non-recursive subject (expose B;) applied unlimited recursion to the ENTIRE diagram, over-collapsing B's branch which should have stayed depth-limited to itself. Added ExposeScopeResolver.MatchesUnlimitedSubject(qualifiedName, scope) - a narrower sibling of IsInSubjectScope that reports whether a feature matches specifically a MembershipRecursive or NamespaceRecursive subject (never via ExplicitMembers). InterconnectionViewLayoutStrategy now decides recursion once per top-level branch (CollectParts at depth 0, and independently per feature in CollectTopLevelScopedParts) via a new DetermineBranchUnlimitedRecursion helper, and threads that concrete per-branch bool (ancestorUnlimitedRecursion) unchanged to every descendant, mirroring how the old global boolean was threaded but decided per-branch instead of per-diagram. HasUnlimitedRecursion is retained only as the rare depth-0-no-qualified-name fallback. Fix 2: Fix simple-name collision in the top-level boxless connector resolution. BuildPartIndex (used by the single-root ResolveConnections path) indexes by simple name only, which is safe there because one containing part def can't have two direct children with the same simple name. That assumption breaks for ResolveTopLevelConnections, which previously built one flat cross-workspace part index from every top-level part CollectTopLevelScopedParts collected across the entire workspace - if two different containing definitions each declared a same-named part (e.g. both part logger : Logger;), TryAdd silently kept only the first occurrence, causing bogus or dropped connections. Added a TopLevelPart(Part, OwnerQualifiedName) record; CollectTopLevelScopedParts now returns each matched feature paired with its owning definition's qualified name. ResolveTopLevelConnections groups top-level parts by OwnerQualifiedName and resolves each definition's own connections only against that definition's own restricted part index, never a flat cross-workspace union. The single-root BuildPartIndex/ResolveConnections path is unchanged. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Layout/Internal/ExposeScopeResolver.cs | 36 +++ .../InterconnectionViewLayoutStrategy.cs | 280 +++++++++++++----- 2 files changed, 243 insertions(+), 73 deletions(-) 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 e53c1937..3bdc5606 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/InterconnectionViewLayoutStrategy.cs @@ -40,11 +40,14 @@ namespace DemaConsulting.SysML2Tools.Layout.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. This recursion is -/// unconditional only when the resolved expose scope has no depth limit — i.e. it contains at -/// least one subject with unlimited-depth recursion, or there is no scope at all (see -/// ); otherwise, under a purely non-recursive scope, expansion -/// stops after the root's own direct part children (depth 0) and any deeper container renders as an -/// intrinsic-sized leaf box instead of recursing further. +/// 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" @@ -54,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 @@ -85,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. @@ -109,10 +137,6 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) var scope = ExposeScopeResolver.ResolveExposedScope(context.Workspace, context.ViewNode); - // Whether interior recursion should be unconditionally unlimited for this whole diagram — - // see HasUnlimitedRecursion's own remarks for the rationale and its documented limitation. - var unlimitedRecursion = HasUnlimitedRecursion(scope); - // Index of candidate container definitions (non-stdlib part defs with at least one part child). // Built before FindRoot so root-selection can resolve which candidates are themselves used as // another candidate's nested part type (see FindRoot's composition-graph-root preference). @@ -132,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, unlimitedRecursion); - 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); @@ -160,7 +184,7 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) visited.Add(root.QualifiedName); } - var interior = LayOutInterior(root, theme, depth: 0, defsByName, visited, scope, unlimitedRecursion); + 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 @@ -197,11 +221,13 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) /// /// The view's resolved expose containment-subtree scope, or null when no scoping applies. /// - /// - /// Whether interior recursion is unconditionally unlimited for this diagram — computed once by - /// from and threaded unchanged - /// through every recursive call so the whole diagram shares one depth-limiting decision. See - /// 's remarks for how this actually gates recursion. + /// + /// 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( @@ -211,9 +237,9 @@ private static InteriorLayout LayOutInterior( IReadOnlyDictionary defsByName, ISet visited, ExposedScope? scope, - bool unlimitedRecursion) + bool? ancestorUnlimitedRecursion) { - var parts = CollectParts(def, theme, depth, defsByName, visited, scope, unlimitedRecursion); + var parts = CollectParts(def, theme, depth, defsByName, visited, scope, ancestorUnlimitedRecursion); var partIndex = BuildPartIndex(parts); var pairs = ResolveConnections(def, partIndex); @@ -351,27 +377,32 @@ private static InteriorLayout LayOutInteriorWithConnections( } /// - /// Returns when interior recursion should be unconditionally unlimited — - /// 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 is a single, diagram-wide decision - /// computed once (in ) and threaded unchanged through every recursive - /// call — it is never re-evaluated per feature or per composition branch. This is a deliberate - /// simplification: a scope that combines a recursive subject (e.g. expose X::**;) with a - /// non-recursive subject (e.g. expose Y;) uses unlimited recursion for the whole - /// diagram, even for parts reached only via the non-recursive subject — precisely attributing - /// recursion depth to "which subject caused this part to be included" would require carrying - /// subject provenance through every part-collection call site, which is a materially larger - /// change reserved for a future iteration if this simplification proves insufficient in - /// practice. + /// 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, meaning - /// interior expansion must stop at depth 0 (see ). + /// when the resolved scope has no unlimited-depth subject. /// private static bool HasUnlimitedRecursion(ExposedScope? scope) => scope is null || @@ -498,11 +529,30 @@ scope is null || /// 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 - /// governed purely by (see ): - /// when the resolved scope has no subject with unlimited-depth recursion, every part is included - /// as a node, but interior expansion stops at depth 0 and each deeper container renders as an - /// intrinsic-sized leaf box instead of recursing further. + /// 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, @@ -510,7 +560,7 @@ private static IReadOnlyList CollectParts( IReadOnlyDictionary defsByName, ISet visited, ExposedScope? scope, - bool unlimitedRecursion) + bool? ancestorUnlimitedRecursion) { var result = new List(); foreach (var feature in root.Children.OfType()) @@ -526,12 +576,39 @@ private static IReadOnlyList CollectParts( continue; } + 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 @@ -548,16 +625,14 @@ private static IReadOnlyList CollectParts( /// Container-definition index keyed by qualified and simple name. /// Qualified names already on the recursion path, guarding against cycles. /// - /// Whether interior recursion is unconditionally unlimited for this diagram — computed once, in - /// , from the resolved expose scope's - /// via . When - /// , a container part's own interior is never expanded — it always - /// renders as an intrinsic-sized leaf box, regardless of . This method - /// is only ever invoked at depth == 0 (directly, from or - /// ) or via its own recursive - /// call below, which is itself gated by this same flag — so gating solely on - /// (without also re-checking ) is - /// sufficient and equivalent to "stop expanding past depth 0". + /// 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( @@ -575,10 +650,11 @@ private static PartItem BuildPartItem( // 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's own qualified name is never - // re-checked against scope; unlimitedRecursion (already true here) is threaded through - // unchanged so the whole diagram continues to share one depth-limiting decision. + // 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, unlimitedRecursion); + 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); } @@ -603,23 +679,17 @@ private static PartItem BuildPartItem( /// The view's resolved expose containment-subtree scope. /// The active rendering theme. /// Container-definition index keyed by qualified and simple name. - /// - /// Whether interior recursion is unconditionally unlimited for this diagram — computed once, in - /// , and forwarded to unchanged so this - /// boxless fallback path applies the exact same depth-limiting decision as the normal - /// container-rooted path. - /// /// - /// 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, - IReadOnlyDictionary defsByName, - bool unlimitedRecursion) + IReadOnlyDictionary defsByName) { var matched = new List(); foreach (var (qualifiedName, node) in workspace.Declarations) @@ -652,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, unlimitedRecursion)); + 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. @@ -672,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)) From 3e96f58efd446c0006419ba297fc026428b43797 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 17:10:29 -0400 Subject: [PATCH 5/8] Add tests for per-branch recursion and owner-scoped top-level connection resolution - ExposeScopeResolverTests: 7 new cases for MatchesUnlimitedSubject covering MembershipRecursive (self/subtree), NamespaceRecursive (subtree), MembershipExact, NamespaceDirectChildren, ExplicitMembers-only, and an unrelated name. - InterconnectionViewLayoutStrategyTests: - ExposeMixedRecursion_OnlyRecursiveBranchRecurses: a scope with expose A::**; expose B; on two sibling parts confirms A's branch fully recurses into its nested parts while B's branch stays depth-limited to itself, in the same diagram. - TopLevelNameCollisionAcrossOwners_ResolvesOwnConnection: two different part defs each declaring a same-named logger part and their own connection, both exposed as top-level scoped features (forcing the boxless fallback), confirms each definition's own connection resolves to its own logger/other parts, never the other definition's same-named part. All pre-existing tests continue to pass unchanged (35 -> 78 total InterconnectionView + ExposeScopeResolver tests, 0 regressions). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../Layout/ExposeScopeResolverTests.cs | 74 ++++++ .../InterconnectionViewLayoutStrategyTests.cs | 224 ++++++++++++++++++ 2 files changed, 298 insertions(+) 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 b0ec4f5e..634f387b 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs @@ -1318,6 +1318,102 @@ public void InterconnectionView_BuildLayout_ExposeNonRecursiveSubjects_DoesNotRe 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 @@ -1821,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); } From ff9094341f4cc9d135103dad41d92ab47e0d1d99 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 17:10:40 -0400 Subject: [PATCH 6/8] Update companion docs for per-branch recursion and owner-scoped connection resolution - design/interconnection-view-layout-strategy.md: rewrote "Depth-Limited Recursion" as "Per-Branch Depth-Limited Recursion", documenting DetermineBranchUnlimitedRecursion and explicitly superseding the earlier global-boolean simplification note; documented the TopLevelPart record and owner-scoped grouping in ResolveTopLevelConnections. - design/expose-scope-resolver.md: documented MatchesUnlimitedSubject alongside IsInSubjectScope. - reqstream/interconnection-view-layout-strategy.yaml: reworded ExposeScopingRecursionDepthLimit for per-branch semantics, added ExposeScopingPerBranchRecursionIndependence (Fix 1) and ScopedTopLevelFeatureFallbackOwnerScopedConnections (Fix 2) requirements. - reqstream/expose-scope-resolver.yaml: added UnlimitedSubjectMatch requirement. - verification/*.md: updated verification approach, acceptance criteria, and test-scenario tables for both units to reflect the new tests and per-branch/owner-scoped behavior. - user_guide/introduction.md: reworded the Interconnection View recursion-depth paragraph to describe the per-branch decision, and documented owner-scoped top-level connection resolution. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../layout/internal/expose-scope-resolver.md | 23 ++ .../interconnection-view-layout-strategy.md | 243 ++++++++++++------ .../internal/expose-scope-resolver.yaml | 26 ++ .../interconnection-view-layout-strategy.yaml | 72 +++++- docs/user_guide/introduction.md | 28 +- .../layout/internal/expose-scope-resolver.md | 28 +- .../interconnection-view-layout-strategy.md | 29 +++ 7 files changed, 341 insertions(+), 108 deletions(-) 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 d16a3434..9bbabc34 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). @@ -23,51 +26,82 @@ container size and content produced by laying out one definition's interior). ###### `BuildLayout(ViewContext context, RenderOptions options)` Entry point. Resolves the view's `expose` scope via `ExposeScopeResolver.ResolveExposedScope`, -computes a single diagram-wide `unlimitedRecursion` boolean via `HasUnlimitedRecursion(scope)` (see -_Depth-Limited Recursion_ below), 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`, 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 `unlimitedRecursion`, threaded unchanged through every -recursive call), 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`. - -###### Depth-Limited Recursion (`HasUnlimitedRecursion`) - -`HasUnlimitedRecursion(scope)` returns `true` — meaning interior recursion is unconditionally -unlimited for the **whole diagram** — when `scope` is `null` (no `expose` statement resolved for -this view, including the synthesized `--auto` view) or when at least one of `scope.Subjects` carries -`ExposeRecursionKind.MembershipRecursive` or `ExposeRecursionKind.NamespaceRecursive` (i.e. an -`expose X::**;` or `expose X::*::**;` form). It returns `false` only when every resolved subject is -non-recursive (`MembershipExact` and/or `NamespaceDirectChildren` — `expose X;` and/or -`expose X::*;` forms, with no recursive subject at all). This single boolean, computed once in -`BuildLayout` from the resolved scope's `Subjects`, is threaded unchanged through -`LayOutInterior`, `CollectParts`, `BuildPartItem`, and `CollectTopLevelScopedParts` — it is never -recomputed per feature or per composition branch. - -When `unlimitedRecursion` is `false`, `BuildPartItem` still includes every part reached at 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 `unlimitedRecursion` is `true` (including -every `scope is null` case), behavior is completely unchanged from before this depth-limiting was -introduced — recursion is unconditional at every depth. - -**Known simplification and its limitation.** This is a deliberate global (diagram-wide), not -per-subject or per-composition-branch, decision. A scope that combines a recursive subject with a -non-recursive subject — e.g. `expose SystemDef::**; expose OtherDef;` in the same view — uses -**unlimited recursion for the entire diagram**, even for parts reached only via the non-recursive -subject. Precisely attributing "which subject caused this part to be included" would require -carrying subject provenance through every part-collection call site (`CollectParts` → -`BuildPartItem` → `TryResolveContainer`), none of which currently track _why_ a part was reached — -a materially larger change than this feature makes. This is a known, sanctioned simplification -rather than a silent gap; it is documented here, in the `HasUnlimitedRecursion` XmlDoc, and in the -corresponding requirement's justification text. +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`, 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`) @@ -81,42 +115,77 @@ drawing. Before this fallback existed, `FindRoot` returning `null` always produc canvas in this shape, even though the scope named something concrete. When `FindRoot` returns `null` and a scope is resolved, `BuildLayout` calls -`CollectTopLevelScopedParts(workspace, scope, theme, defsByName, unlimitedRecursion)`, which scans +`CollectTopLevelScopedParts(workspace, scope, theme, defsByName)`, which scans `workspace.Declarations` for every non-standard-library `SysmlFeatureNode` with `FeatureKeyword == "part"` whose qualified name satisfies `ExposeScopeResolver.IsInSubjectScope`, excludes any matched feature that is itself nested (`"::"`-prefixed) under another matched 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, and gated by the same `unlimitedRecursion` flag -so this boxless fallback path applies the identical depth-limiting decision as the normal -container-rooted path. 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. +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 **only when `unlimitedRecursion` is `true`** (with the resolved child's qualified -name added to a copy of `visited`, and `scope: null` — nested composition structure's own membership -is never re-derived from its own qualified name, matching `CollectParts`'s own documented recursion -behavior) or computing an intrinsic leaf size via `ComputePartSize` otherwise — which happens both +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). 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 scope. +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. @@ -147,9 +216,10 @@ 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 — but only when - `unlimitedRecursion` is `true` (see _Depth-Limited Recursion_ above); when `false`, expansion - never proceeds past depth 0 and a deeper container is instead sized as an intrinsic leaf. The + `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. @@ -251,8 +321,9 @@ 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**, then narrows **which of that root's parts are -shown**, and finally — via `HasUnlimitedRecursion`/`unlimitedRecursion` (see _Depth-Limited -Recursion_ above) — governs **how deep into each shown part's own interior the diagram recurses**. +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 @@ -266,17 +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, `CollectParts` keeps every part, and `unlimitedRecursion` is always `true` — -completely unchanged from the pre-scoping behavior. - -Once a root is selected and its direct parts are narrowed, `unlimitedRecursion` decides whether the -diagram continues to recurse into each shown part's own composed interior at all: when the resolved -scope contains at least one subject with unlimited-depth recursion (`expose X::**;` / -`expose X::*::**;`), or there is no scope at all, every container part fully expands its own nested -parts, unbounded in depth — identical to today's behavior before this depth-limiting was -introduced. When the resolved scope is purely non-recursive (only `expose X;` and/or -`expose X::*;` forms), expansion stops after the root's own direct part children: a deeper container -part still renders as its own box, but with no nested children of its own drawn inside it. +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 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 6b61503d..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 @@ -167,10 +167,10 @@ sections: - id: SysML2Tools-Core-Layout-Internal-InterconnectionViewLayoutStrategy-ExposeScopingRecursionDepthLimit title: >- - When the resolved expose scope has no subject with unlimited-depth recursion (i.e. every - subject is MembershipExact and/or NamespaceDirectChildren), InterconnectionViewLayoutStrategy - shall limit interior expansion to the selected root's (or top-level scoped feature's) own - direct part children, rendering any deeper container part as an intrinsic-sized leaf box + 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 @@ -178,20 +178,45 @@ sections: 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 requirement is deliberately implemented as a single - diagram-wide decision (`HasUnlimitedRecursion`, computed once from the resolved scope's - `Subjects`), not a per-subject or per-composition-branch decision: a scope that combines a - recursive subject with a non-recursive subject uses unlimited recursion for the whole - diagram, which is a known, documented, and sanctioned simplification (see the design - documentation's "Depth-Limited Recursion" section). A scope with at least one recursive - subject, or no scope at all (including the `--auto` view), continues to recurse fully at - every depth, unchanged. + 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 @@ -334,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 1521dde6..68772cb1 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -247,15 +247,25 @@ 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 the resolved `expose` scope is recursive. A scope -containing at least one recursive form (`expose X::**;` or `expose X::*::**;`), or no `expose` -statement at all, recurses fully — every nested container's own interior is shown, at any depth. -A scope containing **only** non-recursive forms (`expose X;` and/or `expose X::*;`, with no -recursive form present anywhere in the view) limits expansion to the selected root's (or -top-level scoped feature's) own direct part children: a deeper nested part still renders as its -own box, but its own interior is not drawn. 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. +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 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 48e6ee76..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 @@ -51,6 +51,24 @@ recursion is unchanged, a regression guard proving the depth-limiting gate never 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 Tests run via `dotnet test` against net8.0, net9.0, and net10.0. No external services, files, or @@ -131,6 +149,15 @@ configuration are required beyond a standard .NET SDK installation. - 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 @@ -168,3 +195,5 @@ configuration are required beyond a standard .NET SDK installation. | `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 | From 42c9c086fb78f49f99adecde9a2cb8c6b3d5aba7 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Thu, 16 Jul 2026 17:50:13 -0400 Subject: [PATCH 7/8] docs: correct Dependencies bullet to reflect per-branch recursion gate Update the InterconnectionViewLayoutStrategy design doc's Dependencies section to name MatchesUnlimitedSubject and IsMoreSpecificCandidate, and clarify that MatchesUnlimitedSubject (via DetermineBranchUnlimitedRecursion) is now the primary per-branch depth-limiting gate, with HasUnlimitedRecursion retained only as the depth-0 no-qualified-name fallback. Addresses a low-severity documentation staleness finding from formal review. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../internal/interconnection-view-layout-strategy.md | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) 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 9bbabc34..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 @@ -385,9 +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`; `ExposedScope.Subjects`' recursion kinds also drive - `HasUnlimitedRecursion`'s depth-limiting decision. + `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`). From c352b5cd26c461c887d736200985ffa25729e070 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Fri, 17 Jul 2026 00:02:00 -0400 Subject: [PATCH 8/8] build: update DemaConsulting.Rendering to 0.1.0-beta.14 Picks up the graduated-falloff fix for ContainmentLayoutAlgorithm's column-count-estimate ratio guard (Rendering PR #26), which previously used a hard MaxColumnEstimateSizeRatio = 2.0 cutoff that collapsed realistic label-driven box sets into a single degenerate column whenever width variance barely exceeded 2x (observed with VersionMarkSystemView: 9 boxes, widths 168-351px, ratio 2.08). Verified: full suite still green (515 core tests x3 TFMs + 355 tool tests), and VersionMarkSystemView now packs into a balanced 2-3 box per row grid instead of a single tall column. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../DemaConsulting.SysML2Tools.Core.csproj | 2 +- .../DemaConsulting.SysML2Tools.Tool.csproj | 4 ++-- .../DemaConsulting.SysML2Tools.Tests.csproj | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) 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.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 @@ - - + +