Fix recursion semantics, update layout strategy, and bump version#36
Merged
Conversation
….1.0-beta.7 - Add ExposeRecursionKind (MembershipExact, MembershipRecursive, NamespaceDirectChildren, NamespaceRecursive) to ExposeMember, correctly derived from the ::** grammar token for MembershipExpose and NamespaceExpose forms in AstBuilder (previously discarded/miscalculated). - Redesign ExposeScopeResolver subject matching to honor exact-match, direct-children-only, and prefix/recursive semantics per recursion kind instead of always treating expose targets as fully recursive subtrees. - Update GeneralViewLayoutStrategy and DynamicViewSynthesizer consumers, and ~44 test call sites, plus new ExposeScopeResolverTests coverage. - Update design/reqstream/verification docs and user guide to reflect the corrected behavior. - Bump DemaConsulting.Rendering.Layout/Svg/Skia to 0.1.0-beta.7 and pin SkiaSharp.NativeAssets to 4.150.0 to resolve a transitive version floor. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…beta.7 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…folder suppression in GeneralViewLayoutStrategy
- Defect A: definitions owning nested definitions (e.g. part def X { part def Y; }) no longer
render as a duplicate sibling folder; they now nest correctly inside the parent definition's
own box via a new recursive PlaceDef helper (childrenByParent/defByQualified partitioning,
containerScopes renamed from folderScopes).
- Defect B: when an expose scope is active, ancestor bare-package folders no longer wrap
exposed content purely because they are qualified-name ancestors; content is promoted
directly to root instead. Unscoped behavior is fully preserved (pinned 21-box drone
regression test unaffected).
- Added 5 new unit tests covering both defects in scoped/unscoped permutations.
- Updated companion reqstream requirements, design doc, and verification doc for the unit.
- Regenerated full gallery (svg+png) for all models; only BatterySubsystemView and the 5
mission-control views changed as expected. Updated gallery README sections 1b and 10 to
accurately describe the corrected behavior.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…siveView section RadioNetwork is a bare package, not a definition, so it is never itself rendered as a box - only its Uplink/Downlink members are in scope and render as flat top-level boxes. The prior text incorrectly implied RadioNetwork appears as an element in the diagram. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request refines SysML v2 expose scoping by modeling recursion explicitly (exact vs direct-children vs whole-subtree), fixes General View layout so nested definitions are not rendered twice, and updates tests/docs/gallery artifacts to match the new semantics while bumping rendering-related package versions.
Changes:
- Introduce
ExposeRecursionKindonExposeMemberand carry it through AST building and scope resolution (ExposeSubject+ recursion-aware matching). - Update General View layout graph construction to treat nested definitions as true containers (preventing duplicate rendering) and suppress bare package folders when scoped.
- Refresh unit tests, documentation, gallery SVG outputs/models, and bump rendering/SkiaSharp package versions.
Reviewed changes
Copilot reviewed 37 out of 62 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| test/DemaConsulting.SysML2Tools.Tests/Semantic/WorkspaceLoaderTests.cs | Adds assertion that bracket-filtered expose members capture recursion kind. |
| test/DemaConsulting.SysML2Tools.Tests/Semantic/AstBuilderMetadataTests.cs | Adds multiple tests verifying recursion-kind capture for all expose grammar forms. |
| test/DemaConsulting.SysML2Tools.Tests/Rendering/DynamicViewSynthesizerTests.cs | Asserts synthesized views use recursive expose scoping. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/StateTransitionViewLayoutStrategyTests.cs | Updates test fixtures to include ExposeRecursionKind in ExposeMember. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/SequenceViewLayoutStrategyTests.cs | Updates test fixtures to include ExposeRecursionKind in ExposeMember. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/InterconnectionViewLayoutStrategyTests.cs | Updates test fixtures to include ExposeRecursionKind in ExposeMember. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/GeneralViewLayoutStrategyTests.cs | Adds regression tests for nested-definition containment + scoped bare-package folder suppression. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs | Updates and expands tests to validate recursion-aware scoping behavior and fallback rules. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/BrowserAndGridViewLayoutStrategyTests.cs | Updates test fixtures to include ExposeRecursionKind in ExposeMember. |
| test/DemaConsulting.SysML2Tools.Tests/Layout/ActionFlowViewLayoutStrategyTests.cs | Updates test fixtures to include ExposeRecursionKind in ExposeMember. |
| test/DemaConsulting.SysML2Tools.Tests/DemaConsulting.SysML2Tools.Tests.csproj | Bumps rendering + SkiaSharp native asset package versions for tests. |
| src/DemaConsulting.SysML2Tools.Tool/DemaConsulting.SysML2Tools.Tool.csproj | Bumps rendering package versions used by the CLI tool. |
| src/DemaConsulting.SysML2Tools.Language/Semantic/Model/SysmlNode.cs | Extends ExposeMember with RecursionKind and introduces ExposeRecursionKind. |
| src/DemaConsulting.SysML2Tools.Language/Semantic/Model/AstBuilder.cs | Derives recursion kind from grammar form/recursion tokens and records it per expose member. |
| src/DemaConsulting.SysML2Tools.Core/Rendering/Internal/DynamicViewSynthesizer.cs | Synthesizes views with recursive expose semantics for dynamic/ad-hoc view rendering. |
| src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs | Prevents duplicate rendering of nested definitions; suppresses bare package folders when scoped. |
| src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs | Reworks scope model to recursion-aware subject matching + improved fallback behavior. |
| src/DemaConsulting.SysML2Tools.Core/DemaConsulting.SysML2Tools.Core.csproj | Bumps DemaConsulting.Rendering.Layout dependency version. |
| README.md | Updates CLI/scoping documentation to describe recursion-aware expose semantics. |
| docs/verification/sysml2-tools-language/semantic/model/sysml-node.md | Updates verification narrative + adds tests mapping for recursion kind capture. |
| docs/verification/sysml2-tools-language/semantic/model/ast-builder.md | Updates verification narrative + adds tests mapping for recursion kind derivation. |
| docs/verification/sysml2-tools-core/layout/internal/general-view-layout-strategy.md | Updates verification assertions and adds regression coverage notes for Defect A/B. |
| docs/verification/sysml2-tools-core/layout/internal/expose-scope-resolver.md | Updates verification narrative for Subjects model + recursion semantics. |
| docs/user_guide/introduction.md | Expands user guide with formal expose recursion semantics and revised worked examples. |
| docs/reqstream/sysml2-tools-language/semantic/model/sysml-node.yaml | Updates requirement text to include recursion-kind classification. |
| docs/reqstream/sysml2-tools-language/semantic/model/ast-builder.yaml | Adds requirement for correct ExposeRecursionKind derivation + associated tests. |
| docs/reqstream/sysml2-tools-core/rendering/internal/dynamic-view-synthesizer.yaml | Updates requirement to specify recursive scoping for synthesized views. |
| docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml | Adds requirements for nested-definition containment + scoped folder suppression. |
| docs/reqstream/sysml2-tools-core/layout/internal/expose-scope-resolver.yaml | Updates/extends requirements for recursion-aware subject matching and fallback semantics. |
| docs/gallery/svg/WorkstationInterconnectionView.svg | Regenerated gallery output reflecting updated layout/renderer behavior. |
| docs/gallery/svg/OperatorConsoleExactView.svg | New gallery SVG illustrating MembershipExact behavior. |
| docs/gallery/svg/OperatorConsoleDeepView.svg | New gallery SVG illustrating recursive membership expose behavior. |
| docs/gallery/svg/MissionControlGeneralView.svg | New gallery SVG used to demonstrate nested-definition rendering fixes. |
| docs/gallery/svg/GroundSegmentRecursiveView.svg | New gallery SVG illustrating recursive namespace expose behavior. |
| docs/gallery/svg/GroundSegmentDirectChildrenView.svg | New gallery SVG illustrating namespace direct-children scoping behavior. |
| docs/gallery/svg/DroneGeneralView.svg | Regenerated gallery output reflecting renderer/layout changes. |
| docs/gallery/svg/CoreLinkInterconnectionView.svg | Regenerated gallery output reflecting renderer/layout changes. |
| docs/gallery/svg/ComputerSystemInterconnectionView.svg | Regenerated gallery output reflecting renderer/layout changes. |
| docs/gallery/svg/BatterySubsystemView.svg | Regenerated gallery output confirming scoped bare-package folder suppression. |
| docs/gallery/README.md | Updates gallery narrative to describe new scoping/recursion behaviors and new examples. |
| docs/gallery/models/10-mission-control-expose-recursion.sysml | Adds a dedicated model demonstrating all four expose recursion forms. |
| docs/design/sysml2-tools-language/semantic/model/sysml-node.md | Updates design docs for ExposeMember shape and recursion-kind semantics. |
| docs/design/sysml2-tools-language/semantic/model/ast-builder.md | Updates design docs describing recursion-kind extraction/derivation logic. |
| docs/design/sysml2-tools-core/rendering/internal/dynamic-view-synthesizer.md | Updates design docs for synthesized view scoping behavior. |
| docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md | Updates design docs for container nesting and scoped folder suppression behavior. |
| docs/design/sysml2-tools-core/layout/internal/expose-scope-resolver.md | Updates design docs for recursion-aware subject matching and fallback. |
| docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md | Updates terminology from PrefixSubjects to Subjects in the filtering design narrative. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
- ExposeScopeResolver.ResolveExposedScope: clarify that the usage-typing
expansion applies to every Subjects entry regardless of recursion kind
(not just non-exact-only subjects), and never to ExplicitMembers.
- DynamicViewSynthesizer: correct example syntax from 'view def V { expose
Target::**; }' to 'view V { expose Target::**; }' - expose is valid in a
named view usage body, not a view def.
- GeneralViewLayoutStrategy.BuildGraph: correct the edge-scoping comment -
it's an exact container-scope-key match, not a genuine
lowest-common-ancestor search.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ive subject inclusion Two follow-up fixes from PR #36 review comments: 1. ExposeScopeResolver.ResolveExposedScope previously re-associated each resolved Expose edge with its originating ExposeMember via a fragile forward-scan + EndsWith("::" + raw) string match. When an earlier expose entry failed to resolve but its raw text happened to be a suffix of a later entry's resolved target, the wrong ExposeMember (and therefore the wrong RecursionKind/BracketFilterExpressionText) could be paired with a resolved edge, corrupting the resolved scope. Fixed by having ReferenceResolver iterate SysmlViewNode.ExposeMembers directly and populate a new SysmlViewNode.ResolvedExposeMembers list that pairs each successfully-resolved ExposeMember with its resolved qualified name unambiguously, by object identity, at resolution time. ExposeScopeResolver now consumes this list directly instead of re-deriving the pairing, and the old memberIndex/scan-and-match loop is removed entirely. The existing SysmlEdgeKind.Expose edges are still populated in parallel, unchanged, for their existing consumers (impact analysis / query command reverse-lookup index). 2. IsInSubjectScope shared one switch arm between MembershipRecursive and NamespaceRecursive, both including "qualifiedName == subject.QualifiedName". Per formal-26-03-02.md Section 8.3.26.4, a NamespaceExpose only ever exposes a namespace's Memberships (its members) -- the namespace is never a member of itself, regardless of the recursive flag. NamespaceRecursive now has its own arm matching only descendants at any depth, never the subject itself. This did not previously manifest in the gallery because every current NamespaceRecursive usage targets a bare package, which never gets its own rendered box; confirmed unchanged gallery output after regenerating all SVG/PNG files. Added regression tests: one reproducing the exact reviewer-flagged re-pairing scenario (unresolved "expose A;" followed by resolved "expose X::A;" with different recursion kinds), and one confirming NamespaceRecursive excludes a SysmlDefinitionNode subject itself while including all of its descendants, contrasted with MembershipRecursive on the same fixture. Updated docs/user_guide/introduction.md to correct the NamespaceRecursive description (previously claimed the same whole-subtree behavior as MembershipRecursive, including the subject itself, which was wrong). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
DynamicViewSynthesizer.Synthesize (the render --view-type/--view-target ad-hoc view feature) constructed a SysmlViewNode using only the old ExposeMembers/ResolvedEdges pair. Since the prior round's fix moved ExposeScopeResolver.ResolveExposedScope to consume the new SysmlViewNode.ResolvedExposeMembers field instead, dynamic views were silently falling back to rendering the entire unscoped workspace instead of narrowing to the requested target - a regression. Fix populates ResolvedExposeMembers alongside the existing fields, and adds a regression test that exercises ExposeScopeResolver.ResolveExposedScope end-to-end against a synthesized dynamic view to catch this class of bug in future. Also corrects two companion-artifact inconsistencies surfaced by the same review round: - docs/reqstream expose-scope-resolver.yaml referenced a renamed/nonexistent test, failing CI's reqstream satisfaction check. Split the requirement into a MembershipRecursive one and a new NamespaceRecursive-specific one referencing the correct, currently-existing tests. - docs/verification expose-scope-resolver.md conflated MembershipRecursive and NamespaceRecursive behavior in a bullet and was missing a test-scenario entry. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ursion RenderOptions.DepthLimit previously only capped package-folder-contents nesting in GeneralViewLayoutStrategy.BuildGraph (truncateFolderContents); nested-definition containment introduced by PlaceDef's own recursion had no depth limiting at all, so a definition nested inside another definition could render to unlimited depth regardless of the requested DepthLimit. PlaceDef now threads a depth counter (starting at 1, matching the existing folder-contents-is-depth-1 convention) plus depthLimit and a shared truncated-container list through its recursion. When the next nesting level would exceed depthLimit, PlaceDef stops recursing into that definition's nested children and instead places a single '+N more...' ellipsis placeholder leaf inside the definition's own body, mirroring the existing package-folder truncation UX. TruncatedFolder is generalized to TruncatedContainer (adding a TitleOffset field so a folder's own title band and a nested definition's already-reserved title band are decorated correctly), and DecorateTruncatedFolders is generalized into a recursive DecorateTruncated that walks every nested container scope to find and decorate a truncated placeholder at any nesting depth, not just at the root. Added regression tests proving: a DepthLimit that previously had no effect on nested-definition containment now truncates it correctly; DepthLimit == 0 still renders full nested-definition depth unchanged; and the pre-existing package-folder-contents truncation path is unaffected. Updated the GeneralViewLayoutStrategy design doc to describe the new depth-threading scheme and the generalized TruncatedContainer/ DecorateTruncated mechanism. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Path.GetTempFileName() creates a real temp file on disk; appending '.sysml' to build the actual test path left the original extensionless file behind as an orphan, since only the extended path was deleted in the finally block. Replaced all 10 occurrences with Path.Combine(Path.GetTempPath(), Path.GetRandomFileName() + '.sysml'), matching the existing convention used elsewhere in the test suite (GetRandomFileName does not create a file on disk). Also corrected an adjacent doc-comment wording inaccuracy found while in this file: a test summary claimed NamespaceRecursive expose includes the subject itself in scope, contradicting the already-fixed actual semantics (NamespaceRecursive excludes the subject). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Markdown code spans cannot contain a line break; the inline-code list of expose grammar forms (�xpose X;/�xpose X::**;/�xpose X::*;/ �xpose X::*::**;) was split mid-span across two lines, which breaks code-span parsing and renders incorrectly. Rewrapped so each backtick pair stays on a single line. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
ExposeMember is a public record; adding RecursionKind as a required third positional parameter removed the previous two-argument constructor, a source-breaking change for any external consumer manually constructing an ExposeMember. Defaulted RecursionKind to ExposeRecursionKind.MembershipRecursive, matching the pre-existing whole-subtree scoping behavior that applied unconditionally before this parameter existed, so the old two-argument call site keeps compiling and behaving identically. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
The GitHub Autofix suggestion applied for the usage-expose-scoping example (accepting that it correctly identified the resolved Vehicle type row also being present in the tree, not just myVehicle) dropped 'non-recursive.' mid-sentence when merging two lines, and produced a 199-char line that failed markdownlint MD013. Restored the missing sentence-ending and rewrapped the paragraph under 120 chars, while keeping the corrected content (myVehicle row and Vehicle row are both in scope; only descendant-expansion is suppressed by exact-match scoping). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…n test helper - GeneralViewLayoutStrategy.DecorateTruncated (recursive worker) previously matched placed LayoutBox entries against a scope's LayoutGraphNode list using a single shared loop index. Since a scope's placed node list can intersperse LayoutLine entries (routed intra-scope edges) among boxes at arbitrary positions, any line encountered before a box permanently shifted the box-to-graph-node pairing, causing truncation/hidden-count decoration to land on the wrong box (or be skipped). Fixed by tracking a separate graphIndex cursor that only advances on actual LayoutBox encounters. - Added a reflection-based regression test invoking the private worker directly with a fixture that places a LayoutLine before two LayoutBox entries, proving the fix (confirmed it fails against the pre-fix code). - TestSysmlViewNodeExtensions.WithResolvedExposeMembers() replaced its silently-truncating Zip() with a fail-fast count validation, since a mismatched ExposeMembers/resolved-Expose-edge count in a test fixture should surface as a clear failure rather than a silently truncated pairing. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Malcolmnixon
added a commit
that referenced
this pull request
Jul 15, 2026
* Fix recursive wildcard import resolution (import X::*::**) Recursive wildcard imports parsed without error but never actually resolved names in namespaces nested more than one level under the imported target, because: - AstBuilder.VisitImportRule discarded the parsed IsRecursive flag - SysmlImportNode had no field to carry recursion state - ReferenceResolver.TryResolve's wildcard-import step only ever checked one namespace level deep This is a legitimate, spec-sanctioned construct present in the OMG conformance corpus (SimpleTests/ImportTest.sysml), but the corpus sweep only checks for parse errors, never resolution correctness, so the gap went uncaught. Fixes: - SysmlImportNode: added IsRecursive property - AstBuilder.VisitImportRule: now passes through the parsed recursion flag instead of discarding it - ReferenceResolver: added FindRecursiveWildcardMatch, which searches the symbol table for a same-named member in any namespace nested at any depth under the imported target when IsRecursive is true, preferring the shallowest match on ambiguity Added 3 regression tests to WorkspaceLoaderTests covering resolution via recursive wildcard import, resolution via nested-level names, and the non-recursive control case (must NOT reach nested members). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix recursive membership import resolution (import X::Y::**) A recursive membership import (import X::Y::**;) is a distinct grammar shape from the recursive namespace-import form (import X::*::**;) fixed in the previous commit: here Y is the explicit membership-import target itself, not a containing namespace being wildcard-searched. It must bring Y into scope by its own short name, in addition to reaching Y's own nested descendants. Auditing the codebase for other places sharing this bug class found that expose's equivalent path (ExposeScopeResolver) already handles both recursive forms correctly with existing tests (PR #36); this gap was isolated to plain import statements' ReferenceResolver. Fixes: - SysmlImportNode: added IsMembershipImport property, set from AstBuilder.VisitImportRule based on which grammar alternative (namespaceImport vs membershipImport) was actually parsed - ReferenceResolver: added a check so a recursive membership import also resolves an unqualified reference to the import target itself (not just its descendants), guarded so it does not fire for the namespace-import recursive form Added a regression test to WorkspaceLoaderTests covering resolution of both the membership-import target's own name and its nested descendants via a single recursive membership import. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix depth tie-break in recursive wildcard import resolution Code review of the prior two commits found FindRecursiveWildcardMatch picked the 'least-nested' match among ambiguous recursive-import candidates by comparing raw qualified-name string length, not actual namespace depth. Since segment names vary in length, a deeper match can have a shorter qualified-name string than a shallower one, silently violating the documented 'prefer the shallowest match' invariant. Fixed to count actual '::'-separated segment depth between the resolved namespace and the matched name, with a length guard to avoid an invalid slice range on the exact one-level match (where the prefix and suffix overlap). Added a regression test with two same-named members at different depths, using deliberately long package names at the shallow depth and short ones at the deep depth, proving the shallower match wins even though its qualified-name string is longer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --------- Co-authored-by: Malcolm Nixon <Malcolm.Nixon@hiarc.inc> 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
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 introduces significant improvements to how view scoping and diagram rendering work, especially for the handling of
exposestatements and containment in SysML v2 diagrams. The changes clarify and expand the scoping model, add support for more granular recursion settings, and fix a long-standing issue where nested definitions could be rendered twice in the same diagram. The documentation is updated to reflect these changes, making the scoping logic and rendering pipeline easier to understand and maintain.Expose scoping and recursion improvements:
exposestatement now supports multiple recursion kinds (exact,direct children, orwhole subtree), derived from the SysML v2 grammar form and recursion setting (e.g.,expose X;,expose X::*;,expose X::**;), instead of always scoping to the full subtree. [1] [2]ExposeSubjectpaired with anExposeRecursionKind, allowing precise matching of candidates based on the specific recursion semantics of eachexposeentry. [1] [2] [3]Containment and rendering fixes:
Documentation and terminology:
These changes together make view scoping more powerful and predictable, and fix a key rendering bug for nested definitions.