Implement depth-limited recursion in InterconnectionView and update tests#43
Merged
Merged
Conversation
…pose 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>
- 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>
- 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>
…ection 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>
…ion 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>
…ction 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>
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>
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request updates the documentation for the
InterconnectionViewLayoutStrategyto clarify and document the new per-branch depth-limited recursion logic for diagram layout, replacing the previous global recursion flag approach. It introduces new methods and intermediate data structures, and explains how recursion depth is now determined independently for each composition branch based on the resolved expose scope. This improves correctness when multiple expose statements with different recursion kinds are present in a view.The most important changes are:
Per-Branch Depth-Limited Recursion
DetermineBranchUnlimitedRecursionandMatchesUnlimitedSubjectmethods, which decide recursion depth independently for each branch based on the expose scope and the matched subject's recursion kind. This replaces the previous global boolean approach, resolving known limitations when mixing recursive and non-recursive subjects in the same scope. [1] [2]unlimitedRecursionflag is true, and this decision is made once per branch at depth 0 and propagated to all descendants. [1] [2]Data Structures and Connection Resolution
TopLevelPartrecord to pair each top-level fallback part with its owning definition's qualified name, enabling correct scoping for connection endpoint resolution in the no-single-root fallback path. [1] [2]Scoping and Filtering Logic
These changes ensure that the layout strategy correctly and independently applies recursion depth limits per exposed subject, improving the accuracy of rendered diagrams when complex expose scopes are used.