diff --git a/.cspell.yaml b/.cspell.yaml index 8a0ac10..e2613f4 100644 --- a/.cspell.yaml +++ b/.cspell.yaml @@ -35,6 +35,8 @@ words: - Hanan - KEBNF - MBSE + - metaclass + - metaclasses - xunit - nameof - pagetitle diff --git a/ROADMAP.md b/ROADMAP.md index a052047..30b1408 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -259,33 +259,117 @@ the previous whole-subtree behavior for that entry, with `LayoutWarnings.ForUnev now warning only on that failure (mirroring `ForUnevaluatedFilter`'s existing failure-only pattern) instead of unconditionally warning whenever any bracket filter was merely present. -**Phase 2b — deferred (zero corpus evidence):** the Phase 1-excluded construct list — -`istype`/`hastype`/`all`, arithmetic operators, conditional (`if`/`else`) expressions, and general -feature-chain navigation (attribute/feature reads not anchored by an `(as Type)` cast). Each -currently produces a clear, non-crashing "unsupported filter construct" diagnostic rather than -silently doing nothing. Across all 251 OMG corpus files sampled during Phase 2 planning, every -real `filter`/bracket-form-`expose` expression already fell within the Phase 1/2a supported -subset — there is no observed real-world need to implement these constructs yet, so they remain -deferred until a concrete corpus example demonstrates a need. - -**Phase 2c — deferred (no current consumer):** metadata annotations on **usages** (as opposed to -definitions) are captured in the semantic model (`SysmlMetadataNode` is attached wherever -`metadataFeature` appears), but filter/bracket-filter narrowing only evaluates classification -tests/attribute reads against `SysmlDefinitionNode` candidates (matching -`CollectDefinitions`'s/`ExposeScopeResolver`'s existing definition-only restriction) — extending -evaluation to usage-level candidates is future work. Today, only `GeneralViewLayoutStrategy` uses -definition-scoped filter candidates at all; no other view kind or consumer currently needs -usage-level candidate filtering, so there is no concrete driver to implement it yet. - -**Scope:** `SysmlNode.cs`/`AstBuilder.cs`/`ReferenceResolver.cs`/`SysmlEdge.cs` (metadata capture, -paired `ExposeMember` model); `DemaConsulting.SysML2Tools.Core.Filtering` (Phase 1 subsystem, -reused unchanged for Phase 2a); `ExposeScopeResolver` (Phase 2a bracket-filter evaluation, shared -by all 7 layout strategies); `GeneralViewLayoutStrategy`/`LayoutWarnings` (filter application, -failure-only bracket-filter warning). -**Visual gate:** a view with a standalone `filter @Type;`-style Phase 1 statement, or a bracketed -`expose ::**[]` Phase 2a statement, renders only the elements satisfying the -predicate, with no "unevaluated"/"not yet evaluated" warning for that statement; an unsupported -construct still falls back to the resolved scope with an explicit diagnostic. +**Phase 2b — narrowed (bare `istype`/`hastype` confirmed not applicable):** the grammar +(`SysMLv2Parser.g4`'s `ownedExpression`) only defines `istype`/`hastype` as **binary** postfix +operators requiring an explicit left operand (`ownedExpression (ISTYPE|HASTYPE) typeReference`, +e.g. the corpus's `engine istype '6CylEngine'`) — there is no bare/self-referential form, unlike +`@Type`'s genuine prefix production (`(AT_SIGN|AT_AT) typeReference`). A `filter`/bracket-filter +predicate has no bound variable to serve as that left operand, so `istype`/`hastype` are not +expressible in filter-expression position at all under the actual SysML v2 grammar — confirmed +by grep across the full 251-file OMG corpus finding zero `filter`/`expose[...]` usages of either +keyword. This part of Phase 2b remains correctly deferred (not a gap — there is nothing to +implement). Arithmetic operators, conditional (`if`/`else`) expressions, and general feature-chain +navigation remain deferred unchanged (zero corpus evidence in filter position). + +**Phase 2d — delivered.** Both coupled fixes landed: + +1. **Metaclass/kind classification-test matching.** `FilterExpressionEvaluator` now matches a + classification test (`@Type`/`@Pkg::Type`) when *either* the existing applied-annotation match + (`FindMetadata`) succeeds *or* the candidate's own AST node kind — + `SysmlDefinitionNode.DefinitionKeyword`/`SysmlFeatureNode.FeatureKeyword` — maps to the + requested metaclass, via a new `MetaclassNames` keyword→bare-metaclass-name table covering + every keyword this project's `AstBuilder` models that has a corresponding stdlib + `metadata def` declaration (part/attribute/item/port/action/state/requirement/constraint/ + interface, and their `def`-suffixed definition counterparts). The bare `allocation` feature + keyword (a connection-usage, mapping to the stdlib's `AllocationUsage` metaclass) is likewise + NOT in this table — it, together with `connection`/`binding`/`message`, is captured by the + dedicated `SysmlConnectionNode` type rather than `SysmlFeatureNode`. `view`/`viewpoint` + keywords are NOT in this table either — they are captured by dedicated `SysmlViewNode`/ + `SysmlViewpointNode` node types rather than `SysmlDefinitionNode`/`SysmlFeatureNode`, so all of + these remain out of scope by construction (see known gaps below), even though their stdlib + metaclasses (`AllocationDefinition`/`AllocationUsage`/`ViewDefinition`/`ViewpointDefinition` + etc.) do exist. Both bare (`@PartUsage`) and `SysML::`-qualified spellings match — including + both the corpus's conventional two-segment form (`@SysML::PartUsage`) and the stdlib's actual, + deeper declaring package path (`@SysML::Systems::PartUsage`) — via a `StartsWith("SysML::")` + + `EndsWith("::" + name)` suffix match, so a filter written against either spelling matches the + same metaclass. The + specialization-conformance stretch goal (Investigation §2) was also delivered: a filter naming + an ancestor metaclass (e.g. `@ConstraintUsage`) also matches a more specific stdlib-derived + kind (e.g. a `requirement` usage, via `RequirementUsage specializes ConstraintUsage`) — walked + via each stdlib metaclass declaration's raw, unresolved `SupertypeNames` text (not the + resolved `Supertype` edge originally envisioned, since stdlib-only nodes are never passed + through `ReferenceResolver.ResolveAll` and so never have resolved edges), resolved to a + declaring stdlib node by a same-simple-name suffix lookup in `SysmlWorkspace.Declarations`, and + cycle-guarded with a visited-name set. This is a deliberately narrow heuristic, not a general + reference-resolution mechanism, and is documented as such in code. + + **Known, documented gaps** (a modeled keyword with no stdlib metaclass counterpart, so no entry + exists in `MetaclassNames` for it): `individual def`; raw KerML classifier keywords + (`datatype`/`class`/`struct`/`assoc`/`assoc struct`/`function`/`predicate`); + `subject`/`actor`/`stakeholder`; bare `enum value` members; control-node keywords + (`merge`/`decide`/`join`/`fork`); `assume constraint`/`require constraint` (deliberately not + folded into the generic `ConstraintUsage`, to avoid over-claiming semantics not evidenced in the + stdlib); `entry`/`do`/`exit` (deliberately not mapped to `ActionUsage`). Also out of scope by + construction, since these use dedicated node types rather than + `SysmlDefinitionNode`/`SysmlFeatureNode`: `SysmlConnectionNode` (`connection`/`allocation`/ + `binding`/`message`) and `SysmlViewNode`/`SysmlViewpointNode` (`view def`/`view`/ + `viewpoint def`/`viewpoint`). + +2. **Usage-level filter candidates.** `GeneralViewLayoutStrategy.CollectDefinitions` — which + doubles as both the filter-candidate source and the box-rendering function for the entire + General View diagram — now admits named `SysmlFeatureNode` usages alongside + `SysmlDefinitionNode`s (`BuildCompartments`/`CollectMemberships` were generalized from + `SysmlDefinitionNode` to the common `SysmlNode` base type to support this, mechanically, with + no logic change). `ExposeScopeResolver`'s bracket-filter candidate query was widened + identically. This unconditional admission **was** a real, unguarded regression in its initial + form: rendering every admitted usage as a standalone box, regardless of nesting depth, more + than doubled the box count of real corpus models (e.g. the gallery's + `01-drone-general.sysml`'s `DroneGeneralView`, 21 → 47 `` boxes) by duplicating nested + usages already shown as compartment rows inside their owning definition/usage's box — found by + quality re-validation and fixed in Retry 1 (see below). The OMG Safety feature-views fixture's + exposed-vehicle-subtree scope, previously documented as intentionally empty, now renders the + vehicle's part usages (still excluding `Safety`/`Security`). + + **Retry 1 fix (quality regression).** Added `GeneralViewLayoutStrategy.RemoveRedundantNestedUsages`, + called in `BuildLayout` after the standalone `filter [];` narrowing and before + `GroupByPackage`: it excludes a usage-level box from standalone rendering when its immediate + parent's qualified name is also present in the final, fully scope-and-filter-narrowed set (i.e., + the usage would duplicate content already shown in the parent's compartment row). + Definitions are never excluded by this rule. Re-verified `docs/gallery/models/01-drone-general.sysml`'s + `DroneGeneralView` renders exactly 21 boxes again, matching the checked-in + `docs/gallery/svg/DroneGeneralView.svg` (now a standing automated regression-guard test), while + the `filter @SysML::PartUsage;`/bare `@PartUsage` metaclass-kind filter tests and the OMG Safety + fixture test continue to pass unchanged. + + **Retry 2 fix (quality regression).** A second quality re-validation found that Retry 1's + single-pass, snapshot-based dedup silently dropped a usage nested **two or more levels** deep + (e.g. `part def A { part b { part c; } }`): `c`'s immediate parent `b` was tested against the + pre-removal snapshot rather than the actually-surviving set, so `c` was excluded alongside `b` + even though `b` no longer rendered anywhere and `BuildCompartments` never shows a grandchild's + row — `c` vanished from the diagram entirely, not shown as its own box nor inside any surviving + compartment. `RemoveRedundantNestedUsages` was reworked to process usage-level boxes in + ascending qualified-name-depth order (fewest `::` occurrences first) while incrementally + building the `excluded` set, so a usage's immediate-parent presence test reflects whether the + parent itself survives dedup rather than merely whether it appeared in the pre-dedup snapshot. + This cascades correctly through any nesting depth in a single ordered pass: `b` is excluded + before `c` is tested, so `c`'s immediate parent is no longer "present" by the time `c` is + considered, and `c` correctly survives as its own standalone box. Added a new 3-level regression + test (`GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersDeeplyNestedGrandchildUsageWhenIntermediateParentExcluded`) + asserting exactly 2 rendered boxes (`A` and `c`, with `b` excluded and never silently lost). + Re-confirmed the existing 21-box gallery regression guard, all three Retry 1 tests, and the + primary `@SysML::PartUsage`/`@Safety`/OMG Safety fixture tests still pass unchanged. + +**Scope:** `DemaConsulting.SysML2Tools.Core.Filtering.FilterExpressionEvaluator` (new +`MetaclassNames`/`MatchesMetaclassKind`/`MetaclassNameMatches`/`ConformsToMetaclass`, workspace +threaded through the private evaluation recursion); `GeneralViewLayoutStrategy.CollectDefinitions`/ +`BuildCompartments`/`CollectMemberships`; `ExposeScopeResolver.ResolveExposedScope`'s bracket-filter +candidate query. +**Visual gate — met.** `filter @SysML::PartUsage;` (or bare `@PartUsage`) against a model with +mixed part/requirement/other usages renders only the `PartUsage` elements — matching the OMG's own +`42.Views/ViewsExample.sysml` corpus pattern (regression-tested in +`GeneralViewLayoutStrategyTests`). Existing definition-level domain-metadata filtering +(`filter @Safety;`, `filter @Safety and (as Safety).isMandatory;`) continues to work identically +(unaffected by the new OR-combined metaclass-kind match path). --- diff --git a/docs/design/sysml2-tools-core/filtering.md b/docs/design/sysml2-tools-core/filtering.md index c8e8130..322d4d7 100644 --- a/docs/design/sysml2-tools-core/filtering.md +++ b/docs/design/sysml2-tools-core/filtering.md @@ -74,6 +74,12 @@ flowchart TD resolved `MetadataType` edge points at the requested type, when that target ends with `"::Type"` for a bare-name filter, or — if the metadata type never resolved — when the raw `TypeReference` text itself matches, allowing graceful fallback for otherwise-usable models. + (Phase 2d) A classification test **also** matches when the candidate's own AST node kind (its + `DefinitionKeyword`/`FeatureKeyword`) maps to the requested built-in SysML metaclass name, via a + keyword-to-metaclass lookup table plus a bounded stdlib `specializes`-chain walk — see + `docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md` for the full mapping + table and matching semantics. This is an additional match path, evaluated alongside (not instead + of) the applied-annotation match above. 6. Bare attribute reads are boolean predicates: they succeed only when the addressed metadata attribute exists and its captured literal kind is Boolean with value `true`. Equality and inequality comparisons support Boolean, Number, and String literals. A missing annotation or a @@ -87,7 +93,11 @@ flowchart TD `GeneralViewLayoutStrategy.CollectDefinitions`'s existing restriction), calls `Evaluate` with that candidate set, and adds the matched subset to the resolved scope's `ExplicitMembers`. No change was required in this subsystem to support the second caller, confirming the evaluator's - candidate-set-agnostic design. + candidate-set-agnostic design. (Phase 2d) Both `ExposeScopeResolver`'s bracket-filter candidate + set and `GeneralViewLayoutStrategy.CollectDefinitions`'s filter-candidate/render set were + widened to admit named `SysmlFeatureNode` (usage-level) candidates in addition to + `SysmlDefinitionNode`s, since a metaclass-kind classification test (e.g. + `filter @SysML::PartUsage;`) is inherently about usages, not definitions. ### Design Constraints @@ -112,3 +122,4 @@ flowchart TD | --- | --- | | SysML2Tools-Core-Filtering-StandaloneViewFilterEvaluation | `Parse`, `Evaluate`, and `FilterExpression.ToString()` | | SysML2Tools-Core-Filtering-BracketFormExposeEvaluation | `ExposeScopeResolver` reusing `Parse`/`Evaluate` | +| SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests | `MatchesMetaclassKind` | diff --git a/docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md b/docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md index db5cd23..78bdc7a 100644 --- a/docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md +++ b/docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md @@ -95,6 +95,68 @@ Reads the attribute value and compares it against the literal using `ValuesEqual or absent attributes evaluate conservatively as false regardless of operator; there is no implicit three-valued logic or default-value synthesis in Phase 1. +##### Metaclass/kind classification-test matching (Phase 2d) + +A classification test (`@Type`/`@Pkg::Type`) matches when *either* `FindMetadata` succeeds +(unchanged) *or* `MatchesMetaclassKind(workspace, node, typeName)` succeeds — the two paths are +evaluated independently and combined with a boolean OR, since both are legitimate under the OMG +`@` classification-test semantics (metaclass membership *or* explicit domain metadata). + +`MatchesMetaclassKind` maps the candidate's own AST node kind — `SysmlDefinitionNode.DefinitionKeyword` +or `SysmlFeatureNode.FeatureKeyword` — to a built-in SysML metaclass's bare (simple) name via the +static `MetaclassNames` lookup table, then checks whether the requested `typeName` matches that +bare name directly, or matches the canonical `SysML::`-qualified spelling (`"SysML::" + name`) — +the spelling real-world corpus filters use (e.g. `filter @SysML::PartUsage;`), which does *not* +literally match the stdlib's actual nested declaration path (`SysML::Systems::PartUsage`); see +"Why bare metaclass names, not the literal stdlib path" below. + +`MetaclassNames` covers every definition/feature keyword this project's `AstBuilder` assigns that +has a corresponding stdlib `metadata def` declaration in `SysML.sysml`, verified by grepping every +`metadata def \w+Usage|\w+Definition` declaration in the stdlib and cross-checking each keyword. +Documented known gaps (keywords with no stdlib metaclass, or deliberately not guessed): `individual +def`; raw KerML classifier keywords (`datatype`/`class`/`struct`/`assoc`/`assoc struct`/`function`/ +`predicate`); `subject`/`actor`/`stakeholder`; bare `enum value` members; control-node keywords +(`merge`/`decide`/`join`/`fork`); `assume constraint`/`require constraint` (not merged into the +generic `ConstraintUsage`, to avoid over-claiming semantics not evidenced in the stdlib); `entry`/ +`do`/`exit` (not mapped to `ActionUsage`, since these are deliberately non-behavioral minimal +captures). Also out of scope by construction: `SysmlConnectionNode` (`connection`/`allocation`/ +`binding`/`message`) and `SysmlViewNode`/`SysmlViewpointNode` (`view def`/`view`/`viewpoint def`/ +`viewpoint`) — these use dedicated node types, not `SysmlDefinitionNode`/`SysmlFeatureNode`. + +**Specialization-conformance walk.** When the candidate's own mapped metaclass does not match +`typeName` exactly, `ConformsToMetaclass` walks the stdlib's `specializes` chain looking for a +matching ancestor metaclass — e.g. a `requirement` usage's mapped metaclass (`RequirementUsage`) +also matches `@ConstraintUsage`, since `RequirementUsage specializes ConstraintUsage` in the +stdlib. This walk is cycle-guarded (a visited-set keyed on bare metaclass name), mirroring +`ReferenceResolver.FindMemberInTypeHierarchy`'s existing guard pattern. + +**Why bare metaclass names, not the literal stdlib path, and why raw `SupertypeNames`, not +resolved `Supertype` edges.** Two investigation findings shaped this design, both empirically +confirmed against the compiled stdlib rather than assumed: + +1. Stdlib `metadata def` metaclass declarations are nested inside `package Systems` within + `package SysML` (`SysML.sysml`'s `standard library package SysML { ... package Systems { metadata + def PartUsage ... } }`), so their actual registered `SysmlWorkspace.Declarations` qualified name + is `SysML::Systems::PartUsage`, not the two-segment `SysML::PartUsage` real-world filter + expressions and this project's own `ROADMAP.md` write. `MetaclassNames`'s values are therefore + bare simple names (`"PartUsage"`), and matching compares the requested `typeName` against that + bare name or the *canonical* `"SysML::" + name` spelling directly — never against the literal, + longer stdlib declaration path — so `@SysML::PartUsage` matches regardless of the stdlib's + actual internal package nesting. +2. `SysmlNode.ResolvedEdges` (including `SysmlEdgeKind.Supertype`) is **never populated for + stdlib-only nodes** — `ReferenceResolver.ResolveAll` only runs over user-file AST roots (see + `SysmlNode.ResolvedEdges`'s own remarks) — so a specialization walk cannot reuse resolved + `Supertype` edges the way ordinary user-model lookups do. `ConformsToMetaclass` instead reads + each stdlib metaclass declaration's raw, unresolved `SupertypeNames` text (populated + unconditionally by `AstBuilder` during parsing, before any resolution pass runs) and resolves + each simple supertype name to its declaring stdlib node by a same-simple-name suffix lookup in + `SysmlWorkspace.Declarations`. This is a deliberately narrow, bounded heuristic — not a general + reference-resolution mechanism — that assumes the stdlib's metaclass simple names are unique + enough for a suffix match to be unambiguous, which holds for the metaclass names this table + covers. This design was reached by first attempting the resolved-edge approach and confirming + empirically (via a probe against `StdlibProvider.GetSymbolTable()`) that it does not work for + stdlib nodes, rather than by assumption. + #### Error Handling `FilterExpressionParser` never throws for unsupported constructs or malformed syntax. Syntax errors @@ -198,3 +260,6 @@ lexer/recursive-descent-parser internals could violate that contract despite `Pa - `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-RoundTripPrettyPrinting` — `FilterExpression.ToString()` overrides, `FilterExpressionParser.Parse`, and `FilterExpressionParser.TryBuildLiteral`'s `double.IsFinite` check (numeric-literal-overflow guard) +- `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests` — + `FilterExpressionEvaluator.MatchesMetaclassKind`, `MetaclassNames`, `MetaclassNameMatches`, + `ConformsToMetaclass` (specialization-conformance walk) 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 ab768fb..50990fe 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 @@ -18,10 +18,11 @@ parameters, and (Phase 2a) resolution output is returned as two internal record the resolved scope. `PrefixSubjects` are exposed subject qualified names whose entire containment subtree is in scope (the pre-Phase-2a whole-subtree behavior, used for `expose` entries with no bracket filter and as the fallback for a bracket-filtered entry that failed to - parse or evaluate). `ExplicitMembers` are individual definition qualified names matched by a - successfully-evaluated bracket filter — exact matches only, not their own nested members unless - those also match. A settable `Failures` init-property (default empty) carries any bracket - filters that failed to parse or evaluate. + parse or evaluate). `ExplicitMembers` are individual qualified names — of a `SysmlDefinitionNode` + **or a named `SysmlFeatureNode`** (Phase 2d — widened from definitions-only, see below) — + matched by a successfully-evaluated bracket filter — exact matches only, not their own nested + members unless those also match. A settable `Failures` init-property (default empty) carries any + bracket filters that failed to parse or evaluate. - `BracketFilterFailure(string ExpressionText, string? Reason)` — one failed bracket-filter expression's raw source text plus a short human-readable reason, feeding `LayoutWarnings.ForUnevaluatedExposeBracketFilter`. @@ -46,9 +47,11 @@ the edge's target with the `ExposeMember` it originated from (see Design below) - when that entry's bracket-filter expression parses and evaluates successfully (Phase 2a), computes the candidate set as every `workspace.Declarations` key that is the target itself or lies in its containment subtree (`qn == target || qn.StartsWith(target + "::")`) *and* is a - `SysmlDefinitionNode` — mirroring `GeneralViewLayoutStrategy.CollectDefinitions`'s existing - restriction — evaluates with `FilterExpressionEvaluator.Evaluate` against that candidate set - unchanged, and adds the matched subset to `ExplicitMembers`; + `SysmlDefinitionNode` **or named `SysmlFeatureNode`** (Phase 2d — widened from + definitions-only so a metaclass-kind classification test like `@SysML::PartUsage` can match a + usage-level candidate too) — mirroring `GeneralViewLayoutStrategy.CollectDefinitions`'s + candidate-set restriction — evaluates with `FilterExpressionEvaluator.Evaluate` against that + candidate set unchanged, and adds the matched subset to `ExplicitMembers`; - when that entry's bracket-filter expression fails to parse or evaluate, falls back to whole-subtree inclusion (`PrefixSubjects`, same as the unfiltered case) and records a `BracketFilterFailure` with the raw expression text and a short reason, so the caller can @@ -61,14 +64,14 @@ scoping to nothing, whole-subtree inclusion also resolves the usage's own `Typin `ResolvedEdges` entry (if any) and adds that type's qualified name to `PrefixSubjects` too, so both the usage and its type's subtree are included. This expansion only applies to whole-subtree inclusion — a successfully-evaluated bracket filter's `ExplicitMembers` already name the exact -matched definitions directly. +matched definitions/usages directly. ###### `IsInSubjectScope(string qualifiedName, ExposedScope scope)` Returns `true` when `qualifiedName` equals one of `scope.PrefixSubjects` or lies within one of their containment subtrees (a `"{subject}::"` prefix match, the same qualified-name-prefix idiom `StdlibFilter.IsStdlibElement` already uses for stdlib-prefix matching), or is an exact match of -one of `scope.ExplicitMembers` (a bracket-filter-matched definition). Used by every strategy to +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. ###### `IsRootRelevantToScope(string candidateQualifiedName, ExposedScope scope)` diff --git a/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md b/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md index a7a939c..846c450 100644 --- a/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md +++ b/docs/design/sysml2-tools-core/layout/internal/general-view-layout-strategy.md @@ -3,8 +3,10 @@ ##### Purpose `GeneralViewLayoutStrategy` implements `ILayoutStrategy` to produce a General View diagram. It -renders every user-defined definition (part, port, interface, requirement, action, and so on) as -a keyword-labeled box, groups the boxes that belong to a package inside a folder-shaped +renders every user-defined definition (part, port, interface, requirement, action, and so on) and +— since Phase 2d (see `ROADMAP.md`'s "View `filter [];` expression evaluation" section) — +every named usage-level declaration too, as a keyword-labeled box, groups the boxes that belong to +a package inside a folder-shaped container, lists each definition's owned usages in compartments, and draws specialization, membership, attribute-typing, redefinition, subsetting, connect, allocate, dependency, and binding edges orthogonally between the boxes. The whole diagram — package @@ -35,12 +37,14 @@ carries the short kind label, raw source/target reference, and human-readable re rendered boxes, feeding `LayoutWarnings.ForDroppedRelationshipEdges`. `FeatureMembership` (a private record) carries each owned feature's keyword, raw type reference (`TypeName`, nullable — a feature may declare a redefinition with no explicit type annotation), simple `Name`, raw -`RedefinedFeatureName` reference, the raw `SubsettedFeatureNames` list (populated verbatim -from `SysmlFeatureNode.SupertypeNames` — a feature's `subsets`/`:>` targets, not a new AST field), -and `ExpressionText` (verbatim from `SysmlFeatureNode.ExpressionText`, non-null only for -constraint-kind features such as `"require constraint"`/`"assume constraint"`/`"constraint"`); -`CollectMemberships` includes a feature when `TypeName`, `RedefinedFeatureName`, -`ExpressionText`, or a non-empty `SubsettedFeatureNames` is present. The private `EdgeKind` +`RedefinedFeatureName` reference, and the raw `SubsettedFeatureNames` list (populated verbatim +from `SysmlFeatureNode.SupertypeNames` — a feature's `subsets`/`:>` targets, not a new AST field). +`FeatureMembership` does not itself carry an +`ExpressionText` field — constraint-kind features' expression text (non-null only for +`"require constraint"`/`"assume constraint"`/`"constraint"` keywords) is read directly from the +owning `SysmlFeatureNode.ExpressionText` where needed (`BuildCompartments`), not copied into +`FeatureMembership`. `CollectMemberships` includes a feature when `TypeName`, +`RedefinedFeatureName`, or a non-empty `SubsettedFeatureNames` is present. The private `EdgeKind` enumeration classifies each edge as `Specialization`, `Membership`, `Typing`, `Redefinition`, `Subsetting`, `Connect`, `Allocate`, `Dependency`, or `Binding`; `Subsetting` is a purely view-layer classification — it does not @@ -66,7 +70,13 @@ carries standalone `FilterExpressionText`, the method next parses it with `defs` to the matched subset and returning the same minimal empty canvas when that subset is empty. A parse failure or unsupported Phase 1 construct does not abort layout: the first parser diagnostic message is remembered as the warning reason and the method continues rendering the -unfiltered resolved scope. The remaining pipeline is unchanged: definitions are grouped by package +unfiltered resolved scope. Once scope and standalone-filter narrowing are both applied, the method +calls `RemoveRedundantNestedUsages(defs)` (see below), which drops any usage-level box whose +nearest still-rendered ancestor is also present in the final `defs` set — this must run after both +narrowing steps, not before or inside `CollectDefinitions`, so a usage whose parent was excluded by +a filter (rather than genuinely absent from scope) survives as its own standalone box. The method returns +the same minimal empty canvas if the dedup empties `defs`. The remaining pipeline is unchanged: +definitions are grouped by package with `GroupByPackage`, the specialization/membership/attribute-typing/redefinition/subsetting/ connect/allocate/dependency/binding relationships are resolved into qualified-name edges (plus any dropped-edge diagnostics) with `BuildModelEdges`, the @@ -89,12 +99,26 @@ the `LayoutTree with { Warnings = … }` record-copy idiom. ###### `CollectDefinitions(workspace, theme, scope)` -Iterates `workspace.Declarations`, keeping each `SysmlDefinitionNode` that is not a +Iterates `workspace.Declarations`, keeping each `SysmlDefinitionNode` **or named** +`SysmlFeatureNode` (Phase 2d — a metaclass-kind classification test like +`filter @SysML::PartUsage;` is inherently about usages, not definitions, and this method doubles +as both the filter-candidate source and the box-rendering function for the whole diagram, so +usages must be admitted here for such a filter to have anything meaningful to narrow; an unnamed +feature is excluded, since it has no stable qualified name to key a box on) that is not a standard-library element (per `StdlibFilter.IsStdlibElement`) and, when `scope` is non-null, is -within `scope` per `ExposeScopeResolver.IsInSubjectScope`. For each kept definition it builds the +within `scope` per `ExposeScopeResolver.IsInSubjectScope`. This admission is unconditional of +nesting depth — it is deliberately a superset used as the filter-candidate pool, including usages +nested inside an already-admitted definition/usage. `BuildLayout` narrows this superset down to +the final rendered set in two later steps: the standalone `filter [];` narrowing, then +`RemoveRedundantNestedUsages` (Retry 1 fix, cascading depth-ordered pass hardened in Retry 2 — +see below), which removes exactly the nested usages +that would duplicate an already-rendered immediate parent's compartment row. For each kept +declaration it builds the compartments from the owned usage features (grouped by keyword, each formatted via `FormatFeatureRow`), collects the typed memberships, and computes the box size from the title -and the longest compartment row. +and the longest compartment row. `BuildCompartments`/`CollectMemberships` take the common +`SysmlNode` base type (not `SysmlDefinitionNode`) precisely so a usage-level candidate's owned +features are collected identically to a definition's. Each compartment's title comes from `Pluralize(keyword)`, which special-cases a small set of individually-significant single-member keywords to a guillemet-wrapped stereotype form instead of @@ -112,6 +136,52 @@ features only) as its raw expression text alone (unnamed) or as `"{name}: {expr} instead of the generic `"name : Type [multiplicity]"` shape used by every other feature kind, since a constraint's defining content is its expression, not a type/multiplicity pair. +###### `RemoveRedundantNestedUsages(defs)` (Retry 1 fix; cascading depth-ordered pass hardened in Retry 2) + +Removes usage-level (`DefBox.IsUsage`) boxes from `defs` whose nearest still-rendered ancestor is +also present in `defs` — i.e., a usage nested (directly, or transitively through one or more other +excluded usages) inside a box that already shows it as a compartment row. Since every node's +qualified name is a strict `parent::child` containment chain, every ancestor is strictly shallower +(fewer `::`-separated segments) than its descendants, and a usage's immediate-parent qualified name +is recovered by stripping the qualified name's last `::`-separated segment (a top-level usage, with +no `::` in its qualified name, is never excluded). Boxes are processed in ascending depth order +(fewest `::` occurrences first) while incrementally building an `excluded` set, so that by the time +a usage is tested, every one of its ancestors has already been finally decided. A usage is excluded +only when its immediate parent is present in `defs` **and has not itself already been excluded** +earlier in this same pass — i.e., only when the immediate parent is itself still rendered, and +therefore already shows the usage as one of the parent's compartment rows (`BuildCompartments` +renders a definition/usage's direct `SysmlNode.Children` only — it never reaches into +grandchildren). Conversely, when the immediate parent has itself just been excluded in this same +pass, it no longer renders anywhere, so its own compartment can no longer show this usage — the +usage is therefore not excluded, and survives as its own standalone box. This cascades correctly +through any nesting depth: two, three, or more levels of usage-in-usage nesting all resolve +correctly in this single ordered pass, so a deeper-nested usage is never silently dropped merely +because an intermediate ancestor between it and the nearest rendered box was itself excluded. +Definitions (`DefBox.IsUsage` is `false`) are never excluded by this rule, preserving all +pre-Phase-2d definition-rendering behavior. + +This exists because `CollectDefinitions`'s usage-level widening (Phase 2d) admits every named +usage regardless of nesting depth — necessary so a metaclass-kind filter like +`filter @SysML::PartUsage;` has a complete candidate pool to narrow — but that superset, if +rendered unfiltered, duplicates every nested usage already shown as a compartment row inside its +owning definition's box (empirically, 21 → 47 rendered boxes for +`docs/gallery/models/01-drone-general.sysml`'s `DroneGeneralView`, a real regression found by +quality re-validation). The initial Retry 1 fix computed the "which parents are present" set once +from the pre-dedup `defs` list and tested every usage against that single fixed snapshot; a +subsequent quality re-validation found this silently dropped a usage nested **two or more levels** +deep (e.g. `part def A { part b { part c; } }`), since `c`'s immediate parent `b` was present in +the pre-removal snapshot even though `b` was itself simultaneously being excluded — `c` vanished +from the diagram entirely, shown neither as its own box nor inside any surviving compartment. The +current depth-ordered, incrementally-shrinking-`excluded`-set pass (Retry 2) fixes this: `b` is +excluded before `c` is considered, so `c`'s immediate parent is no longer treated as present by the +time `c` is tested, and `c` correctly survives as its own standalone box. `BuildLayout` calls this +method **after** the standalone `filter [];` narrowing and **before** `GroupByPackage`, so +the dedup sees only the truly final, fully scope-and-filter-narrowed set: a nested usage whose +immediate parent was excluded by scope, by that filter, or by this same dedup pass (rather than +genuinely still present and rendered) is correctly kept as its own standalone box, since it either +duplicates a still-rendered ancestor's compartment row (and is correctly excluded) or is never +shown anywhere else and must render standalone (and is therefore never silently lost). + ###### `GroupByPackage(defs)` Groups definitions by the qualified-name prefix before the last `::`, preserving first-seen order. diff --git a/docs/reqstream/sysml2-tools-core/filtering.yaml b/docs/reqstream/sysml2-tools-core/filtering.yaml index a2bbc55..871b611 100644 --- a/docs/reqstream/sysml2-tools-core/filtering.yaml +++ b/docs/reqstream/sysml2-tools-core/filtering.yaml @@ -29,11 +29,14 @@ sections: - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-AttributeReads - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-UnsupportedConstructDiagnostics - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-RoundTripPrettyPrinting + - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests tests: - DiagramRenderer_RenderWorkspace_SafetyPartsView_FiltersToAnnotatedParts - DiagramRenderer_RenderWorkspace_MandatorySafetyPartsView_FiltersToMandatoryPart - GeneralViewLayoutStrategy_BuildLayout_FilterExpressionMatchesNothing_RendersEmpty - GeneralViewLayoutStrategy_BuildLayout_FilterExpressionPresent_EmitsNotYetEvaluatedWarning + - GeneralViewLayoutStrategy_BuildLayout_QualifiedPartUsageFilter_RendersOnlyPartUsages + - GeneralViewLayoutStrategy_BuildLayout_BarePartUsageFilter_RendersOnlyPartUsages - id: SysML2Tools-Core-Filtering-BracketFormExposeEvaluation title: >- @@ -53,7 +56,9 @@ sections: - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-BooleanConnectives - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-AttributeReads - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-UnsupportedConstructDiagnostics + - SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests tests: - ResolveExposedScope_BracketFilterEvaluatesSuccessfully_NarrowsToMatchedDefinitionsOnly - ResolveExposedScope_MixedFilteredAndUnfilteredEntries_NarrowsIndependently - ResolveExposedScope_BracketFilterFailsToParse_FallsBackToWholeSubtreeAndRecordsFailure + - ResolveExposedScope_BracketFilterMetaclassKind_MatchesUsageLevelCandidate diff --git a/docs/reqstream/sysml2-tools-core/filtering/filter-expression-evaluator.yaml b/docs/reqstream/sysml2-tools-core/filtering/filter-expression-evaluator.yaml index f1f0eac..fce3f62 100644 --- a/docs/reqstream/sysml2-tools-core/filtering/filter-expression-evaluator.yaml +++ b/docs/reqstream/sysml2-tools-core/filtering/filter-expression-evaluator.yaml @@ -124,3 +124,30 @@ sections: - Parse_RoundTrip_PrettyPrintedTextReparsesToEquivalentTree - Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity - Parse_LargeButFiniteRealLiteral_StillParsesSuccessfully + + - id: SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests + title: >- + FilterExpressionEvaluator shall also match a classification test (`@Type`/`@Pkg::Type`) + when the candidate's own AST node kind (its definition or feature keyword) maps to the + requested built-in SysML metaclass, in both bare (`@PartUsage`) and `SysML::`-qualified + (`@SysML::PartUsage`) spelling, honoring the stdlib's `specializes` chain so a filter + naming an ancestor metaclass also matches a more specific descendant kind — without + altering existing applied-annotation classification-test matching, which remains an + independent, OR-combined match path. + justification: | + Real-world SysML v2 view filters (e.g. OMG's `42.Views/ViewsExample.sysml`, + `filter @SysML::PartUsage;`) classify candidates by their own metaclass/kind, not by an + explicitly applied domain-metadata annotation — Phase 1's applied-annotation-only match + could never satisfy this common pattern, silently rendering such views empty instead of + narrowed. Supporting the bare and `SysML::`-qualified spellings together matches both + conventions seen in corpus filters. The specialization-conformance walk keeps a filter + naming a general metaclass (e.g. `@ConstraintUsage`) meaningful against more specific + stdlib-derived kinds (e.g. a `requirement` usage), mirroring how classification tests + behave against a real metaclass hierarchy rather than only exact-kind matching. + tests: + - Evaluate_BareMetaclassKind_MatchesUsage + - Evaluate_QualifiedMetaclassKind_MatchesUsage + - Evaluate_MetaclassKind_MatchesDefinition + - Evaluate_MetaclassKind_NonMatchingMetaclass_DoesNotMatch + - Evaluate_ClassificationTest_AppliedAnnotationMatchingUnaffectedByMetaclassKindAddition + - Evaluate_MetaclassKind_SpecializationConformance_MatchesAncestorMetaclass 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 5c85772..1853b60 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 @@ -103,3 +103,17 @@ sections: - IsMoreSpecificCandidate_EqualLength_FallsBackToScore_False - IsMoreSpecificCandidate_SameDepthSiblingsDifferentLength_ShorterWithBetterScoreWins - IsMoreSpecificCandidate_SameDepthSiblingsDifferentLength_ShorterWithWorseScoreLoses + + - id: SysML2Tools-Core-Layout-Internal-ExposeScopeResolver-BracketFilterUsageLevelCandidates + title: >- + When a bracket-filter expression's candidate set is computed (Phase 2a), it shall + include named usage-level declarations (feature usages), not only definitions, so a + metaclass-kind classification filter (e.g. `[@SysML::PartUsage]`) can match a usage-level + candidate within an exposed path's containment subtree. + justification: | + `ResolveExposedScope`'s bracket-filter candidate set mirrors + `GeneralViewLayoutStrategy.CollectDefinitions`'s own candidate set (Phase 2d), which was + widened to admit usages for the same reason: a metaclass-kind filter is inherently about + usages, not just definitions, and a definitions-only candidate set could never match one. + tests: + - ResolveExposedScope_BracketFilterMetaclassKind_MatchesUsageLevelCandidate diff --git a/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml b/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml index 45d9c7b..7527fcb 100644 --- a/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml +++ b/docs/reqstream/sysml2-tools-core/layout/internal/general-view-layout-strategy.yaml @@ -322,6 +322,39 @@ sections: tests: - GeneralViewLayoutStrategy_BuildLayout_FilterExpressionPresent_EmitsNotYetEvaluatedWarning + - id: SysML2Tools-Core-Layout-Internal-GeneralViewLayoutStrategy-UsageLevelCandidates + title: >- + GeneralViewLayoutStrategy shall admit named usage-level declarations (feature usages), + not only definitions, as filter/render candidates, so a metaclass-kind classification + filter (e.g. `filter @SysML::PartUsage;`) that targets usages has matching candidates to + narrow, and shall render such a usage-level candidate as a standalone box only when it is + not already shown as a compartment row of an independently-rendered immediate parent. + justification: | + `CollectDefinitions` doubles as both the filter-candidate source and the box-rendering + function for the entire General View diagram. Real-world corpus filters (e.g. OMG's + `42.Views/ViewsExample.sysml`, `filter @SysML::PartUsage;`) classify by usage-level + metaclass/kind, which a definitions-only candidate set could never satisfy — such a view + would render empty rather than narrowed. Widening the candidate set to include named + usages is a deliberate, planned behavior change, but rendering every admitted usage + unconditionally (Phase 2d, before Retry 1) was itself a genuine regression: a usage + nested inside an already-rendered definition/usage duplicates the compartment row it + already occupies there, more than doubling the box count of real corpus models (e.g. the + gallery's `01-drone-general.sysml`, 21 → 47 boxes). `RemoveRedundantNestedUsages` (Retry + 1, hardened to a depth-ordered cascading pass in Retry 2 to avoid silently dropping + usages nested two or more levels deep) restores the pre-Phase-2d default-rendering box + count by excluding exactly the nested usages whose nearest still-rendered ancestor is + also independently rendered, while still admitting usages that survive scope/filter + narrowing when their parent does not. + tests: + - GeneralViewLayoutStrategy_BuildLayout_QualifiedPartUsageFilter_RendersOnlyPartUsages + - GeneralViewLayoutStrategy_BuildLayout_BarePartUsageFilter_RendersOnlyPartUsages + - GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersUsageLevelCandidatesToo + - GeneralViewLayoutStrategy_BuildLayout_OmgSafetyFeatureViewsFixture_ScopesToExposedVehicleSubtree + - GeneralViewLayoutStrategy_BuildLayout_NoFilter_ExcludesUsageNestedInsideRenderedDefinition + - GeneralViewLayoutStrategy_BuildLayout_MetaclassFilter_KeepsNestedUsageWhenParentExcluded + - GeneralViewLayoutStrategy_BuildLayout_DroneGalleryModel_RendersExactly21BoxesMatchingCheckedInSvg + - GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersDeeplyNestedGrandchildUsageWhenIntermediateParentExcluded + - id: SysML2Tools-Core-Layout-Internal-GeneralViewLayoutStrategy-DroppedRelationshipEdgeWarning title: >- When a Connect/Allocate/Dependency/Binding edge's endpoint fails to resolve to a diff --git a/docs/verification/sysml2-tools-core/filtering/filter-expression-evaluator.md b/docs/verification/sysml2-tools-core/filtering/filter-expression-evaluator.md index 302d5b1..9e64854 100644 --- a/docs/verification/sysml2-tools-core/filtering/filter-expression-evaluator.md +++ b/docs/verification/sysml2-tools-core/filtering/filter-expression-evaluator.md @@ -35,6 +35,13 @@ SDK and the repository's committed SysML fixtures. instead of silently discarding the trailing tokens. - A numeric literal that overflows `double` during parsing (producing a non-finite value) reports a diagnostic instead of silently round-tripping to unparsable `"Infinity"`/`"NaN"` text. +- A classification test (`@Type`/`@Pkg::Type`) matches a candidate whose own AST node kind + (`DefinitionKeyword`/`FeatureKeyword`) maps to the requested built-in SysML metaclass name, in + both bare (`@PartUsage`) and `SysML::`-qualified (`@SysML::PartUsage`) spelling, on both a usage + and a definition, without affecting existing applied-annotation classification-test matching. +- A metaclass-kind classification test for an unrelated metaclass does not match. +- A metaclass-kind classification test also matches via the stdlib's `specializes` chain (e.g. + `@ConstraintUsage` matches a `RequirementUsage`-kind candidate). #### Requirement-to-Test Mapping @@ -89,6 +96,13 @@ SDK and the repository's committed SysML fixtures. - `Parse_RoundTrip_PrettyPrintedTextReparsesToEquivalentTree` - `Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity` - `Parse_LargeButFiniteRealLiteral_StillParsesSuccessfully` +- `SysML2Tools-Core-Filtering-FilterExpressionEvaluator-MetaclassKindClassificationTests` + - `Evaluate_BareMetaclassKind_MatchesUsage` + - `Evaluate_QualifiedMetaclassKind_MatchesUsage` + - `Evaluate_MetaclassKind_MatchesDefinition` + - `Evaluate_MetaclassKind_NonMatchingMetaclass_DoesNotMatch` + - `Evaluate_ClassificationTest_AppliedAnnotationMatchingUnaffectedByMetaclassKindAddition` + - `Evaluate_MetaclassKind_SpecializationConformance_MatchesAncestorMetaclass` #### Test Scenarios @@ -123,3 +137,17 @@ SDK and the repository's committed SysML fixtures. - `Parse_NumericLiteralOverflow_ReturnsDiagnosticInsteadOfInfinity` — a numeric literal that overflows `double` (`3.14e400`) is reported as a diagnostic instead of silently becoming `Infinity` +- `Evaluate_BareMetaclassKind_MatchesUsage` — bare `@PartUsage` matches a `part`-keyword usage via + its own AST node kind, with no applied annotation present +- `Evaluate_QualifiedMetaclassKind_MatchesUsage` — `@SysML::PartUsage` matches identically to the + bare spelling +- `Evaluate_MetaclassKind_MatchesDefinition` — metaclass-kind matching also applies to + definition-level candidates, not just usages +- `Evaluate_MetaclassKind_NonMatchingMetaclass_DoesNotMatch` — a metaclass filter unrelated to the + candidate's kind does not match +- `Evaluate_ClassificationTest_AppliedAnnotationMatchingUnaffectedByMetaclassKindAddition` — + existing applied-annotation classification-test matching on a usage is unaffected by the new + metaclass-kind OR-path +- `Evaluate_MetaclassKind_SpecializationConformance_MatchesAncestorMetaclass` — `@ConstraintUsage` + matches a `requirement` usage via the stdlib's `RequirementUsage specializes ConstraintUsage` + chain 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 b854c01..921a40e 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 @@ -35,6 +35,9 @@ configuration are required beyond a standard .NET SDK installation. - (Phase 2a) A bracket-filter expression that fails to parse or evaluate degrades gracefully to whole-subtree inclusion for that entry (added to `PrefixSubjects`, same as the unfiltered case) and records a `BracketFilterFailure` (expression text plus a reason) in `ExposedScope.Failures`. +- (Phase 2d) A successfully-evaluated bracket-filter expression's candidate set includes named + usage-level (`SysmlFeatureNode`) declarations, not just `SysmlDefinitionNode`s, so a + metaclass-kind filter like `@SysML::PartUsage` can match a usage-level candidate. - `IsInSubjectScope` returns `true` for an exact qualified-name match against a `PrefixSubjects` entry. - `IsInSubjectScope` returns `true` for a qualified name nested under a `PrefixSubjects` entry (a @@ -86,6 +89,9 @@ configuration are required beyond a standard .NET SDK installation. - `ResolveExposedScope_BracketFilterFailsToParse_FallsBackToWholeSubtreeAndRecordsFailure`: A bracket filter that fails to parse falls back to whole-subtree inclusion in `PrefixSubjects` and records a `BracketFilterFailure` (expression text and reason) in `Failures` +- `ResolveExposedScope_BracketFilterMetaclassKind_MatchesUsageLevelCandidate`: + A successfully-evaluated bracket-filter metaclass-kind expression matches a named usage-level + candidate (`SysmlFeatureNode`), not just definitions - `IsInSubjectScope_ExactMatch_ReturnsTrue`: Exact qualified-name match against a `PrefixSubjects` entry is in scope - `IsInSubjectScope_SubtreeMatch_ReturnsTrue`: diff --git a/docs/verification/sysml2-tools-core/layout/internal/general-view-layout-strategy.md b/docs/verification/sysml2-tools-core/layout/internal/general-view-layout-strategy.md index 653f8a5..3f19f9d 100644 --- a/docs/verification/sysml2-tools-core/layout/internal/general-view-layout-strategy.md +++ b/docs/verification/sysml2-tools-core/layout/internal/general-view-layout-strategy.md @@ -119,6 +119,42 @@ configuration are required beyond a standard .NET SDK installation. - A constraint-kind feature (non-null `ExpressionText`) renders its raw expression text in place of the generic `name : Type [multiplicity]` row shape. - An `"enum value"`-keyword feature compartment is titled `"enum values"`. +- `CollectDefinitions` admits named usage-level (`SysmlFeatureNode`) candidates alongside + definitions (Phase 2d), so a bare or `SysML::`-qualified metaclass-kind filter + (`filter @PartUsage;`/`filter @SysML::PartUsage;`) renders only the matching usage-kind boxes, + reproducing the OMG `42.Views/ViewsExample.sysml` pattern, instead of an empty canvas. +- With no filter present at all, usage-level candidates render as boxes too, but only when they + are not already shown as a compartment row of an independently-rendered nearest ancestor — + `RemoveRedundantNestedUsages` (Retry 1 fix; hardened to a depth-ordered cascading pass in Retry 2) + excludes a nested usage from standalone rendering when its immediate parent is also present in + the final rendered set and has not itself already been excluded, restoring the pre-Phase-2d box + count for models with nested usages (e.g. the gallery's `01-drone-general.sysml`) while still + admitting genuinely top-level or filter-surviving usages as their own boxes, and — critically — + never silently dropping a usage nested two or more levels deep whose intermediate parent was + itself excluded (that usage instead correctly survives as its own standalone box). +- The OMG Safety feature-views fixture's exposed-vehicle-subtree scope (a whole-subtree `expose` + of the vehicle usage, no bracket filter) now renders a non-empty scoped diagram containing the + vehicle's part usages (e.g. `seatBelt`, `bumper`), where it previously rendered empty because + `CollectDefinitions` admitted only definitions, not usages — while still excluding + `Safety`/`Security` from the scoped result. These usages' immediate containing usages (`vehicle` + and its intermediate subassemblies) are never independently admitted into the exposed scope's + matched-member set, so `RemoveRedundantNestedUsages` does not remove them. +- A usage nested directly inside an independently-rendered definition/usage is excluded from + standalone box rendering in the default (unfiltered, unexposed) case — the direct regression + reproduction requested by quality re-validation (Retry 1). +- A nested usage whose immediate parent is excluded from the final rendered set by an active + metaclass filter (rather than by scope) still renders as its own standalone box, proving + `RemoveRedundantNestedUsages` runs after — not before — standalone filter narrowing (Retry 1). +- The real gallery corpus model `docs/gallery/models/01-drone-general.sysml`'s `DroneGeneralView` + renders exactly 21 boxes, matching the checked-in `docs/gallery/svg/DroneGeneralView.svg` — the + automated regression guard for the 21 → 47 box-count defect found by quality re-validation + (Retry 1). +- A usage nested two or more levels deep (e.g. `part def A { part b { part c; } }`) renders as its + own standalone box (`c`) when its intermediate parent (`b`) is itself excluded as a redundant + nested usage — the direct regression reproduction for the silent-data-loss defect found by + quality re-validation (Retry 2): exactly 2 boxes render (`A` and `c`), `b` never appears as its + own box, and `c` is never silently dropped merely because its immediate parent was itself + excluded in the same pass. ##### Test Scenarios @@ -235,3 +271,36 @@ configuration are required beyond a standard .NET SDK installation. compartment titles - `GeneralViewLayoutStrategy_BuildLayout_EnumDefLiteralValues_ProducesEnumValuesCompartment`: `"enum value"`-keyword features are grouped under an `"enum values"` compartment title +- `GeneralViewLayoutStrategy_BuildLayout_QualifiedPartUsageFilter_RendersOnlyPartUsages`: + A `filter @SysML::PartUsage;` expression renders only usage-level candidates whose own keyword + maps to `PartUsage`, reproducing the OMG `42.Views/ViewsExample.sysml` regression pattern against + a mixed part/requirement/other-usage workspace +- `GeneralViewLayoutStrategy_BuildLayout_BarePartUsageFilter_RendersOnlyPartUsages`: + The bare `filter @PartUsage;` spelling matches identically to the qualified form +- `GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersUsageLevelCandidatesToo`: + With no filter present, flat/top-level usage-level candidates render as boxes by default (Phase + 2d widening); this synthetic workspace has no nesting, so `RemoveRedundantNestedUsages` (Retry 1) + has nothing to exclude here +- `GeneralViewLayoutStrategy_BuildLayout_NoFilter_ExcludesUsageNestedInsideRenderedDefinition` + (Retry 1): a usage nested directly inside an independently-rendered definition is excluded from + standalone box rendering in the default (unfiltered) case — the direct regression reproduction + requested by quality re-validation +- `GeneralViewLayoutStrategy_BuildLayout_MetaclassFilter_KeepsNestedUsageWhenParentExcluded` + (Retry 1): a nested usage whose immediate parent is excluded from the final rendered set by an + active metaclass filter still renders as its own standalone box, proving the dedup step runs + after — not before — standalone filter narrowing +- `GeneralViewLayoutStrategy_BuildLayout_DroneGalleryModel_RendersExactly21BoxesMatchingCheckedInSvg` + (Retry 1): the real gallery corpus model `docs/gallery/models/01-drone-general.sysml`'s + `DroneGeneralView` renders exactly 21 boxes, matching the checked-in + `docs/gallery/svg/DroneGeneralView.svg` — the automated regression guard for the 21 → 47 + box-count defect +- `GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersDeeplyNestedGrandchildUsageWhenIntermediateParentExcluded` + (Retry 2): a 3-level nested workspace (`Root::A` definition, `Root::A::b` and `Root::A::b::c` + usages) renders exactly 2 boxes — `A` and `c` — with `b` correctly excluded as redundant but `c` + never silently dropped, proving the depth-ordered cascading dedup pass correctly treats `b` as + absent for `c`'s own test once `b` is itself excluded, instead of the prior single-pass, + pre-dedup-snapshot logic that silently lost `c` entirely +- `GeneralViewLayoutStrategy_BuildLayout_OmgSafetyFeatureViewsFixture_ScopesToExposedVehicleSubtree`: + The OMG Safety feature-views fixture's exposed-vehicle-subtree scope now renders a non-empty + scoped diagram containing the vehicle's part usages (`seatBelt`/`bumper`), while still excluding + `Safety`/`Security` diff --git a/src/DemaConsulting.SysML2Tools.Core/Filtering/FilterExpressionEvaluator.cs b/src/DemaConsulting.SysML2Tools.Core/Filtering/FilterExpressionEvaluator.cs index 8513131..931331c 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Filtering/FilterExpressionEvaluator.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Filtering/FilterExpressionEvaluator.cs @@ -44,9 +44,108 @@ public sealed record FilterEvaluationResult( /// , and any comparison against an absent read is treated as /// regardless of operator — a conservative, documented Phase 1 limitation /// (see docs/design/sysml2-tools-core/filtering/filter-expression-evaluator.md). +/// A classification test (@Type/@Pkg::Type) also matches when the candidate's own +/// AST node kind — (e.g. "part def") or +/// (e.g. "part") — maps to the requested +/// built-in SysML metaclass name (Phase 2d, see ROADMAP.md's "View +/// filter [<expr>]; expression evaluation" section), via the +/// keyword-to-metaclass-name lookup table. This is an +/// additional match path, evaluated alongside (not instead of) the existing applied- +/// annotation match above, since both are legitimate under the OMG @ classification-test +/// semantics (metaclass membership or explicit domain metadata). A keyword with no known +/// stdlib metaclass mapping (see 's remarks for the documented list of +/// known gaps) never matches via this path. When the candidate's own mapped metaclass does not +/// match the requested type name exactly, this also walks the mapped metaclass's stdlib +/// specializes chain (the raw, unresolved captured +/// for every stdlib metadata def declaration — not +/// , which is never populated for stdlib-only nodes; see that +/// property's remarks — resolved here by a same-simple-name lookup, cycle-guarded) looking for a +/// matching ancestor metaclass — e.g. a candidate whose own kind maps to RequirementUsage +/// also matches @ConstraintUsage, since RequirementUsage specializes ConstraintUsage +/// in the stdlib. /// public static class FilterExpressionEvaluator { + /// + /// Maps a or + /// to its corresponding built-in SysML metaclass's + /// bare (simple) name, used to match a classification test (@Type) against a candidate's + /// own AST node kind (Phase 2d). A candidate matches a classification test's type reference + /// when the reference equals this bare name, or equals "SysML::" + name — the canonical + /// spelling real-world filter/bracket-filter expressions use (e.g. + /// filter @SysML::PartUsage;), mirroring how the stdlib's actual nested declaration + /// package (SysML::Systems::PartUsage) is conventionally referred to by its outer + /// SysML:: namespace in corpus usage rather than its literal declaring package path. + /// Covers every definition/feature keyword this project's AstBuilder models that has a + /// corresponding stdlib metaclass declaration in SysML.sysml — verified by grepping + /// every metadata def \w+Usage|\w+Definition declaration in the stdlib and cross- + /// checking each keyword this project's AstBuilder assigns. + /// + /// + /// Known gaps (keywords this project models with no corresponding stdlib metaclass, or whose + /// mapping was deliberately not guessed — see ROADMAP.md Phase 2d for the full + /// rationale): individual def (no IndividualDefinition metaclass in stdlib); raw + /// KerML classifier keywords (datatype/class/struct/assoc/ + /// assoc struct/function/predicate, captured via BuildClassifierNode, + /// not a SysML *Definition metaclass); subject/actor/stakeholder (no + /// distinct stdlib metaclass); bare enum value members; control-node keywords + /// (merge/decide/join/fork, KerML control-node kinds not modeled as + /// metadata definitions in SysML.sysml); assume constraint/require constraint + /// (no distinct stdlib metaclass beyond the generic ConstraintUsage — deliberately not + /// merged in, to avoid over-claiming semantics not evidenced in the stdlib); entry/ + /// do/exit (deliberately not mapped to ActionUsage despite plausibility, + /// since these are captured as minimal, deliberately non-behavioral features, not full semantic + /// ActionUsage instances). Also out of scope by construction: + /// (connection/allocation/binding/message keywords — corresponding + /// stdlib metaclasses exist, but this node kind is not enumerated in Phase 2d's scope) and + /// / (view def/view/ + /// viewpoint def/viewpoint — these elements use dedicated node types, not + /// /). + /// + private static readonly IReadOnlyDictionary MetaclassNames = new Dictionary(StringComparer.Ordinal) + { + // Definition keywords (SysmlDefinitionNode.DefinitionKeyword). + ["part def"] = "PartDefinition", + ["attribute def"] = "AttributeDefinition", + ["item def"] = "ItemDefinition", + ["port def"] = "PortDefinition", + ["connection def"] = "ConnectionDefinition", + ["allocation def"] = "AllocationDefinition", + ["flow def"] = "FlowDefinition", + ["occurrence def"] = "OccurrenceDefinition", + ["rendering def"] = "RenderingDefinition", + ["metadata def"] = "MetadataDefinition", + ["enum def"] = "EnumerationDefinition", + ["interface def"] = "InterfaceDefinition", + ["action def"] = "ActionDefinition", + ["state def"] = "StateDefinition", + ["calc def"] = "CalculationDefinition", + ["constraint def"] = "ConstraintDefinition", + ["requirement def"] = "RequirementDefinition", + ["concern def"] = "ConcernDefinition", + ["case def"] = "CaseDefinition", + ["analysis def"] = "AnalysisCaseDefinition", + ["verification def"] = "VerificationCaseDefinition", + ["use case def"] = "UseCaseDefinition", + + // Feature (usage) keywords (SysmlFeatureNode.FeatureKeyword). + ["part"] = "PartUsage", + ["port"] = "PortUsage", + ["attribute"] = "AttributeUsage", + ["item"] = "ItemUsage", + ["ref"] = "ReferenceUsage", + ["enum"] = "EnumerationUsage", + ["occurrence"] = "OccurrenceUsage", + ["action"] = "ActionUsage", + ["accept"] = "AcceptActionUsage", + ["send"] = "SendActionUsage", + ["requirement"] = "RequirementUsage", + ["concern"] = "ConcernUsage", + ["constraint"] = "ConstraintUsage", + ["state"] = "StateUsage", + }; + + /// /// Evaluates against each candidate in /// , returning the subset that matches. @@ -68,7 +167,7 @@ public static FilterEvaluationResult Evaluate( foreach (var qualifiedName in candidateQualifiedNames) { if (workspace.Declarations.TryGetValue(qualifiedName, out var node) && - Evaluate(node, expression)) + Evaluate(workspace, node, expression)) { matched.Add(qualifiedName); } @@ -78,16 +177,18 @@ public static FilterEvaluationResult Evaluate( } /// Evaluates 's boolean value against a single candidate node. - private static bool Evaluate(SysmlNode node, FilterExpression expression) => + private static bool Evaluate(SysmlWorkspace workspace, SysmlNode node, FilterExpression expression) => expression switch { - ClassificationTestExpression classificationTest => FindMetadata(node, classificationTest.TypeName) is not null, - NotFilterExpression not => !Evaluate(node, not.Operand), + ClassificationTestExpression classificationTest => + FindMetadata(node, classificationTest.TypeName) is not null || + MatchesMetaclassKind(workspace, node, classificationTest.TypeName), + NotFilterExpression not => !Evaluate(workspace, node, not.Operand), BooleanFilterExpression boolean => boolean.Connective switch { - BooleanConnective.And => Evaluate(node, boolean.Left) && Evaluate(node, boolean.Right), - BooleanConnective.Or => Evaluate(node, boolean.Left) || Evaluate(node, boolean.Right), - BooleanConnective.Xor => Evaluate(node, boolean.Left) ^ Evaluate(node, boolean.Right), + BooleanConnective.And => Evaluate(workspace, node, boolean.Left) && Evaluate(workspace, node, boolean.Right), + BooleanConnective.Or => Evaluate(workspace, node, boolean.Left) || Evaluate(workspace, node, boolean.Right), + BooleanConnective.Xor => Evaluate(workspace, node, boolean.Left) ^ Evaluate(workspace, node, boolean.Right), _ => false, }, AttributeReadExpression attributeRead => ReadAttribute(node, attributeRead) is { Kind: MetadataAttributeValueKind.Boolean } value @@ -96,6 +197,148 @@ private static bool Evaluate(SysmlNode node, FilterExpression expression) => _ => false, }; + /// + /// Determines whether 's own AST node kind (its + /// or + /// ) maps — directly, or transitively via the + /// stdlib's specializes chain — to the requested built-in SysML metaclass name + /// (bare, e.g. PartUsage, or qualified, e.g. + /// SysML::PartUsage). See for the keyword mapping table + /// and its documented known gaps. + /// + private static bool MatchesMetaclassKind(SysmlWorkspace workspace, SysmlNode node, string typeName) + { + var keyword = node switch + { + SysmlDefinitionNode definition => definition.DefinitionKeyword, + SysmlFeatureNode feature => feature.FeatureKeyword, + _ => null, + }; + + if (string.IsNullOrEmpty(keyword) || !MetaclassNames.TryGetValue(keyword, out var bareMetaclassName)) + { + return false; + } + + return MetaclassNameMatches(bareMetaclassName, typeName) || + ConformsToMetaclass(workspace, bareMetaclassName, typeName, new HashSet(StringComparer.Ordinal)); + } + + /// + /// Returns when (e.g. + /// PartUsage) matches the requested : either bare + /// (typeName == bareMetaclassName) or qualified under the SysML:: namespace, + /// ending in "::" + bareMetaclassName. This covers both the canonical two-segment + /// spelling real-world filter expressions use (SysML::PartUsage) and the stdlib's + /// actual, deeper declaring package path (e.g. SysML::Systems::PartUsage) — a filter + /// written against either spelling matches the same metaclass. The SysML:: namespace + /// prefix requirement keeps this from over-matching an unrelated user package that happens to + /// declare its own type with a colliding simple name (e.g. Acme::Widgets::PartUsage). + /// + private static bool MetaclassNameMatches(string bareMetaclassName, string typeName) => + typeName == bareMetaclassName || + (typeName.StartsWith("SysML::", StringComparison.Ordinal) && + typeName.EndsWith("::" + bareMetaclassName, StringComparison.Ordinal)); + + /// + /// Walks the stdlib specializes chain starting from the stdlib metadata def + /// declaration whose simple name is , looking for an + /// ancestor metaclass matching . + /// + /// + /// Stdlib-only nodes are never passed through (see + /// 's remarks), so this walk cannot use resolved + /// edges the way ordinary user-model specialization + /// lookups do (e.g. ReferenceResolver.FindMemberInTypeHierarchy). Instead it uses each + /// stdlib metaclass declaration's raw, unresolved text + /// (populated unconditionally by AstBuilder during parsing, before any resolution pass + /// runs) and resolves each simple supertype name to its declaring stdlib node by a + /// same-simple-name suffix lookup restricted to entries + /// of (stdlib metaclasses are declared with a longer + /// nested-package qualified name than their conventional SysML:: spelling, e.g. + /// SysML::Systems::PartUsage). This lookup is precomputed once per + /// and cached (see ) rather than re-scanned on every + /// specialization-walk step, since a classification test may walk many candidates' chains + /// during a single filter evaluation. Restricting the lookup to + /// keeps this sound in the presence of a user model that happens to declare its own + /// definition/usage with a colliding simple name (e.g. a user part def PartUsage;): such + /// user declarations are never candidates for the stdlib specialization walk. This is a + /// deliberately narrow, bounded heuristic (not a general reference-resolution mechanism): it + /// assumes the stdlib's metaclass simple names are unique enough for a suffix match to be + /// unambiguous, which holds for the metaclass names this table covers. + /// guards against a cyclical specialization chain (defensive only — + /// the stdlib itself is well-formed), mirroring ReferenceResolver.FindMemberInTypeHierarchy's + /// cycle-guard pattern. + /// + private static bool ConformsToMetaclass( + SysmlWorkspace workspace, + string bareMetaclassName, + string typeName, + HashSet visited) + { + if (!visited.Add(bareMetaclassName)) + { + return false; + } + + if (!GetStdlibBareNameLookup(workspace).TryGetValue(bareMetaclassName, out var qualifiedName) || + !workspace.Declarations.TryGetValue(qualifiedName, out var declaration)) + { + return false; + } + + foreach (var supertypeName in declaration.SupertypeNames) + { + var bareSupertypeName = supertypeName.Contains("::", StringComparison.Ordinal) + ? supertypeName[(supertypeName.LastIndexOf("::", StringComparison.Ordinal) + 2)..] + : supertypeName; + + if (MetaclassNameMatches(bareSupertypeName, typeName) || + ConformsToMetaclass(workspace, bareSupertypeName, typeName, visited)) + { + return true; + } + } + + return false; + } + + /// + /// A per-workspace cache of 's bare-simple-name → declaring + /// stdlib qualified-name lookup, keyed by the owning instance so a + /// long-lived workspace's lookup is built at most once regardless of how many candidates or + /// filter evaluations consult it. + /// is used (rather than a plain dictionary) so a workspace can still be garbage-collected + /// normally once nothing else references it. + /// + private static readonly System.Runtime.CompilerServices.ConditionalWeakTable> StdlibBareNameLookups = new(); + + /// + /// Returns (building and caching on first use per ) a dictionary + /// mapping each stdlib metaclass's bare simple name (e.g. PartUsage) to its full + /// declaring qualified name (e.g. SysML::Systems::PartUsage), restricted to + /// entries so a colliding user-declared simple name + /// is never a candidate. When two stdlib declarations share the same simple name (not expected + /// for the metaclass names this table covers — see 's + /// remarks), the first one encountered wins, matching the previous linear-scan behavior's + /// first-match semantics. + /// + private static IReadOnlyDictionary GetStdlibBareNameLookup(SysmlWorkspace workspace) => + StdlibBareNameLookups.GetValue(workspace, static ws => + { + var lookup = new Dictionary(StringComparer.Ordinal); + foreach (var name in ws.StdlibNames) + { + var bareName = name.Contains("::", StringComparison.Ordinal) + ? name[(name.LastIndexOf("::", StringComparison.Ordinal) + 2)..] + : name; + lookup.TryAdd(bareName, name); + } + + return lookup; + }); + + /// Evaluates a comparison expression: absent attribute reads always evaluate to false. private static bool EvaluateComparison(SysmlNode node, ComparisonFilterExpression comparison) { diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs index 10b1c44..123ef4f 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/ExposeScopeResolver.cs @@ -14,7 +14,7 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// The resolved qualified-name scope a view's expose statements restrict a diagram to, /// distinguishing unfiltered "whole containment subtree" exposed paths from bracket-filtered /// (expose <path>::**[<expr>]) exposed paths that narrow to specific matched -/// descendant definitions only. +/// descendant definitions and/or named usages only. /// /// /// Exposed subject qualified names whose entire containment subtree is in scope (a @@ -23,9 +23,9 @@ namespace DemaConsulting.SysML2Tools.Layout.Internal; /// failed to parse or evaluate. /// /// -/// Individual definition qualified names matched by a successfully-evaluated bracket-filter -/// expression — exact matches only; a matched definition's own nested members are not -/// automatically included unless they themselves also match the filter. +/// Individual definition or named-usage qualified names matched by a successfully-evaluated +/// bracket-filter expression — exact matches only; a matched declaration's own nested members are +/// not automatically included unless they themselves also match the filter. /// internal sealed record ExposedScope( IReadOnlyList PrefixSubjects, @@ -75,7 +75,7 @@ internal static class ExposeScopeResolver /// scope as well, so both the usage and its type's subtree are included. This expansion only /// applies to whole-subtree () entries — a /// successfully-evaluated bracket filter's already - /// name the exact matched definitions. + /// name the exact matched definitions or usages. /// /// The workspace, used to look up each exposed target's declaration. /// The view's AST node, or null for the synthetic --auto view. @@ -137,7 +137,7 @@ internal static class ExposeScopeResolver var candidates = workspace.Declarations .Where(kvp => (kvp.Key == target || kvp.Key.StartsWith(target + "::", StringComparison.Ordinal)) && - kvp.Value is SysmlDefinitionNode) + (kvp.Value is SysmlDefinitionNode || (kvp.Value is SysmlFeatureNode && kvp.Value.Name is not null))) .Select(kvp => kvp.Key) .ToList(); var evaluation = FilterExpressionEvaluator.Evaluate(workspace, candidates, expression); @@ -176,7 +176,7 @@ private static void AddWholeSubtreeSubject(SysmlWorkspace workspace, string targ /// their containment subtrees (a "{subject}::" prefix match, reusing the same /// qualified-name-prefix idiom /// already uses for stdlib-prefix matching), or is an exact match of one of 's - /// (a bracket-filter-matched definition). + /// (a bracket-filter-matched definition or usage). /// public static bool IsInSubjectScope(string qualifiedName, ExposedScope scope) => scope.PrefixSubjects.Any(subject => diff --git a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs index e9d7461..3110baa 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Layout/Internal/GeneralViewLayoutStrategy.cs @@ -141,7 +141,22 @@ kind is EdgeKind.Typing or EdgeKind.Allocate or EdgeKind.Dependency or EdgeKind. ? LineStyle.Dashed : LineStyle.Solid; - /// A user-defined definition together with its computed box size and supertypes. + /// A user-defined definition or named usage together with its computed box size and supertypes. + /// The definition or usage's fully qualified name. + /// The definition or usage's simple (unqualified) name. + /// The definition or usage keyword (e.g. part, requirement). + /// The declared supertype names (specialization/subsetting/redefinition targets). + /// The owned feature memberships to render as compartment rows. + /// The computed compartments to render inside the box. + /// The computed box width. + /// The computed box height. + /// The applied annotation metadata associated with this node. + /// + /// when this box represents a usage + /// (rather than a definition). Used by + /// to decide which boxes are eligible for + /// nested-duplicate exclusion — definitions are never excluded by that rule. + /// private sealed record DefBox( string QualifiedName, string SimpleName, @@ -151,7 +166,8 @@ private sealed record DefBox( IReadOnlyList Compartments, double Width, double Height, - IReadOnlyList Annotations); + IReadOnlyList Annotations, + bool IsUsage); /// Where a located definition's node lives: the node itself and its owning package. private readonly record struct Location(LayoutGraphNode Node, string Package); @@ -229,6 +245,21 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) } } + // Drop any usage-level box that would duplicate content already shown as a compartment row + // of its nearest still-rendered ancestor box (see RemoveRedundantNestedUsages, which + // processes boxes shallowest-first so that a usage excluded earlier in the same pass is + // treated as absent for its own children's test — cascading correctly through any nesting + // depth instead of silently dropping a deeper-nested usage). This must run after the + // standalone filter narrowing above (not before it and not inside CollectDefinitions), so + // that a usage whose parent was excluded by the filter — rather than by scope — is + // correctly kept as its own standalone box instead of being wrongly dropped by an + // earlier-run dedup pass that saw the parent still present in a pre-filter set. + defs = RemoveRedundantNestedUsages(defs); + if (defs.Count == 0) + { + return new LayoutTree(200.0, 100.0, []); + } + // Group definitions by their owning package (prefix before the last "::"). var groups = GroupByPackage(defs); @@ -267,10 +298,33 @@ public LayoutTree BuildLayout(ViewContext context, RenderOptions options) } /// - /// Collects every user-defined from the workspace and computes + /// Collects every user-defined (and every named + /// usage, at any nesting depth) from the workspace and computes /// each box's intrinsic size from its keyword and name, restricted to /// when non-null (the view's resolved expose containment subtrees). /// + /// + /// This method implements the first of a two-stage admit-then-dedup design (Phase 2d, see + /// ROADMAP.md's "View filter [<expr>]; expression evaluation" section): + /// usage-level () candidates are admitted here alongside + /// definitions, unconditionally of nesting depth, because this method also doubles as the + /// filter-candidate source — a metaclass-kind classification test like + /// filter @SysML::PartUsage; is inherently about usages, not definitions, and every + /// admitted usage must be present here for such a filter to have anything meaningful to + /// narrow. This superset necessarily includes usages nested inside an already-admitted + /// definition/usage, which would duplicate that parent's compartment row if rendered as its own + /// box; the second stage, , runs later in + /// — after the standalone filter [<expr>]; narrowing — + /// and removes exactly those nested usages whose nearest still-rendered ancestor is present in + /// the final, fully scope-and-filter-narrowed set (cascading through any nesting depth so a + /// deeper-nested usage is never silently dropped merely because an intermediate ancestor was + /// itself excluded). An unnamed usage ( + /// is ) is excluded, since it has no stable qualified name to key a box on; + /// an unnamed , by contrast, is still admitted — it falls back to + /// keying its box on the declaration's own qualified name, exactly as it did before usage-level + /// candidates were introduced. + /// + /// private static IReadOnlyList CollectDefinitions( SysmlWorkspace workspace, Theme theme, @@ -280,7 +334,8 @@ private static IReadOnlyList CollectDefinitions( foreach (var (qualifiedName, declaration) in workspace.Declarations) { - if (declaration is not SysmlDefinitionNode def) + if (declaration is not (SysmlDefinitionNode or SysmlFeatureNode) || + (declaration is SysmlFeatureNode && declaration.Name is null)) { continue; } @@ -295,25 +350,128 @@ private static IReadOnlyList CollectDefinitions( continue; } - var simpleName = def.Name ?? qualifiedName; - var keyword = string.IsNullOrEmpty(def.DefinitionKeyword) ? "def" : def.DefinitionKeyword; - - // Build compartments from the definition's owned usages (attributes, ports, parts, …). - var compartments = BuildCompartments(def); - - var memberships = CollectMemberships(def); + var simpleName = declaration.Name ?? qualifiedName; + var rawKeyword = declaration switch + { + SysmlDefinitionNode def => def.DefinitionKeyword, + SysmlFeatureNode feature => feature.FeatureKeyword, + _ => string.Empty, + }; + var keyword = string.IsNullOrEmpty(rawKeyword) ? "def" : rawKeyword; + + // Specialization edges (below, in BuildModelEdges) are drawn from DefBox.SupertypeNames. + // On a SysmlFeatureNode, SupertypeNames is populated only from a usage-level `subsets`/`:>` + // clause (see SysmlNode.SupertypeNames's remarks) — subsetting, not specialization — so it + // must not be surfaced here as a specialization target; only definition-level SupertypeNames + // (`part def X :> Y`) are genuine specialization targets. + var specializationSupertypeNames = declaration is SysmlDefinitionNode + ? declaration.SupertypeNames + : Array.Empty(); + + // Build compartments from the declaration's owned usages (attributes, ports, parts, …). + var compartments = BuildCompartments(declaration); + + var memberships = CollectMemberships(declaration); var (width, height) = ComputeBoxSize(simpleName, keyword, compartments, theme); - result.Add(new DefBox(qualifiedName, simpleName, keyword, def.SupertypeNames, memberships, compartments, width, height, def.Annotations)); + result.Add(new DefBox(qualifiedName, simpleName, keyword, specializationSupertypeNames, memberships, compartments, width, height, declaration.Annotations, declaration is SysmlFeatureNode)); } return result; } /// - /// Builds compartments for a definition by grouping its owned usage features by keyword and - /// formatting each as a name : Type [n] row. + /// Removes usage-level () boxes from whose + /// nearest still-rendered ancestor is a definition/usage that is itself independently rendered + /// as a box — i.e., a usage nested (directly, or transitively through one or more other + /// excluded usages) inside a box that already shows it as a compartment row. + /// + /// + /// This is the second stage of 's admit-then-dedup design. + /// Every node's qualified name is built as a strict parent::child containment chain, so + /// a usage's immediate-parent qualified name is recovered by stripping the qualified name's + /// last "::"-separated segment, and every ancestor is therefore strictly shallower + /// (fewer "::"-separated segments) than its descendants. Boxes are processed in + /// ascending depth order (fewest "::" occurrences first) while incrementally building an + /// excluded set, so that by the time a usage is tested, every one of its ancestors has + /// already been finally decided. A usage is excluded only when its immediate parent is present + /// in and has not itself already been excluded earlier in this + /// same pass — i.e., only when the immediate parent is itself still rendered, and therefore + /// already shows the usage as one of its compartment rows (see , + /// which renders a definition's direct only, never a deeper + /// descendant). Conversely, when the immediate parent has itself just been excluded in this + /// same pass, it no longer renders anywhere, so its own compartment can no longer show this + /// usage — the usage is therefore not excluded, and survives as its own standalone box. This + /// cascades correctly through any nesting depth (two, three, or more levels of usage-in-usage + /// nesting all resolve in this single ordered pass), guaranteeing a deeper-nested usage is never + /// silently dropped merely because an intermediate ancestor between it and the nearest rendered + /// box was itself excluded. A top-level usage (its qualified name has no "::", i.e. it + /// has no containing declaration) is never excluded. Definitions ( + /// is ) are never excluded by this rule, preserving all pre-Phase-2d + /// definition-rendering behavior. The caller () must invoke this after + /// all scope and filter narrowing has been applied to , so that a usage + /// whose immediate parent was excluded by a metaclass filter (rather than genuinely absent from + /// scope) is correctly kept as its own standalone box instead of being wrongly dropped against a + /// pre-filter parent set. + /// + private static IReadOnlyList RemoveRedundantNestedUsages(IReadOnlyList defs) + { + var qualifiedNames = new HashSet(defs.Select(d => d.QualifiedName), StringComparer.Ordinal); + var excluded = new HashSet(StringComparer.Ordinal); + + // Process shallowest-first so that, by the time a usage is tested, its immediate parent's + // own exclusion decision has already been finalized (parents always have strictly fewer + // "::" occurrences than their descendants). + var ordered = defs + .Select((d, index) => (Box: d, Index: index)) + .OrderBy(t => CountOccurrences(t.Box.QualifiedName, "::")) + .ThenBy(t => t.Index); + + foreach (var (box, _) in ordered) + { + if (!box.IsUsage) + { + continue; + } + + var separatorIndex = box.QualifiedName.LastIndexOf("::", StringComparison.Ordinal); + if (separatorIndex < 0) + { + continue; + } + + var parentQualifiedName = box.QualifiedName[..separatorIndex]; + if (qualifiedNames.Contains(parentQualifiedName) && !excluded.Contains(parentQualifiedName)) + { + excluded.Add(box.QualifiedName); + } + } + + return defs.Where(d => !excluded.Contains(d.QualifiedName)).ToList(); + } + + /// + /// Counts the number of non-overlapping occurrences of in + /// , used by as a nesting-depth + /// proxy for ordering (only relative shallower-before-deeper order matters, not an exact count). + /// + private static int CountOccurrences(string value, string token) + { + var count = 0; + var index = 0; + while ((index = value.IndexOf(token, index, StringComparison.Ordinal)) >= 0) + { + count++; + index += token.Length; + } + + return count; + } + + /// + /// Builds compartments for a definition or usage by grouping its owned usage features by + /// keyword and formatting each as a name : Type [n] row. /// - private static IReadOnlyList BuildCompartments(SysmlDefinitionNode def) + private static IReadOnlyList BuildCompartments(SysmlNode def) { // Preserve keyword first-seen order so compartments appear in declaration order. var order = new List(); @@ -380,7 +538,7 @@ private static string FormatFeatureRow(SysmlFeatureNode feature) /// name, redefined-feature reference (if any), and subsetted-feature reference(s) (if any) of /// each owned feature that carries a type annotation, a redefinition, and/or a subsetting. /// - private static IReadOnlyList CollectMemberships(SysmlDefinitionNode def) + private static IReadOnlyList CollectMemberships(SysmlNode def) { var result = new List(); foreach (var child in def.Children) diff --git a/test/DemaConsulting.SysML2Tools.Tests/Filtering/FilterExpressionEvaluatorTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Filtering/FilterExpressionEvaluatorTests.cs index 7e0eb0a..16f72a6 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Filtering/FilterExpressionEvaluatorTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Filtering/FilterExpressionEvaluatorTests.cs @@ -208,4 +208,159 @@ public async Task Evaluate_UnknownCandidate_SkipsGracefully() Assert.Empty(result.MatchedQualifiedNames); } + + private const string UsageAndDefinitionSource = """ + package Q { + metadata def Safety { + } + + part def Engine { + part cylinder; + } + + requirement def Req1; + + part myEngine : Engine { + @Safety; + } + + requirement myRequirement : Req1; + + item myItem; + } + """; + + /// + /// Existing applied-annotation matching (@Safety) still works on a usage-level + /// candidate, unaffected by the new metaclass-kind match path being additionally checked. + /// + [Fact] + public async Task Evaluate_ClassificationTest_AppliedAnnotationMatchingUnaffectedByMetaclassKindAddition() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("Safety"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::myEngine", "Q::myRequirement", "Q::myItem"], expression); + + Assert.Equal(["Q::myEngine"], result.MatchedQualifiedNames); + } + + /// A bare metaclass-kind classification test matches a usage of the matching keyword. + [Fact] + public async Task Evaluate_BareMetaclassKind_MatchesUsage() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("PartUsage"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::myEngine", "Q::myRequirement", "Q::myItem"], expression); + + Assert.Equal(["Q::myEngine"], result.MatchedQualifiedNames); + } + + /// A SysML::-qualified metaclass-kind classification test matches a usage identically to the bare form. + [Fact] + public async Task Evaluate_QualifiedMetaclassKind_MatchesUsage() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("SysML::PartUsage"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::myEngine", "Q::myRequirement", "Q::myItem"], expression); + + Assert.Equal(["Q::myEngine"], result.MatchedQualifiedNames); + } + + /// A metaclass-kind classification test also matches a definition of the corresponding *Definition metaclass. + [Fact] + public async Task Evaluate_MetaclassKind_MatchesDefinition() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("SysML::PartDefinition"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::Engine", "Q::Req1"], expression); + + Assert.Equal(["Q::Engine"], result.MatchedQualifiedNames); + } + + /// A metaclass-kind classification test for an unrelated metaclass does not match. + [Fact] + public async Task Evaluate_MetaclassKind_NonMatchingMetaclass_DoesNotMatch() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("RequirementUsage"); + + var result = FilterExpressionEvaluator.Evaluate(workspace, ["Q::myEngine"], expression); + + Assert.Empty(result.MatchedQualifiedNames); + } + + /// + /// A metaclass-kind classification test also matches via the stdlib's specializes + /// chain: a requirement usage's mapped metaclass (RequirementUsage) + /// specializes ConstraintUsage in the stdlib, so @ConstraintUsage matches it + /// too. + /// + [Fact] + public async Task Evaluate_MetaclassKind_SpecializationConformance_MatchesAncestorMetaclass() + { + var workspace = await LoadAsync(UsageAndDefinitionSource); + var expression = new ClassificationTestExpression("SysML::ConstraintUsage"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::myRequirement", "Q::myEngine"], expression); + + Assert.Equal(["Q::myRequirement"], result.MatchedQualifiedNames); + } + + /// + /// The specialization-conformance walk in ConformsToMetaclass resolves the stdlib + /// metaclass declaration by simple name (see that method's remarks). A user model may + /// happen to declare its own definition/usage whose simple name collides with a genuine + /// stdlib metaclass name (here, a user part def RequirementUsage; that has nothing + /// to do with the real SysML::RequirementUsage metaclass and does not specialize + /// anything). The walk must still resolve the genuine stdlib RequirementUsage + /// metaclass declaration — not the colliding user declaration — so + /// @ConstraintUsage still matches a requirement usage via the stdlib's + /// RequirementUsage specializes ConstraintUsage chain, unaffected by the collision. + /// + [Fact] + public async Task Evaluate_MetaclassKind_SpecializationConformance_UnaffectedByUserModelNameCollision() + { + var workspace = await LoadAsync(UsageAndDefinitionSourceWithCollidingUserDeclaration); + var expression = new ClassificationTestExpression("SysML::ConstraintUsage"); + + var result = FilterExpressionEvaluator.Evaluate( + workspace, ["Q::myRequirement", "Q::myEngine"], expression); + + Assert.Equal(["Q::myRequirement"], result.MatchedQualifiedNames); + } + + private const string UsageAndDefinitionSourceWithCollidingUserDeclaration = """ + package Q { + metadata def Safety { + } + + part def Engine { + part cylinder; + } + + requirement def Req1; + + part myEngine : Engine { + @Safety; + } + + requirement myRequirement : Req1; + + item myItem; + + // Colliding user-model declaration: same simple name as the stdlib + // "RequirementUsage" metaclass, but an unrelated user part definition + // that does not specialize anything. + part def RequirementUsage; + } + """; } diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs index 8165410..ba3626d 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/ExposeScopeResolverTests.cs @@ -322,6 +322,39 @@ public void ResolveExposedScope_TwoExposeEdges_DefinitionAndUsageTarget_UnionsBo Assert.Contains("Root::Vehicle", scope.PrefixSubjects); } + /// + /// A bracket-filtered expose entry's candidate set also includes usage-level + /// () declarations, not only s + /// (Phase 2d) — a metaclass-kind classification test (@SysML::PartUsage) can + /// therefore match a part usage nested within the exposed target's containment subtree. + /// + [Fact] + public void ResolveExposedScope_BracketFilterMetaclassKind_MatchesUsageLevelCandidate() + { + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::Container"] = new SysmlDefinitionNode { Name = "Container", QualifiedName = "Root::Container", DefinitionKeyword = "part def" }, + ["Root::Container::myPart"] = new SysmlFeatureNode { Name = "myPart", QualifiedName = "Root::Container::myPart", FeatureKeyword = "part" }, + ["Root::Container::myRequirement"] = new SysmlFeatureNode { Name = "myRequirement", QualifiedName = "Root::Container::myRequirement", FeatureKeyword = "requirement" } + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + ExposeMembers = [new ExposeMember("Root::Container", "@SysML::PartUsage")], + ResolvedEdges = [new SysmlEdge("Root::V", "Root::Container", SysmlEdgeKind.Expose)] + }; + + var scope = ExposeScopeResolver.ResolveExposedScope(workspace, viewNode); + + Assert.NotNull(scope); + Assert.Equal(["Root::Container::myPart"], scope.ExplicitMembers); + Assert.Empty(scope.Failures); + } + /// /// A bracket-filtered expose entry (expose Root::Container::**[@Safety]) that /// parses and evaluates successfully narrows to only the descendant definitions diff --git a/test/DemaConsulting.SysML2Tools.Tests/Layout/GeneralViewLayoutStrategyTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Layout/GeneralViewLayoutStrategyTests.cs index 6c16748..efddb9f 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Layout/GeneralViewLayoutStrategyTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Layout/GeneralViewLayoutStrategyTests.cs @@ -1204,6 +1204,303 @@ public void GeneralViewLayoutStrategy_BuildLayout_FilterExpressionMatchesNothing Assert.Empty(CollectBoxes(layout.Nodes)); } + /// + /// A workspace of mixed usage kinds (part/requirement/other), reproducing the shape of the + /// OMG's 42.Views/ViewsExample.sysml corpus example (a filter statement + /// dominated by usage-level @SysML::PartUsage classification tests over a model + /// with no part def declarations at all). + /// + private static SysmlWorkspace BuildMixedUsageKindWorkspace() => new() + { + Declarations = new Dictionary + { + ["Root::myPart"] = new SysmlFeatureNode { Name = "myPart", QualifiedName = "Root::myPart", FeatureKeyword = "part" }, + ["Root::myRequirement"] = new SysmlFeatureNode { Name = "myRequirement", QualifiedName = "Root::myRequirement", FeatureKeyword = "requirement" }, + ["Root::myAttribute"] = new SysmlFeatureNode { Name = "myAttribute", QualifiedName = "Root::myAttribute", FeatureKeyword = "attribute" } + } + }; + + /// + /// Regression test for the OMG's own canonical 42.Views/ViewsExample.sysml corpus + /// pattern (see ROADMAP.md's Phase 2d visual gate): a standalone + /// filter @SysML::PartUsage; statement, with no expose, against a model with + /// mixed part/requirement/attribute usages and no part def declarations at all, + /// renders only the PartUsage element(s) — not empty, as it did before Phase 2d. + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_QualifiedPartUsageFilter_RendersOnlyPartUsages() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = BuildMixedUsageKindWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + FilterExpressionText = "@SysML::PartUsage" + }; + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + Assert.Empty(layout.Warnings); + var labels = CollectBoxes(layout.Nodes).Select(b => b.Label).ToList(); + Assert.Contains("myPart", labels); + Assert.DoesNotContain("myRequirement", labels); + Assert.DoesNotContain("myAttribute", labels); + } + + /// + /// The bare-spelling variant of the above (filter @PartUsage;) renders identically, + /// confirming both the bare and SysML::-qualified metaclass-name spellings work. + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_BarePartUsageFilter_RendersOnlyPartUsages() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = BuildMixedUsageKindWorkspace(); + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + FilterExpressionText = "@PartUsage" + }; + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + Assert.Empty(layout.Warnings); + var labels = CollectBoxes(layout.Nodes).Select(b => b.Label).ToList(); + Assert.Contains("myPart", labels); + Assert.DoesNotContain("myRequirement", labels); + Assert.DoesNotContain("myAttribute", labels); + } + + /// + /// A view with no filter/expose at all now renders usage-level candidates too + /// (Phase 2d's CollectDefinitions widening), alongside pre-existing definitions — + /// confirming the widened candidate set is also the default (unfiltered) render behavior, + /// not merely a filter-narrowing behavior. + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersUsageLevelCandidatesToo() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = BuildMixedUsageKindWorkspace(); + var context = new ViewContext("v", workspace); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var labels = CollectBoxes(layout.Nodes).Select(b => b.Label).ToList(); + Assert.Contains("myPart", labels); + Assert.Contains("myRequirement", labels); + Assert.Contains("myAttribute", labels); + } + + /// + /// Retry-1 regression fix: a usage nested directly inside an independently-rendered + /// definition must not also render as its own standalone box — that would duplicate the + /// compartment row the usage already occupies inside the definition's box. This is the + /// direct, minimal reproduction of the quality-reported 21 → 47 box-count regression on + /// docs/gallery/models/01-drone-general.sysml's DroneGeneralView (see the + /// dedicated gallery-corpus regression guard test below for the full-scale case). + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_NoFilter_ExcludesUsageNestedInsideRenderedDefinition() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::Drone"] = new SysmlDefinitionNode + { + Name = "Drone", + QualifiedName = "Root::Drone", + DefinitionKeyword = "part def", + Children = [new SysmlFeatureNode { Name = "airframe", QualifiedName = "Root::Drone::airframe", FeatureKeyword = "part", FeatureTyping = "Frame" }] + }, + // A real workspace (built by WorkspaceLoader) registers every named nested + // declaration under its own qualified-name key too, not merely as a Children entry + // of its owner — reproducing that shape here is what actually exercises + // CollectDefinitions's usage-level widening for this nested usage. + ["Root::Drone::airframe"] = new SysmlFeatureNode { Name = "airframe", QualifiedName = "Root::Drone::airframe", FeatureKeyword = "part", FeatureTyping = "Frame" } + } + }; + var context = new ViewContext("v", workspace); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var rectangles = CollectBoxes(layout.Nodes).Where(b => b.Shape == BoxShape.Rectangle).ToList(); + Assert.Single(rectangles); + Assert.Equal("Drone", rectangles[0].Label); + } + + /// + /// Retry-1 regression fix: a nested usage whose immediate parent is excluded from the final + /// rendered set by a metaclass filter (rather than by scope) must still render as its own + /// standalone box — proving RemoveRedundantNestedUsages runs after (not before) + /// standalone filter narrowing, so it only removes usages whose parent survived the filter. + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_MetaclassFilter_KeepsNestedUsageWhenParentExcluded() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::Container"] = new SysmlFeatureNode + { + Name = "Container", + QualifiedName = "Root::Container", + FeatureKeyword = "requirement", + Children = [new SysmlFeatureNode { Name = "child", QualifiedName = "Root::Container::child", FeatureKeyword = "part" }] + }, + ["Root::Container::child"] = new SysmlFeatureNode { Name = "child", QualifiedName = "Root::Container::child", FeatureKeyword = "part" } + } + }; + var viewNode = new SysmlViewNode + { + Name = "V", + QualifiedName = "Root::V", + FilterExpressionText = "@SysML::PartUsage" + }; + var context = new ViewContext("v", workspace, viewNode); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + // "Container" may still appear as a package-folder label — GroupByPackage treats any + // qualified-name prefix as a folder path regardless of whether that prefix belongs to a + // package or an excluded definition/usage — but it must not appear as its own rendered + // rectangle box (which would mean the excluded metaclass-filtered-out usage was rendered + // after all). "child" must render as its own rectangle box. + var rectangleLabels = CollectBoxes(layout.Nodes) + .Where(b => b.Shape == BoxShape.Rectangle) + .Select(b => b.Label) + .ToList(); + Assert.DoesNotContain("Container", rectangleLabels); + Assert.Contains("child", rectangleLabels); + } + + /// + /// Retry-2 regression fix: a usage nested two or more levels deep (e.g. + /// part def A { part b { part c; } }) must not be silently dropped when its + /// immediate parent is itself excluded as a redundant nested usage. Since b is + /// excluded (its parent A is rendered), b no longer renders anywhere and its + /// compartment can no longer show c — so c must survive as its own standalone + /// box rather than vanishing entirely (the single-pass, pre-dedup-snapshot bug the prior + /// quality re-validation found). + /// + [Fact] + public void GeneralViewLayoutStrategy_BuildLayout_NoFilter_RendersDeeplyNestedGrandchildUsageWhenIntermediateParentExcluded() + { + var strategy = new GeneralViewLayoutStrategy(); + var workspace = new SysmlWorkspace + { + Declarations = new Dictionary + { + ["Root::A"] = new SysmlDefinitionNode + { + Name = "A", + QualifiedName = "Root::A", + DefinitionKeyword = "part def", + Children = [new SysmlFeatureNode { Name = "b", QualifiedName = "Root::A::b", FeatureKeyword = "part" }] + }, + // A real workspace (built by WorkspaceLoader) registers every named nested + // declaration under its own qualified-name key too, not merely as a Children entry + // of its owner — reproducing that shape here is what actually exercises + // CollectDefinitions's usage-level widening for both nested usages. + ["Root::A::b"] = new SysmlFeatureNode + { + Name = "b", + QualifiedName = "Root::A::b", + FeatureKeyword = "part", + Children = [new SysmlFeatureNode { Name = "c", QualifiedName = "Root::A::b::c", FeatureKeyword = "part" }] + }, + ["Root::A::b::c"] = new SysmlFeatureNode { Name = "c", QualifiedName = "Root::A::b::c", FeatureKeyword = "part" } + } + }; + var context = new ViewContext("v", workspace); + var options = new RenderOptions(Themes.Light); + + var layout = strategy.BuildLayout(context, options); + + var rectangles = CollectBoxes(layout.Nodes).Where(b => b.Shape == BoxShape.Rectangle).ToList(); + var rectangleLabels = rectangles.Select(b => b.Label).ToList(); + Assert.Equal(2, rectangles.Count); + Assert.Contains("A", rectangleLabels); + Assert.DoesNotContain("b", rectangleLabels); + Assert.Contains("c", rectangleLabels); + } + + /// + /// Real-corpus regression guard for the quality-reported 21 → 47 box-count regression: the + /// project's own checked-in gallery example docs/gallery/models/01-drone-general.sysml + /// must render exactly the 21 rectangle-shaped definition boxes matching the currently + /// checked-in docs/gallery/svg/DroneGeneralView.svg's <rect> count + /// (independently re-counted via Select-String -Pattern '<rect') when laid out + /// through the actual . Only + /// boxes are counted (excluding the single package folder and the Battery definition's + /// documentation-annotation note box, neither of which is drawn as a checked-in <rect> + /// element), matching the checked-in SVG's own count basis. This converts the quality + /// agent's one-off manual git stash empirical check into a standing, automated test. + /// + [Fact] + public async Task GeneralViewLayoutStrategy_BuildLayout_DroneGalleryModel_RendersExactly21BoxesMatchingCheckedInSvg() + { + var galleryModelsRoot = FindGalleryModelsRoot(); + if (galleryModelsRoot is null) + { + return; + } + + var modelPath = Path.Combine(galleryModelsRoot, "01-drone-general.sysml"); + if (!File.Exists(modelPath)) + { + return; + } + + var (stdlibTable, _) = StdlibProvider.GetSymbolTable(); + var result = await WorkspaceLoader.LoadAsync([modelPath], stdlibTable); + Assert.NotNull(result.Workspace); + var workspace = result.Workspace!; + + const string viewQualifiedName = "QuadcopterDrone::DroneGeneralView"; + var viewNode = Assert.IsType(workspace.Declarations[viewQualifiedName]); + + var strategy = new GeneralViewLayoutStrategy(); + var options = new RenderOptions(Themes.Light); + var layout = strategy.BuildLayout(new ViewContext("v", workspace, viewNode), options); + + var boxes = CollectBoxes(layout.Nodes).Where(b => b.Shape == BoxShape.Rectangle).ToList(); + Assert.Equal(21, boxes.Count); + } + + /// + /// Finds the repository's docs/gallery/models directory relative to the test + /// assembly, mirroring 's upward path-walk convention. + /// + private static string? FindGalleryModelsRoot() + { + var dir = AppContext.BaseDirectory; + while (dir is not null) + { + var candidate = Path.Combine(dir, "docs", "gallery", "models"); + if (Directory.Exists(candidate)) + { + return candidate; + } + dir = Directory.GetParent(dir)?.FullName; + } + + return null; + } + /// /// A view with no expose statement (a null , e.g. /// the --auto synthesized view) renders identically to the pre-scoping-change @@ -1239,11 +1536,17 @@ public void GeneralViewLayoutStrategy_BuildLayout_NullViewNode_RendersFullWorksp /// into, leaving the view with zero Expose edges and causing /// to silently fall back to rendering the entire /// workspace. After the fix, this view's layout must be scoped to the vehicle - /// subtree — which, in this fixture, contains no part def declarations (only - /// usages), so the correctly-scoped rendering drops to zero boxes, strictly fewer than the - /// unscoped full-workspace rendering (which still includes the unrelated - /// AnnotationDefinitions::Safety/Security metadata definitions). Before the - /// fix, scoped and full box counts were identical (both rendered everything). + /// subtree. Since Phase 2d (see ROADMAP.md's "View filter [<expr>]; + /// expression evaluation" section), 's internal + /// candidate collection admits usage-level candidates too, so the `vehicle` subtree's part + /// *usages* (this + /// fixture declares no `part def`) render as boxes: the bracket filter + /// @Safety and (as Safety).isMandatory narrows those usages to exactly the ones + /// carrying a mandatory @Safety annotation (seatBelt, bumper), while + /// the unrelated AnnotationDefinitions::Safety/Security metadata definitions + /// — outside the vehicle subtree entirely — remain excluded from both renderings' + /// scoped result. Before the Phase 2d fix, the correctly-scoped rendering dropped to zero + /// boxes (usages were not renderable candidates at all). /// // cspell:ignore Feaure -- typo present verbatim in the real OMG corpus fixture's package name [Fact] @@ -1285,16 +1588,13 @@ public async Task GeneralViewLayoutStrategy_BuildLayout_OmgSafetyFeatureViewsFix var scoped = strategy.BuildLayout(new ViewContext("scoped", workspace, viewNode), options); var full = strategy.BuildLayout(new ViewContext("full", workspace), options); - // Assert: the scoped view renders strictly fewer boxes than the full workspace. In this - // fixture the `vehicle` subtree is built entirely from part *usages* (not `part def` - // declarations), which `GeneralViewLayoutStrategy.CollectDefinitions` does not render as - // boxes — so the correctly-scoped view renders zero boxes here, while the unscoped full - // workspace still renders the two unrelated `AnnotationDefinitions` metadata definitions - // (`Safety`, `Security`). That drop from 2 boxes to 0 is itself the regression signal: - // before the fix, `ResolveExposedScope` saw no `Expose` edges and fell back to rendering - // the entire workspace (i.e. scoped would equal full, not be strictly smaller). + // Assert: the scoped view renders a non-empty subset of the vehicle's mandatory-safety + // part usages, strictly fewer than the full workspace, and excludes the unrelated + // AnnotationDefinitions metadata definitions from both the scoped subset and (by name) + // from being conflated with the vehicle's own usages. var scopedBoxes = CollectBoxes(scoped.Nodes); var fullBoxes = CollectBoxes(full.Nodes); + Assert.NotEmpty(scopedBoxes); Assert.True(scopedBoxes.Count < fullBoxes.Count, $"expected scoped box count ({scopedBoxes.Count}) < full box count ({fullBoxes.Count})"); var fullLabels = fullBoxes.Select(b => b.Label).ToList(); @@ -1303,6 +1603,8 @@ public async Task GeneralViewLayoutStrategy_BuildLayout_OmgSafetyFeatureViewsFix var scopedLabels = scopedBoxes.Select(b => b.Label).ToList(); Assert.DoesNotContain("Safety", scopedLabels); Assert.DoesNotContain("Security", scopedLabels); + Assert.Contains("seatBelt", scopedLabels); + Assert.Contains("bumper", scopedLabels); } ///