From 910a5e87fa8114be28d9ab85044fcd3550636f49 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Wed, 29 Jul 2026 14:24:25 -0400 Subject: [PATCH 1/4] Add connection-aware impact analysis to Query subsystem Add an opt-in `--include-connections` flag to `query impact`, backed by `QueryOptions.IncludeConnections`. Default impact semantics are unchanged. When enabled, `Connect` and `Binding` edges are traversed undirected, since `SysmlEdge` documents their source/target ordering as carrying no causal direction. All other edge kinds keep the existing reverse-only directed traversal. Connection endpoints are rolled up through containment in both directions, reusing the `Connections` verb's prefix logic (extracted as `IsSelfOrNestedUnder`), so a subject part matches endpoints nested inside it and each far endpoint is attributed to its nearest owning declaration. Connector hops are bounded by `WalkDepth ?? 1` via a per-frontier-item hop counter, guarding against the transitive degeneracy of shared buses. Reference edge depth semantics are unchanged. Add structured, machine-readable members to `QueryResultEntry` for programmatic clients: `int? Depth`, `SysmlEdgeKind? Relation`, and `string? ViaQualifiedName`, all omitted from JSON when null and registered in the source-generated serializer context to stay AOT-safe. `Detail` remains populated for human-readable Markdown, but the structured fields are authoritative so clients need not parse "depth N" from English text. All changes are additive and source-compatible. Requirements, design, verification docs, README, and the user guide are updated so the feature is fully traced. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 1 + docs/design/sysml2-tools-core/query.md | 126 ++++++- docs/design/sysml2-tools-tool/query.md | 17 + docs/reqstream/sysml2-tools-core/query.yaml | 64 +++- docs/reqstream/sysml2-tools-tool/query.yaml | 15 + docs/user_guide/introduction.md | 41 ++- docs/verification/sysml2-tools-core/query.md | 119 ++++++- docs/verification/sysml2-tools-tool/query.md | 12 + .../Query/NamespaceDoc.cs | 27 ++ .../Query/QueryArgumentParser.cs | 9 +- .../Query/QueryEngine.cs | 310 +++++++++++++++--- .../Query/QueryOptions.cs | 25 +- .../Query/QueryResult.cs | 33 ++ .../Query/QueryResultSerializerContext.cs | 1 + .../Query/QueryCommand.cs | 4 + .../Query/QueryStrings.cs | 12 + .../Query/QueryStrings.resx | 12 + .../Query/QueryEngineImpactTests.cs | 226 +++++++++++++ .../Query/QueryRenderingTests.cs | 92 ++++++ .../Cli/ContextTests.cs | 16 + .../Query/QuerySubsystemTests.cs | 28 ++ .../Query/QueryTestFixtures.cs | 52 +++ .../Query/QueryVerbsTests.cs | 214 ++++++++++++ 23 files changed, 1390 insertions(+), 66 deletions(-) create mode 100644 test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs diff --git a/README.md b/README.md index 21a6aead..5eef1ae9 100644 --- a/README.md +++ b/README.md @@ -259,6 +259,7 @@ sysml2tools help [lint|render|query []|export] | `--format markdown\|json` | Output format (default: `markdown`); distinct from `render`'s `--format` (`svg`/`png`) | | `--output ` | Write query report to this **file** (default: stdout); differs from `render`'s `--output` dir | | `--walk-depth <#>` | Maximum impact-walk depth (`impact` verb only) | +| `--include-connections` | `impact` only: also follow `connect`/`bind` edges, undirected (1 hop) | | `--direction up\|down\|both` | Traversal direction (`hierarchy` verb only) | | `--kind ` | Element-kind filter (`list`/`find` verbs only) | | `--name ` | Name substring filter (`list`/`find` verbs only) | diff --git a/docs/design/sysml2-tools-core/query.md b/docs/design/sysml2-tools-core/query.md index 68d4c53c..ab3a3e0a 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -13,7 +13,8 @@ This subsystem contains the following public types: - `QueryVerb` / `QueryVerbParsing` — the fixed 12-verb vocabulary, token conversion, and the `RequiresElement` rule. - `QueryOptions` — the immutable option record shared by every verb; Core callers supply an - already-loaded workspace, so this record has no input-files property. + already-loaded workspace, so this record has no input-files property. `IncludeConnections` + opts the `impact` verb into connector traversal and is meaningful for no other verb. - `QueryArgumentParser` — parses a token list into `(QueryOptions? Options, IReadOnlyList Files)` for callers that accept the same verb/option grammar as the CLI. @@ -68,7 +69,10 @@ flowchart TD - *Type*: Sealed records plus enum. - *Role*: Data container. - *Contract*: `Verb`, optional `Element`, free-form `Summary` lines, and deterministic - `Entries`; `Direction` is populated only for `dependencies`. + `Entries`; `Direction` is populated only for `dependencies`, and the `Depth`/`Relation`/ + `ViaQualifiedName` traversal metadata only by the traversing verbs. Every optional member is + omitted from JSON when null, so adding members never changes an existing verb's payload + shape. **`QueryResultRenderer`**: Result-to-text renderer. @@ -95,8 +99,8 @@ flowchart TD 1. `QueryArgumentParser` validates that the first token is one of the 12 verb tokens, captures `--element`/`--format`/`--walk-depth`/`--direction`/`--kind`/`--name`/ - `--include-stdlib`/`--heading`, and returns any trailing positional tokens separately as the - caller-owned `Files` list. + `--include-stdlib`/`--include-connections`/`--heading`, and returns any trailing positional + tokens separately as the caller-owned `Files` list. 2. `QueryEngine.Execute` is the public dispatcher. It validates the element-required rule for library callers, then routes `QueryOptions.Verb` to the matching verb method so the Tool project and any other caller reuse the exact same switch. @@ -108,7 +112,44 @@ flowchart TD 5. `Dependencies` combines `Uses` and `UsedBy` for the same subject, tagging entries with `QueryEntryDirection.Outgoing` or `Incoming` rather than performing a third traversal. 6. `Impact` performs a breadth-first transitive closure over incoming edges, optionally bounded - by `WalkDepth`, with a visited set preventing infinite loops on cyclic graphs. + by `WalkDepth`, with a visited set preventing infinite loops on cyclic graphs. Each frontier + item is a `(qualified name, connector hops taken)` pair, and each item is expanded by two + collaborating helpers: + - `CollectImpactReferences` — always on, and unchanged from the original algorithm: it reads + `workspace.Index.GetIncomingEdges` and emits one entry per newly-reached source, carrying + the connector-hop count through unchanged (a reference hop never consumes a connector hop). + - `CollectImpactConnections` — only when `QueryOptions.IncludeConnections` is set. It walks + the connector edges collected once per invocation by `CollectConnectorEdges` for + `ImpactConnectorEdgeKinds` (`Connect` and `Binding`) and follows each one **undirected**: + a connector is traversed whenever exactly one of its two endpoints satisfies + `IsSelfOrNestedUnder` for the frontier item, and the other endpoint is then reported. + Requiring *exactly* one side to match simultaneously rejects unrelated connectors and + self-loops whose two ends are both nested inside the subject. `Connect` and `Binding` are + the only two kinds treated this way, because they are the only kinds whose recorded + source/target order is a textual convention rather than a semantic direction; every other + edge kind keeps the reverse-only directed traversal of the first helper. + Containment roll-up applies in both directions. On the subject side, `IsSelfOrNestedUnder` + (shared verbatim with the `connections` verb) lets a `part` subject match a connector + attached to one of its nested ports. On the far side, `RollUpToNearestDeclaration` probes the + endpoint itself first and returns it unchanged when it is already a declared qualified name, + so a connector naming a sibling part directly (`connect alpha to beta;`) reports `beta` + rather than the enclosing definition that also owns the subject. Stripping of trailing `::` + segments applies only to endpoints absent from `Declarations` — typically ports inherited + through a typed usage — so a port endpoint such as `System::hub::J1` is attributed to + `System::hub`. Probing the endpoint before stripping is what keeps `impact` and `connections` + in agreement about the same connector's topology. No information is lost: the raw far + endpoint is preserved structurally in `ViaQualifiedName` and textually in the entry's `Notes` + alongside the originating connector keyword, and an endpoint with no declared ancestor is + reported unchanged rather than dropped. + Two independent bounds are applied, because they differ when `WalkDepth` is `null`. The + reference closure keeps its exact existing semantics (`null` means unlimited), enforced by + the outer breadth-first loop. Connector hops are bounded per traversal path by `WalkDepth` + when supplied and by `DefaultConnectionHopLimit` (one hop) when it is not — a bound the outer + loop therefore cannot express, so a per-frontier-item hop counter carries it instead. The + one-hop default exists because connector graphs in real models are dense meshes, in which an + unbounded connector closure degenerates to "the whole assembly". A single shared ordinal + `visited` set spans both helpers, so cyclic connector topologies terminate and no element is + reported twice. 7. `Describe` reports the target element's own kind and qualified name in `Summary`, then adds resolved supertypes, typing, annotations, applied metadata annotations, and a `Children: N` count. `N` is the count of visible, named child entries actually placed in `Entries` (i.e. @@ -125,9 +166,21 @@ flowchart TD either the source or the target, using direction-aware detail text. 10. `Interface` reports direct child features that are ports or have a resolved typing, using the feature keyword as `Kind` and typing plus multiplicity as `Detail`. -11. `Connections` walks nodes reachable from workspace declarations because resolved `Connect` - edges live on each connector node's own `ResolvedEdges` rather than in `SemanticIndex`'s - global edge list. Entries record the other endpoint, connector keyword, and endpoint role. +11. `Connections` reports resolved `Connect` edges through the shared `CollectConnectorEdges` + node walk, which visits every node reachable from workspace declarations and reads each + connector node's own `ResolvedEdges`. Entries record the other endpoint, connector keyword, + and endpoint role. + **Correction (previously documented otherwise):** connector edges *are* present in + `SemanticIndex.AllEdges` — `ReferenceResolver`'s feature-chain resolution pass appends + `Connect`/`Binding`/`Transition` edges into the same aggregate edge list that constructs the + index, and the index keys every edge by both source and target with no kind filtering. The + earlier claim that they "live on each connector node's own `ResolvedEdges` rather than in + `SemanticIndex`'s global edge list" was factually wrong. The node walk is nevertheless + retained, for a different and correct reason: a `SysmlEdge` carries only + `(Source, Target, Kind)` and not the originating connector's keyword + (`connect`/`connection`/`message`/`bind`), which this verb reports as each entry's `Kind` + and which `Impact` reports in each connection entry's `Notes`. Sharing the one collector + between the two verbs also guarantees they can never disagree about connection topology. 12. `States` recursively walks descendants, collecting `state` features and transition children. When a resolved `Transition` edge is present it uses that edge's endpoints; otherwise it falls back to the transition node's raw source/target text. @@ -156,14 +209,49 @@ flowchart TD - `QueryResult` carries `Verb`, optional `Element`, free-form `Summary` lines, and `Entries`. - `QueryResultEntry` carries `QualifiedName`, optional `Kind`, optional `Detail`, optional - multi-line `Notes`, and optional `Direction`. + multi-line `Notes`, optional `Direction`, and the three optional traversal-metadata members + `Depth`, `Relation`, and `ViaQualifiedName`. - `QueryEntryDirection` is populated only for `dependencies`, and JSON omits the property when it is `null`. +- `Depth` (`int?`) is the 1-based traversal depth at which an entry was reached, populated by + the traversing verbs (`impact`, `hierarchy`). It is the authoritative, machine-readable + counterpart to the `"depth N"` text that `Detail` continues to carry; API consumers read + `Depth` and never parse `Detail`. `Detail` is deliberately left unchanged so Markdown output + stays human-readable and every existing Markdown assertion is byte-identical. +- `Relation` (`SysmlEdgeKind?`) records which relationship kind reached an entry, so + "referenced by" (`Supertype`, `Typing`, …) and "connected to" (`Connect`, `Binding`) are + machine-distinguishable within one combined `impact` result. `SysmlEdgeKind` is reused + directly rather than mirrored into a Query-local enum, because Core already exposes Language + semantic-model types (`SysmlWorkspace`, `SysmlNode`) in `QueryEngine`'s public signatures, so + a mirrored enum would be pure duplication requiring lock-step maintenance. +- `Relation` serializes as its enum member name (e.g. `"Connect"`) via + `JsonStringEnumConverter`, a deliberate and documented asymmetry with the + pre-existing numeric `Direction`: `Direction`'s numeric JSON shape is an established contract + that cannot be changed, whereas a brand-new property is free to adopt the better shape. String + serialization is also the safer of the two here, since it is immune to `SysmlEdgeKind` member + reordering. +- `ViaQualifiedName` (`string?`) names the actual far endpoint an entry was attributed from + when containment roll-up occurred — for a connection entry, the nested port the connector + reached, whose nearest owning declaration is reported as `QualifiedName`. It is `null` when + the far endpoint was already a declaration and no roll-up occurred, matching the + `QueryResultEntry` XML documentation; the raw endpoints remain named in the entry's `Notes`. +- All three traversal-metadata members are nullable and carry + `[JsonIgnore(WhenWritingNull)]`, exactly mirroring `Direction`, so no non-traversing verb's + JSON payload changes shape. - `QueryResultSerializerContext` contains the source-generated JSON metadata for `QueryResult` - and preserves the same sorted order used by Markdown rendering. + and `QueryResultEntry` and preserves the same sorted order used by Markdown rendering. #### Known Model Gaps +- Nested port features are absent from `workspace.Declarations` and are therefore not + resolvable as a query subject via `--element`. The `impact` verb's containment roll-up does + not close this gap: it maps port endpoints *outward* to their declared owners, it does not + make port names resolvable as query subjects. Closing it would require a nested-feature + lookup fallback in the Tool's element resolution and is out of scope here. +- Connector edges are recorded against usage-scoped qualified names (e.g. + `System::hub::J1`), never definition-scoped names, so `impact --include-connections` on a + `part def` subject reports nothing from the connector branch even when its usages are + connected. Reaching a definition's usages would require a separate def-to-usage traversal. - State-usage bodies containing `accept then ;` trigger-shorthand transitions still inherit a pre-existing grammar and `AstBuilder` limitation: the shorthand transition can absorb an adjacent sibling `state` usage instead of producing its own `SysmlTransitionNode`. @@ -185,6 +273,18 @@ flowchart TD in traversal order without affecting external output stability. - `QualifiedNameShortener` affects only `dependencies` Markdown output. JSON output, and every other verb's Markdown output, remains fully qualified. +- The default `impact` semantics are frozen: `IncludeConnections` defaults to `false`, and when + it is unset no connector edges are collected and no connector branch runs, so results are + identical to those produced before connection-aware impact existed. Additions to + `QueryResultEntry` are nullable and null-omitted from JSON for the same reason. +- Connector traversal is bounded by default (`DefaultConnectionHopLimit` = 1 hop) rather than + unbounded, because real connector graphs are dense meshes in which an unbounded closure + answers "the whole assembly". A caller widens the bound deliberately by supplying + `WalkDepth`. +- `Relation`'s string JSON serialization couples the Query JSON contract to `SysmlEdgeKind` + member *names*. This is an accepted, deliberate trade: string serialization is immune to + member reordering (which numeric serialization is not), at the cost of making a future + member *rename* a breaking JSON change. ### Requirements Traceability @@ -194,12 +294,14 @@ flowchart TD | SysML2Tools-Core-Query-UsedBy | `QueryEngine.UsedBy` | | SysML2Tools-Core-Query-Dependencies | `QueryEngine.Dependencies`; `QueryResultRenderer.RenderMarkdown`/`RenderJson` | | SysML2Tools-Core-Query-DependenciesNameShortening | `QueryResultRenderer.RenderMarkdown`; `QualifiedNameShortener` | -| SysML2Tools-Core-Query-Impact | `QueryEngine.Impact` | +| SysML2Tools-Core-Query-Impact | `QueryEngine.Impact`; `QueryEngine.CollectImpactReferences` | +| SysML2Tools-Core-Query-ImpactConnections | `QueryEngine.CollectImpactConnections`; `RollUpToNearestDeclaration` | +| SysML2Tools-Core-Query-EntryTraversalMetadata | `QueryResultEntry.Depth`/`Relation`/`ViaQualifiedName` | | SysML2Tools-Core-Query-Describe | `QueryEngine.Describe` | | SysML2Tools-Core-Query-Hierarchy | `QueryEngine.Hierarchy` | | SysML2Tools-Core-Query-Requirements | `QueryEngine.Requirements` | | SysML2Tools-Core-Query-Interface | `QueryEngine.Interface` | -| SysML2Tools-Core-Query-Connections | `QueryEngine.Connections`; `QueryEngine.CollectConnectEdges` | +| SysML2Tools-Core-Query-Connections | `QueryEngine.Connections`; `CollectConnectorEdges`; `IsSelfOrNestedUnder` | | SysML2Tools-Core-Query-States | `QueryEngine.States`; `QueryEngine.CollectStates` | | SysML2Tools-Core-Query-List | `QueryEngine.List` | | SysML2Tools-Core-Query-Find | `QueryEngine.Find` | diff --git a/docs/design/sysml2-tools-tool/query.md b/docs/design/sysml2-tools-tool/query.md index b2959beb..2d535c5b 100644 --- a/docs/design/sysml2-tools-tool/query.md +++ b/docs/design/sysml2-tools-tool/query.md @@ -78,6 +78,22 @@ flowchart TD and `query --help`. Every printed line is sourced from `QueryStrings`, including the `--output` help text, the workflow note, and the per-verb example/schema-hint enrichment. +#### Pass-Through Options + +- `QueryCliArgumentParser` pre-extracts only `--output`; every other token is forwarded verbatim + to Core's `QueryArgumentParser`, and `Context` stores the resulting `QueryOptions` whole. + Adding a new Core option therefore requires no parsing change in this subsystem. +- `--include-connections` (connection-aware `impact` analysis, Core's + `QueryOptions.IncludeConnections`) is exactly such an option: the only Tool-side work is help + text. `Query_GeneralOptionIncludeConnections` is printed by `PrintGeneralHelp` immediately + after the `--include-stdlib` line, and `Query_OptionIncludeConnectionsImpact` by + `PrintVerbHelp`'s `QueryVerb.Impact` arm immediately after `Query_OptionWalkDepthImpact`. Both + keys live in `QueryStrings.resx` with matching `QueryStrings` accessor properties, as the + resx-parity tests require. +- Because the Tool does not parse the flag itself, keeping this help text in lockstep with + Core's accepted grammar is a Tool-subsystem responsibility verified by a dedicated help test; + a Core-only option with no Tool help line would be silently undiscoverable. + #### Output File Option - `Context.QueryOutput` is a **file path** (not a directory) from `--output`; when omitted, @@ -121,3 +137,4 @@ flowchart TD | SysML2Tools-Tool-Query-ReportHeading | `HeadingDepth`; `QueryOptions.Heading`; `QueryResultRenderer.RenderMarkdown` | | SysML2Tools-Tool-Query-HelpEnrichment | `QueryStrings.GetExample`/`Query_SchemaHint_*`; workflow-note lines | | SysML2Tools-Tool-Query-OutputFile | `QueryCliArgumentParser.Parse`; `QueryCommand.RunAsync`; `QueryResultExporter` | +| SysML2Tools-Tool-Query-IncludeConnections | Core `QueryArgumentParser.Parse` pass-through; `QueryStrings` help lines | diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index 3b19993b..7cbc1ff6 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -98,10 +98,64 @@ sections: justification: | A single-hop reverse-reference view understates true blast radius for deeply-referenced elements. A bounded transitive closure gives callers a practical, terminating answer to - "what is affected if this changes?". + "what is affected if this changes?". These reference-only semantics are the default and + are unchanged by the opt-in connection traversal described by + `SysML2Tools-Core-Query-ImpactConnections`; connector edges are followed only when + `IncludeConnections` is explicitly supplied. tests: - Impact_DepthOne_OnlyReachesDirectReferences - Impact_Unbounded_ReachesTransitiveClosure + - Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult + + - id: SysML2Tools-Core-Query-ImpactConnections + title: >- + When `IncludeConnections` is supplied, the `impact` verb shall additionally traverse + resolved `Connect` and `Binding` connector relationships without regard to their + recorded endpoint order, matching a subject element against endpoints nested inside it + and attributing each far endpoint to its nearest owning declaration, bounding connector + hops along any single traversal path to `WalkDepth` when supplied or to one hop + otherwise, and terminating on cyclic connector topologies. + justification: | + A connector's two ends carry no semantic source-causes-target direction, so the + reverse-only reference closure reports nothing for a part joined to the rest of the + assembly only by `connect` statements — the exact case a change-impact question is + asked about. Connector endpoints are also nested ports, while users reason about the + parts that own them, so containment roll-up in both directions is required for the + answer to be meaningful. Because connector graphs are dense meshes, an unbounded + connector closure degenerates to "the whole assembly", so a one-hop default keeps the + opt-in answer useful while `WalkDepth` remains available to widen it deliberately. + tests: + - Impact_IncludeConnections_ReachesConnectedSiblingPart + - Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart + - Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage + - Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne + - Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop + - Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates + - Impact_IncludeConnections_BindingEdges_AreTraversedUndirected + - Impact_IncludeConnections_ThroughPublicApi_ReturnsConnectedPartEntries + - Impact_IncludeConnections_DeclaredFarEndpoint_ReportsEndpointItself + - Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition + + - id: SysML2Tools-Core-Query-EntryTraversalMetadata + title: >- + Every traversal-produced result entry shall carry machine-readable `Depth`, `Relation`, + and (where an endpoint was rolled up) `ViaQualifiedName` values alongside the existing + human-readable `Detail` text; each of the three shall be omitted from JSON output when + it is null, and `Relation` shall serialize as its edge-kind member name. + justification: | + Non-CLI API consumers previously had to parse the free-form `Detail` string to recover + a traversal depth, and had no way at all to tell which kind of relationship reached an + entry — a distinction that becomes essential once reference and connector traversal are + combined in one result. Exposing the values structurally makes the API contract + authoritative while retaining `Detail` so Markdown output stays human-readable, and + omitting nulls keeps every existing verb's JSON payload unchanged. + tests: + - Impact_ConnectionEntry_RecordsDepthRelationAndViaQualifiedName + - Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName + - Impact_ReferenceEntry_RecordsDepthAndReferenceRelation + - RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips + - RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia + - RenderMarkdown_ImpactEntry_DetailRemainsHumanReadableDepthText - id: SysML2Tools-Core-Query-Describe title: >- @@ -168,9 +222,11 @@ sections: including dotted feature-chain resolution, and shall label each entry with the connector keyword that produced it. justification: | - Resolved connection edges do not live in the semantic index's global edge list, so the - Query subsystem must perform the node walk on behalf of callers that need connection - topology. + A `SysmlEdge` records only its source, target, and kind — not the connector keyword + (`connect`/`connection`/`message`/`bind`) that produced it — so the Query subsystem + must perform the node walk on behalf of callers that need connection topology labeled + by the syntax that declared it. The same walk is shared with the `impact` verb's + connector traversal so the two verbs can never disagree about connection topology. tests: - Connections_ReportsResolvedEndpointsIncludingFeatureChain - Connections_ConnectionsExampleFixture_ReportsAtLeastOneEndpoint diff --git a/docs/reqstream/sysml2-tools-tool/query.yaml b/docs/reqstream/sysml2-tools-tool/query.yaml index 44c5d179..b9d4ab40 100644 --- a/docs/reqstream/sysml2-tools-tool/query.yaml +++ b/docs/reqstream/sysml2-tools-tool/query.yaml @@ -167,6 +167,21 @@ sections: - QuerySubsystem_QueryVerbHelp_MentionsExampleInvocationAndSchemaHints - QuerySubsystem_QueryHelp_NoVerb_MentionsTypicalWorkflow + - id: SysML2Tools-Tool-Query-IncludeConnections + title: >- + The query command shall accept the `--include-connections` flag, forwarding it to + Core's `QueryOptions.IncludeConnections`, and shall document it in both general help + and `impact` verb help through culture-aware resx text. + justification: | + Connection-aware impact analysis changes the answer the `impact` verb gives, so it + must be an explicit, discoverable user choice rather than a silent default. Because + the Tool's CLI parser forwards every non-`--output` token to Core's parser unchanged, + the flag needs no Tool-side parsing — only help text, which must stay in lockstep with + the grammar Core actually accepts. + tests: + - Context_Create_QueryCommand_WithIncludeConnectionsFlag_SetsIncludeConnectionsTrue + - QuerySubsystem_ImpactVerbHelp_MentionsIncludeConnectionsOption + - id: SysML2Tools-Tool-Query-OutputFile title: >- The query command shall write its rendered document to the file path supplied via diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 685bc63a..12c74182 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -434,6 +434,10 @@ sysml2tools query dependencies --element Model::Engine "src/**/*.sysml" # Transitive blast radius of a change, optionally bounded sysml2tools query impact --element Model::Engine --walk-depth 2 "src/**/*.sysml" +# Also follow connect/bind edges, so a part joined to the rest of the assembly only by +# connectors is no longer reported as impacting nothing +sysml2tools query impact --element Model::System::motorA --include-connections "src/**/*.sysml" + # A single-element "fact sheet": kind, supertypes, typing, annotations, applied metadata, children sysml2tools query describe --element Model::Vehicle "src/**/*.sysml" @@ -460,6 +464,40 @@ sysml2tools query find --name Engine "src/**/*.sysml" --format json sysml2tools query describe --element Model::Vehicle --depth 2 --heading "Vehicle Report" "src/**/*.sysml" ``` +## Connection-Aware Impact Analysis + +By default, `query impact` follows only *reference* relationships — specialization, typing, +imports, and similar resolved references — in the reverse direction. A part that is joined to +the rest of an assembly purely by `connect` statements therefore reports no impacted elements, +because a connector is not a reference. + +Adding `--include-connections` makes `impact` follow `connect` and `bind` relationships as +well. Three rules apply: + +- **Connectors are followed in both directions.** A connector's two ends carry no + "source causes target" meaning, so the connected element is reported no matter which end you + query from. Querying a motor reports the hub it is plugged into, and querying that hub + reports every motor plugged into it. +- **Port endpoints roll up to the part that owns them.** Connectors join nested ports, but you + normally ask about parts. An endpoint such as `System::hub::J1` is reported as + `System::hub`, with the actual port named in the entry's notes (and in the + `ViaQualifiedName` field of `--format json` output) so nothing is lost. Roll-up applies only + to endpoints that are not themselves declared elements — typically ports inherited through a + typed usage such as `part hub : Hub`. An endpoint that *is* a declared element, such as a + directly connected sibling part (`connect alpha to beta;`) or a port declared inline on a + usage, is reported as-is, and `ViaQualifiedName` is then absent. +- **Connector hops are bounded.** Real models connect many parts to a shared hub, so an + unbounded connector walk quickly reports the whole assembly. Each traversal path may take at + most one connector hop unless you supply `--walk-depth `, which raises the limit to `n`. + `--walk-depth` continues to bound reference-edge depth exactly as before; omitting it still + means *unlimited* reference depth, and only the connector limit defaults to one. + +Reference-only results are completely unchanged when the flag is omitted, so adding it to an +existing command can only ever add rows, never remove or alter them. `--format json` entries +additionally carry `Depth` (the traversal depth), `Relation` (`Connect`/`Binding` for a +connector, or the reference edge kind otherwise), and `ViaQualifiedName`, so scripts can +distinguish "referenced by" from "connected to" without parsing the human-readable detail text. + ## Query Output Formats | Format | Flag | Notes | @@ -477,7 +515,7 @@ the same order, so either format can be relied on for automated comparisons. | `uses` | yes | What does this element depend on? | | `used-by` | yes | What depends on this element? | | `dependencies` | yes | What does this element depend on, and what depends on it (combined, as prose)? | -| `impact` | yes | What is transitively affected by a change (`--walk-depth` to bound)? | +| `impact` | yes | What is transitively affected by a change (`--walk-depth`, `--include-connections`)? | | `describe` | yes | What is this element (kind, supertypes, typing, annotations, applied metadata, children)? | | `hierarchy` | yes | What is the supertype/subtype tree (`--direction up`\|`down`\|`both`)? | | `requirements` | yes | What satisfy/verify/allocate relationships involve this element? | @@ -495,6 +533,7 @@ the same order, so either format can be relied on for automated comparisons. | `--format markdown\|json` | Output format (default: `markdown`); distinct from `render`'s `--format` (`svg`/`png`) | | `--output ` | Write to this **file** (default: stdout); `render`'s `--output` is a *directory* instead | | `--walk-depth <#>` | Maximum impact-walk depth (`impact` verb only) | +| `--include-connections` | `impact` only: also follow `connect`/`bind` edges, undirected (see above) | | `--direction up\|down\|both` | Traversal direction (`hierarchy` verb only) | | `--kind ` | Element-kind filter (`list`/`find` verbs only) | | `--name ` | Name substring filter (`list`/`find` verbs only) | diff --git a/docs/verification/sysml2-tools-core/query.md b/docs/verification/sysml2-tools-core/query.md index 4c3c519f..8e17fe1b 100644 --- a/docs/verification/sysml2-tools-core/query.md +++ b/docs/verification/sysml2-tools-core/query.md @@ -4,7 +4,8 @@ The Query subsystem is verified by direct tests in `test/DemaConsulting.SysML2Tools.Tests/Query/QueryOmgFixtureTests.cs`, -`QueryRenderingTests.cs`, and `QueryResultExporterTests.cs`, plus dedicated unit tests for the +`QueryRenderingTests.cs`, `QueryEngineImpactTests.cs`, and `QueryResultExporterTests.cs`, plus +dedicated unit tests for the `QualifiedNameShortener` helper in `test/DemaConsulting.SysML2Tools.Tests/Utilities/QualifiedNameShortenerTests.cs`. These tests call `QueryEngine`, `QueryResultRenderer`, `QueryResultExporter`, and `QualifiedNameShortener` @@ -35,7 +36,8 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne ### Acceptance Criteria -- All `QueryOmgFixtureTests`, `QueryRenderingTests`, `QueryResultExporterTests`, and +- All `QueryOmgFixtureTests`, `QueryRenderingTests`, `QueryEngineImpactTests`, + `QueryResultExporterTests`, and `QualifiedNameShortenerTests` pass with zero failures across all three target frameworks. - The `uses`, `used-by`, `dependencies`, `impact`, `interface`, `list`, `find`, and stdlib- filtering behaviors are proven at the `QueryEngine`/`QueryOptions` level by the @@ -62,6 +64,27 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne `describe`), falling back to a generic `**Entries**` / `_No entries._` label for any unrecognized verb. The label is always plain bold text, never an ATX heading, so the report never branches into a Markdown sub-section regardless of the caller's requested heading depth. +- `impact` without `IncludeConnections` reports exactly the reference-only result it reported + before connection-aware impact existed, including for a subject whose only relationship to + the rest of the model is a connector. +- `impact` with `IncludeConnections` reaches the part usage on the far side of a connector, from + either end of that connector, proving the traversal is undirected rather than reverse-only. +- Connector endpoints that are nested ports are attributed to their nearest owning part usage, + with the raw far endpoint preserved in the entry's `ViaQualifiedName` and `Notes`. A far + endpoint that is itself a declared element is reported unchanged, with no roll-up performed + and `ViaQualifiedName` left null, so `impact` and `connections` agree on the same connector's + topology and the enclosing definition that also owns the subject is never reported in the + endpoint's place. +- Connector hops are bounded to one per traversal path when no `WalkDepth` is supplied, and to + `WalkDepth` when it is, while reference-edge depth semantics are unchanged. +- A cyclic connector topology terminates and reports each impacted element exactly once. +- `Binding` connectors (`bind A = B;`) are traversed undirected exactly like `Connect` + connectors. +- The public `QueryEngine.Impact`/`QueryOptions` API delivers all of the above with no CLI + involvement, as consumed by non-CLI clients such as SysML2Workbench. +- Traversal-produced entries carry structured `Depth`, `Relation`, and (where roll-up occurred) + `ViaQualifiedName` values; JSON emits `Relation` as its enum member name, omits all three + when null, and `Detail` remains the same human-readable `"depth N"` text in Markdown. ### Test Scenarios @@ -151,6 +174,52 @@ applies Markdown-only qualified-name shortening. **`RenderJson_NonDependenciesVerb_DirectionFieldOmittedFromOutput`**: Verifies that non- `dependencies` JSON omits the `Direction` property entirely when it is `null`. +**`RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips`**: Renders an impact +entry carrying `Depth`, `Relation`, and `ViaQualifiedName`; verifies that JSON contains +`"Depth": 1` and `"Relation": "Connect"` (the enum member *name*, not its numeric value) and +that all three values round-trip back through `QueryResultSerializerContext`, which also proves +`JsonStringEnumConverter` is compatible with the source-generated context. + +**`RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia`**: Verifies that an entry +produced by a non-traversing verb omits `Depth`, `Relation`, and `ViaQualifiedName` from JSON +entirely rather than serializing them as `null`, so no existing verb's payload shape changes. + +**`RenderMarkdown_ImpactEntry_DetailRemainsHumanReadableDepthText`**: Verifies that an impact +entry carrying structured metadata still renders its `Detail` cell as the plain `depth 1` text +and that the structured `Relation` value does not leak into Markdown output. + +#### QueryEngineImpactTests.cs + +These scenarios exercise the public Core API only — a temp-file workspace loaded through +`WorkspaceLoader`, a hand-constructed `QueryOptions`, and a direct `QueryEngine.Impact` call — +proving that a non-CLI client such as SysML2Workbench obtains connection-aware impact results +and their structured metadata without any dependency on the Tool project. + +**`Impact_IncludeConnections_ThroughPublicApi_ReturnsConnectedPartEntries`**: Runs `Impact` +twice over the same connected-parts fixture, once with `IncludeConnections` unset and once with +it set; verifies the first returns no entries and the second returns the connected hub part +usage. + +**`Impact_ConnectionEntry_RecordsDepthRelationAndViaQualifiedName`**: Verifies that a connection +entry records `Depth` of 1, `Relation` of `SysmlEdgeKind.Connect`, and a `ViaQualifiedName` of +the nested port endpoint the entry was rolled up from. + +**`Impact_ReferenceEntry_RecordsDepthAndReferenceRelation`**: Verifies that a reference entry +records `Depth` of 1 and `Relation` of `SysmlEdgeKind.Supertype`, with no `ViaQualifiedName` +since no roll-up occurred. + +**`Impact_IncludeConnections_DeclaredFarEndpoint_ReportsEndpointItself`**: Uses the +declared-endpoints fixture — sibling part usages `alpha`, `beta`, and `gamma` joined by +`connect alpha to beta;` and `bind beta = gamma;` with no ports anywhere — and queries `alpha` +with `IncludeConnections` set; verifies the impacted entry is `Model::System::beta` itself at +`Depth` 1 with `Relation` of `SysmlEdgeKind.Connect`, and that neither the enclosing +`Model::System` definition nor the subject `Model::System::alpha` appears among the entries. + +**`Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName`**: Uses the same +declared-endpoints fixture and subject; verifies that the `Model::System::beta` entry leaves +`ViaQualifiedName` null because no roll-up occurred, while its `Notes` still name both raw +connector endpoints so no information is lost. + **`WriteMarkdown_HappyPath_MatchesRendererOutput`**: Verifies that `WriteMarkdown` writes the same Markdown text produced by `QueryResultRenderer.RenderMarkdown`. @@ -247,6 +316,52 @@ references. **`Impact_Unbounded_ReachesTransitiveClosure`**: Verifies that unbounded `QueryEngine.Impact` reaches the full transitive closure of incoming references without looping on cycles. +The connection-aware `impact` scenarios below all use the shared `QueryTestFixtures. +GantryConnections` fixture (a minimal reduction of the three-axis-gantry topology: two motor +part usages, each connecting one of its nested ports to a distinct port of a shared hub part +usage) except where a local variant is named. No motor references the other and no motor has an +incoming reference edge, so any element reported was necessarily reached through a connector. + +**`Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult`**: Regression lock for +unchanged default semantics — verifies that `impact` on a motor without the flag reports zero +impacted elements, does not name the connected hub, and does not mention connections in its +summary. + +**`Impact_IncludeConnections_ReachesConnectedSiblingPart`**: Verifies that the same query with +`--include-connections` reaches the connected hub part usage and reports the one-hop connection +bound in its summary. + +**`Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart`**: Undirected proof +— verifies that querying from the hub (the endpoint named second in every connector) reaches +both motors, the mirror image of the previous scenario. + +**`Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage`**: Verifies that the +reported entry is the owning part usage with kind `part` (not the nested port), and that the raw +port-to-port endpoint pair is preserved in the entry's notes. + +**`Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne`**: Verifies that with no +`--walk-depth` supplied the connected hub is reached but the second motor — two connector hops +away through the hub — is not. + +**`Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop`**: Verifies that +`--walk-depth 2` raises the connector hop bound to two, reaching the second motor, and reports +the raised bound in the summary. + +**`Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates`**: Uses a local +variant joining one motor and the hub with two connectors written in opposite textual order, +with `--walk-depth 5`; verifies the query terminates and reports the hub exactly once. + +**`Impact_IncludeConnections_BindingEdges_AreTraversedUndirected`**: Uses a local `bind A = B;` +variant and verifies that each bound part is reachable from the other, proving `Binding` edges +are traversed exactly like `Connect` edges. + +**`Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition`**: +Uses the `QueryTestFixtures.DeclaredEndpointConnections` fixture, in which every connector and +binding names a directly declared sibling part usage rather than a nested port, and queries +`Model::System::alpha` with `--include-connections`; verifies the rendered table contains the +`| Model::System::beta | part |` row and never the `| Model::System | part def |` row for the +enclosing definition. + **`Interface_ReportsPortsAndTypedFeatures`**: Verifies that `QueryEngine.Interface` reports a target definition's ports and typed features. diff --git a/docs/verification/sysml2-tools-tool/query.md b/docs/verification/sysml2-tools-tool/query.md index 7a64a7ee..34508968 100644 --- a/docs/verification/sysml2-tools-tool/query.md +++ b/docs/verification/sysml2-tools-tool/query.md @@ -36,6 +36,9 @@ renderer behavior, and Core-side file-export helper behavior are verified separa - `--output ` writes the rendered document to the named file instead of stdout. - Help output remains localized through `QueryStrings`, including the workflow note, example invocations, schema hints, and the `--output` help text. +- `--include-connections` is accepted by the CLI without any Tool-side parsing (it is forwarded + verbatim to Core's parser) and sets `QueryOptions.IncludeConnections`, and it is documented in + `impact` verb help through `QueryStrings`. - Error paths are covered: no input files, patterns supplied but none matching on disk, target element not found, invalid `--walk-depth`, invalid `--format`, and parse-error-containing input files that still complete best-effort. @@ -103,6 +106,11 @@ Markdown/JSON schema hints for every verb. Verifies that `query uses --help` prints verb-specific help and returns exit code 0 without requiring `--element`. +#### QuerySubsystem_ImpactVerbHelp_MentionsIncludeConnectionsOption + +Verifies that `query impact --help` prints the `--include-connections` option line, keeping the +Tool's resx-sourced help text in lockstep with the grammar Core's parser actually accepts. + #### Dependencies_DepthAndHeadingOptions_ApplyToHeadingLikeOtherVerbs Verifies end-to-end that the CLI passes heading-depth and heading-text options through to the @@ -153,6 +161,10 @@ leaves `Context.Query` null so the general-help path can run without a fake verb **`Context_Create_QueryCommand_WithIncludeStdlibFlag_SetsIncludeStdlibTrue`**: Verify parsing of query-specific option fields. +**`Context_Create_QueryCommand_WithIncludeConnectionsFlag_SetsIncludeConnectionsTrue`**: +Verifies that `--include-connections` sets `Query.IncludeConnections`, proving the Tool CLI +forwards the flag to Core's parser without any Tool-side parsing of its own. + **`Context_Create_QueryCommand_WithFormatMarkdown_SetsQueryFormat`** / **`Context_Create_QueryCommand_WithFormatJson_SetsQueryFormat`**: Verify that query's `--format` is parsed independently of render's `--format`. diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/NamespaceDoc.cs b/src/DemaConsulting.SysML2Tools.Core/Query/NamespaceDoc.cs index 8e3b4712..24c993c7 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/NamespaceDoc.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/NamespaceDoc.cs @@ -47,6 +47,33 @@ namespace DemaConsulting.SysML2Tools.Query; /// QueryResultExporter.WriteMarkdown(result, "uses.md"); /// /// +/// +/// A non-CLI application (for example, SysML2Workbench) uses the same API without any console +/// or file I/O, and reads the structured traversal metadata on each entry rather than parsing +/// the human-readable text: +/// +/// var (stdlibTable, _) = StdlibProvider.GetSymbolTable(); +/// var loadResult = await WorkspaceLoader.LoadAsync(["Model.sysml"], stdlibTable); +/// var workspace = loadResult.Workspace!; +/// +/// var options = new QueryOptions +/// { +/// Verb = QueryVerb.Impact, +/// Element = "Model::System::motorA", +/// IncludeConnections = true +/// }; +/// workspace.Declarations.TryGetValue(options.Element!, out var element); +/// +/// var result = QueryEngine.Impact(workspace, element!, options); +/// foreach (var entry in result.Entries) +/// { +/// // entry.Depth : int? - traversal depth; never parse "depth N" from Detail +/// // entry.Relation : SysmlEdgeKind? - Supertype/Typing/... versus Connect/Binding +/// // entry.ViaQualifiedName : string? - the far port a connection entry rolled up from +/// Console.WriteLine($"{entry.QualifiedName} {entry.Relation} @{entry.Depth} via {entry.ViaQualifiedName}"); +/// } +/// +/// internal static class NamespaceDoc { } diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryArgumentParser.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryArgumentParser.cs index b4ac792c..333e1583 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryArgumentParser.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryArgumentParser.cs @@ -17,7 +17,8 @@ namespace DemaConsulting.SysML2Tools.Query; /// verb is present and help was not requested, a clear is /// thrown rather than leaving the caller in a silent null/None state. Remaining tokens /// recognize --element/-e, --direction, --kind, --name, -/// --include-stdlib, --format, --walk-depth, and --heading, plus +/// --include-stdlib, --include-connections, --format, +/// --walk-depth, and --heading, plus /// positional file glob patterns returned separately (this type has no file-glob or CLI-I/O /// concept of its own — does not carry an input-files property); /// any other --prefixed token is rejected, including --output, which is a @@ -89,6 +90,7 @@ public static (QueryOptions? Options, IReadOnlyList Files) Parse( int? walkDepth = null; string? heading = null; var includeStdlib = false; + var includeConnections = false; var files = new List(); while (index < commandArgs.Count) @@ -136,6 +138,10 @@ public static (QueryOptions? Options, IReadOnlyList Files) Parse( includeStdlib = true; break; + case "--include-connections": + includeConnections = true; + break; + default: if (arg.StartsWith("-", StringComparison.Ordinal)) { @@ -158,6 +164,7 @@ public static (QueryOptions? Options, IReadOnlyList Files) Parse( Kind = kind, NameFilter = nameFilter, IncludeStdlib = includeStdlib, + IncludeConnections = includeConnections, Heading = heading }; diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs index c4571326..115339cc 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs @@ -28,6 +28,33 @@ public static class QueryEngine SysmlEdgeKind.Allocate ]; + /// + /// The connector edge kinds reported by the connections verb. + /// + private static readonly SysmlEdgeKind[] ConnectionsVerbEdgeKinds = [SysmlEdgeKind.Connect]; + + /// + /// The connector edge kinds traversed by the impact verb when + /// is set. Both kinds join two endpoints + /// that carry no semantic source-causes-target direction, so both are traversed + /// undirected. + /// + private static readonly SysmlEdgeKind[] ImpactConnectorEdgeKinds = + [ + SysmlEdgeKind.Connect, + SysmlEdgeKind.Binding + ]; + + /// + /// Default maximum number of connector hops per traversal path when + /// is set but no explicit + /// was supplied. Connector graphs in real models + /// are dense meshes (every port of every part joined to every port of a hub), so an + /// unbounded connector closure degenerates to "the whole assembly" and answers nothing + /// useful. + /// + private const int DefaultConnectionHopLimit = 1; + /// /// Dispatches to the verb method selected by , the single /// entry point library callers can use instead of writing their own 12-arm switch (this is @@ -199,48 +226,52 @@ public static QueryResult Dependencies(SysmlWorkspace workspace, SysmlNode eleme /// /// Reports the transitive "blast radius" of a change to a given element: the transitive /// closure of , bounded by when - /// specified (unlimited otherwise). + /// specified (unlimited otherwise), optionally extended with undirected connector + /// (connect/bind) traversal when + /// is set. /// /// The loaded workspace. /// The target element. /// The parsed query options. /// The query result. + /// + /// Two independent bounds are applied, because they differ when + /// is . The reference-edge + /// closure keeps its existing semantics exactly ( means + /// unlimited), enforced by the outer breadth-first loop. Connector hops are bounded per + /// traversal path by or, when that is + /// , by — so the outer + /// loop cannot enforce it and a per-frontier-item hop counter is carried instead. + /// public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, QueryOptions options) { var qualifiedName = QualifiedNameOf(element, options); var visited = new HashSet(StringComparer.Ordinal) { qualifiedName }; var entries = new List(); - var frontier = new List { qualifiedName }; + + // Collected once per call, and only when requested, rather than once per frontier item. + List<(string Source, string Target, string Keyword, SysmlEdgeKind Kind)> connectorEdges = + options.IncludeConnections + ? CollectConnectorEdges(workspace, ImpactConnectorEdgeKinds) + : []; + var connectionHopLimit = options.WalkDepth ?? DefaultConnectionHopLimit; + + var frontier = new List<(string Name, int ConnectionHops)> { (qualifiedName, 0) }; var depth = 0; while (frontier.Count > 0 && (options.WalkDepth is not { } maxDepth || depth < maxDepth)) { depth++; - var next = new List(); + var next = new List<(string Name, int ConnectionHops)>(); - foreach (var current in frontier) + foreach (var (current, hops) in frontier) { - foreach (var edge in workspace.Index.GetIncomingEdges(current)) - { - if (edge.SourceQualifiedName is not { Length: > 0 } source || !visited.Add(source)) - { - continue; - } - - next.Add(source); - - if (!IsVisible(source, workspace, options.IncludeStdlib)) - { - continue; - } + CollectImpactReferences(workspace, options, current, hops, depth, visited, entries, next); - workspace.Declarations.TryGetValue(source, out var sourceNode); - entries.Add(new QueryResultEntry - { - QualifiedName = source, - Kind = sourceNode is not null ? DescribeKind(sourceNode) : EdgeKindLabel(edge.Kind), - Detail = $"depth {depth}" - }); + if (options.IncludeConnections && hops < connectionHopLimit) + { + CollectImpactConnections( + workspace, options, current, hops, depth, connectorEdges, visited, entries, next); } } @@ -248,15 +279,139 @@ public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, Qu } var depthSuffix = options.WalkDepth is { } d ? $" (depth <= {d})" : string.Empty; + var connectionSuffix = options.IncludeConnections + ? $", including connections (connection hops <= {connectionHopLimit})" + : string.Empty; return new QueryResult { Verb = "impact", Element = qualifiedName, - Summary = [$"{entries.Count} element(s) transitively impacted by a change to '{qualifiedName}'{depthSuffix}."], + Summary = + [ + $"{entries.Count} element(s) transitively impacted by a change to " + + $"'{qualifiedName}'{depthSuffix}{connectionSuffix}." + ], Entries = entries }; } + /// + /// Expands one impact frontier item over its incoming reference edges — the original, + /// always-on reverse closure — appending newly-reached names to + /// with their connector-hop count carried through unchanged + /// (a reference hop never consumes a connector hop). + /// + /// The loaded workspace. + /// The parsed query options. + /// The frontier item's qualified name. + /// The number of connector hops already taken to reach . + /// The 1-based traversal depth of the names being reached. + /// The shared cycle guard. + /// The result entries accumulated so far. + /// The next frontier being built. + private static void CollectImpactReferences( + SysmlWorkspace workspace, + QueryOptions options, + string current, + int hops, + int depth, + HashSet visited, + List entries, + List<(string Name, int ConnectionHops)> next) + { + foreach (var edge in workspace.Index.GetIncomingEdges(current)) + { + if (edge.SourceQualifiedName is not { Length: > 0 } source || !visited.Add(source)) + { + continue; + } + + next.Add((source, hops)); + + if (!IsVisible(source, workspace, options.IncludeStdlib)) + { + continue; + } + + workspace.Declarations.TryGetValue(source, out var sourceNode); + entries.Add(new QueryResultEntry + { + QualifiedName = source, + Kind = sourceNode is not null ? DescribeKind(sourceNode) : EdgeKindLabel(edge.Kind), + Detail = $"depth {depth}", + Depth = depth, + Relation = edge.Kind + }); + } + } + + /// + /// Expands one impact frontier item over connector edges, undirected: a connector is + /// followed whenever exactly one of its two endpoints is the frontier item itself or a + /// feature nested inside it, and the other endpoint is rolled up to its nearest owning + /// declaration. Consumes one connector hop per reached element. + /// + /// The loaded workspace. + /// The parsed query options. + /// The frontier item's qualified name. + /// The number of connector hops already taken to reach . + /// The 1-based traversal depth of the names being reached. + /// The connector edges collected once for this invocation. + /// The shared cycle guard. + /// The result entries accumulated so far. + /// The next frontier being built. + private static void CollectImpactConnections( + SysmlWorkspace workspace, + QueryOptions options, + string current, + int hops, + int depth, + List<(string Source, string Target, string Keyword, SysmlEdgeKind Kind)> connectorEdges, + HashSet visited, + List entries, + List<(string Name, int ConnectionHops)> next) + { + foreach (var (source, target, keyword, kind) in connectorEdges) + { + var nearIsSource = IsSelfOrNestedUnder(source, current); + var nearIsTarget = IsSelfOrNestedUnder(target, current); + + // Rejects both "neither end belongs to the subject" and the self-loop case where + // both ends are nested under it (which would otherwise report the subject itself). + if (nearIsSource == nearIsTarget) + { + continue; + } + + var near = nearIsSource ? source : target; + var far = nearIsSource ? target : source; + var owner = RollUpToNearestDeclaration(workspace, far); + if (!visited.Add(owner)) + { + continue; + } + + next.Add((owner, hops + 1)); + + if (!IsVisible(owner, workspace, options.IncludeStdlib)) + { + continue; + } + + workspace.Declarations.TryGetValue(owner, out var ownerNode); + entries.Add(new QueryResultEntry + { + QualifiedName = owner, + Kind = ownerNode is not null ? DescribeKind(ownerNode) : keyword, + Detail = $"depth {depth}", + Notes = [$"connected via {keyword}: {near} -> {far}"], + Depth = depth, + Relation = kind, + ViaQualifiedName = string.Equals(owner, far, StringComparison.Ordinal) ? null : far + }); + } + } + /// /// Describes a single element in detail: kind, qualified name, supertypes, typing, /// annotations (comments/documentation), applied metadata annotations (type and attribute @@ -509,16 +664,13 @@ public static QueryResult Interface(SysmlWorkspace workspace, SysmlNode element, public static QueryResult Connections(SysmlWorkspace workspace, SysmlNode element, QueryOptions options) { var qualifiedName = QualifiedNameOf(element, options); - var prefix = qualifiedName + "::"; - var connectEdges = CollectConnectEdges(workspace); + var connectEdges = CollectConnectorEdges(workspace, ConnectionsVerbEdgeKinds); var entries = new List(); - bool Matches(string? name) => name is not null && (name == qualifiedName || name.StartsWith(prefix, StringComparison.Ordinal)); - - foreach (var (source, target, keyword) in connectEdges) + foreach (var (source, target, keyword, _) in connectEdges) { - var sourceMatches = Matches(source); - var targetMatches = Matches(target); + var sourceMatches = IsSelfOrNestedUnder(source, qualifiedName); + var targetMatches = IsSelfOrNestedUnder(target, qualifiedName); if (!sourceMatches && !targetMatches) { continue; @@ -744,7 +896,13 @@ private static void WalkHierarchy( if (IsVisible(next, workspace, options.IncludeStdlib)) { - entries.Add(new QueryResultEntry { QualifiedName = next, Kind = label, Detail = $"depth {depth}" }); + entries.Add(new QueryResultEntry + { + QualifiedName = next, + Kind = label, + Detail = $"depth {depth}", + Depth = depth + }); } WalkHierarchy(workspace, next, options, entries, visited, depth + 1, label, getEdges, selectNext); @@ -775,19 +933,32 @@ private static bool MatchesFilters(SysmlNode node, string qualifiedName, QueryOp } /// - /// Collects every resolved edge in the workspace, - /// together with its originating connector's keyword (connect, connection, - /// or message), by walking every node reachable from - /// and reading each connector node's own - /// (populated in-place by ReferenceResolver - /// regardless of whether the connector node itself is named). Connect edges are not - /// exposed via , so this walk is required. + /// Collects every resolved connector edge in the workspace whose kind is in + /// , together with its originating connector's keyword + /// (connect, connection, message, or bind), by walking every + /// node reachable from and reading each + /// connector node's own (populated in-place by + /// ReferenceResolver regardless of whether the connector node itself is named). /// /// The loaded workspace. - /// The list of resolved connect edges with their originating keyword. - private static List<(string Source, string Target, string Keyword)> CollectConnectEdges(SysmlWorkspace workspace) + /// The connector edge kinds to collect. + /// The list of resolved connector edges with their originating keyword and kind. + /// + /// The connector edges themselves are present in + /// (ReferenceResolver's feature-chain + /// resolution pass appends them to the aggregate edge list that builds the index). The + /// node walk is nevertheless required because a carries only + /// (Source, Target, Kind) and not the originating connector's keyword, which the + /// connections verb reports as each entry's Kind and which the + /// impact verb reports in each connection entry's notes. Sharing this one + /// collector between both verbs also guarantees they can never disagree about the + /// workspace's connection topology. + /// + private static List<(string Source, string Target, string Keyword, SysmlEdgeKind Kind)> CollectConnectorEdges( + SysmlWorkspace workspace, + IReadOnlyList kinds) { - var results = new List<(string, string, string)>(); + var results = new List<(string, string, string, SysmlEdgeKind)>(); var visited = new HashSet(); void Walk(SysmlNode node) @@ -800,9 +971,9 @@ void Walk(SysmlNode node) var keyword = node is SysmlConnectionNode connection ? connection.ConnectionKeyword : "connect"; foreach (var edge in node.ResolvedEdges) { - if (edge.Kind == SysmlEdgeKind.Connect && edge.SourceQualifiedName is { Length: > 0 } source) + if (kinds.Contains(edge.Kind) && edge.SourceQualifiedName is { Length: > 0 } source) { - results.Add((source, edge.TargetQualifiedName, keyword)); + results.Add((source, edge.TargetQualifiedName, keyword, edge.Kind)); } } @@ -820,6 +991,55 @@ void Walk(SysmlNode node) return results; } + /// + /// Determines whether is the subject element itself or a + /// feature nested (at any depth) inside it, using the qualified-name containment prefix + /// rule. Shared by and so both verbs + /// agree on what "belongs to this element" means. + /// + /// The candidate qualified name (typically a connector endpoint). + /// The subject element's qualified name. + /// when the candidate is the subject or nested under it. + private static bool IsSelfOrNestedUnder(string? candidate, string subjectQualifiedName) => + candidate is not null && + (string.Equals(candidate, subjectQualifiedName, StringComparison.Ordinal) || + candidate.StartsWith(subjectQualifiedName + "::", StringComparison.Ordinal)); + + /// + /// Rolls a connector endpoint up to its nearest owning declaration. The endpoint itself + /// is probed first and returned unchanged when it is itself present in + /// (for example a directly connected sibling + /// part usage). Only endpoints absent from + /// (frequently ports inherited through a typed usage) have trailing :: segments + /// stripped until a declared qualified name is found. + /// + /// The loaded workspace. + /// The connector endpoint's qualified name. + /// + /// itself when it is declared, otherwise the nearest + /// declared owning qualified name, or unchanged when + /// no ancestor is declared, so a connection is never silently dropped. + /// + private static string RollUpToNearestDeclaration(SysmlWorkspace workspace, string qualifiedName) + { + var probe = qualifiedName; + while (true) + { + if (workspace.Declarations.ContainsKey(probe)) + { + return probe; + } + + var index = probe.LastIndexOf("::", StringComparison.Ordinal); + if (index < 0) + { + return qualifiedName; + } + + probe = probe[..index]; + } + } + /// /// Recursively collects state entries (child with /// FeatureKeyword == "state") and transition entries (child diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs index 334c0092..f42e0d85 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryOptions.cs @@ -47,7 +47,11 @@ public sealed record QueryOptions /// /// /// Only meaningful for , where it bounds the transitive - /// impact walk; means unlimited. + /// impact walk; means unlimited. When + /// is also set, this value additionally bounds the + /// number of connector hops taken along any single traversal path, and + /// then means "one connector hop" rather than "unlimited" (see + /// ). /// public int? WalkDepth { get; init; } @@ -98,4 +102,23 @@ public sealed record QueryOptions /// excluded from results unless explicitly requested). /// public bool IncludeStdlib { get; init; } + + /// + /// Gets a value indicating whether the impact walk should also follow connector + /// (connect/bind) relationships in addition to resolved reference edges. + /// + /// + /// Only meaningful for ; defaults to + /// , so the default impact semantics (a reverse-only + /// closure over resolved reference edges) are unchanged. When set, Connect and + /// Binding semantic edges are traversed undirected (a connector's two + /// ends carry no semantic + /// source-causes-target direction), and endpoints are rolled up through containment in + /// both directions so a part subject matches a connector attached to one of its + /// nested ports and a far-side port is attributed to its nearest owning declaration. + /// Because connector graphs are dense meshes, connector hops along a single traversal + /// path are bounded by when supplied, and by one hop when + /// is . + /// + public bool IncludeConnections { get; init; } } diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResult.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResult.cs index e1d29152..34f2c21c 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResult.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResult.cs @@ -3,6 +3,7 @@ // using System.Text.Json.Serialization; +using DemaConsulting.SysML2Tools.Semantic.Model; namespace DemaConsulting.SysML2Tools.Query; @@ -82,6 +83,38 @@ public sealed record QueryResultEntry /// [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] public QueryEntryDirection? Direction { get; init; } + + /// + /// Gets the 1-based traversal depth at which this entry was reached, or + /// for verbs that do not traverse. This is the authoritative, + /// machine-readable counterpart to the human-readable "depth N" text carried in + /// ; API consumers shall read this property rather than parsing + /// . Omitted from JSON output entirely when . + /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public int? Depth { get; init; } + + /// + /// Gets the resolved semantic edge kind that reached this entry (e.g. + /// for a definitional reference, + /// for a connector), or when + /// the entry was not produced by traversing a resolved edge. Serialized as its enum + /// member name (e.g. "Connect") so the JSON contract is immune to member + /// reordering, and omitted from JSON output entirely when . + /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + [JsonConverter(typeof(JsonStringEnumConverter))] + public SysmlEdgeKind? Relation { get; init; } + + /// + /// Gets the qualified name of the actual far endpoint that attributed this entry to + /// — for connection roll-up, the nested port the connector + /// actually reached, whose nearest owning declaration is reported as + /// . when no roll-up occurred. + /// Omitted from JSON output entirely when . + /// + [JsonIgnore(Condition = JsonIgnoreCondition.WhenWritingNull)] + public string? ViaQualifiedName { get; init; } } /// diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultSerializerContext.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultSerializerContext.cs index c69d7537..0d8aa967 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultSerializerContext.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryResultSerializerContext.cs @@ -13,6 +13,7 @@ namespace DemaConsulting.SysML2Tools.Query; /// project. /// [JsonSerializable(typeof(QueryResult))] +[JsonSerializable(typeof(QueryResultEntry))] [JsonSourceGenerationOptions(WriteIndented = true)] public partial class QueryResultSerializerContext : JsonSerializerContext { diff --git a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryCommand.cs b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryCommand.cs index 5d255c75..5f58f783 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryCommand.cs +++ b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryCommand.cs @@ -235,6 +235,7 @@ public static void PrintGeneralHelp(Context context) context.WriteLine(QueryStrings.Query_GeneralOptionKind); context.WriteLine(QueryStrings.Query_GeneralOptionName); context.WriteLine(QueryStrings.Query_GeneralOptionIncludeStdlib); + context.WriteLine(QueryStrings.Query_GeneralOptionIncludeConnections); context.WriteLine(QueryStrings.Query_GeneralOptionOutput1); context.WriteLine(QueryStrings.Query_GeneralOptionOutput2); context.WriteLine(""); @@ -270,6 +271,9 @@ public static void PrintVerbHelp(Context context, QueryVerb verb) { case QueryVerb.Impact: context.WriteLine(QueryStrings.Query_OptionWalkDepthImpact); + context.WriteLine(QueryStrings.Query_OptionIncludeConnectionsImpact1); + context.WriteLine(QueryStrings.Query_OptionIncludeConnectionsImpact2); + context.WriteLine(QueryStrings.Query_OptionIncludeConnectionsImpact3); break; case QueryVerb.Dependencies: diff --git a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.cs b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.cs index aa16198a..b6776bc4 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.cs +++ b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.cs @@ -128,6 +128,9 @@ internal static class QueryStrings /// Gets the general --include-stdlib option line. public static string Query_GeneralOptionIncludeStdlib => ResourceManager.GetString(nameof(Query_GeneralOptionIncludeStdlib))!; + /// Gets the general --include-connections option line. + public static string Query_GeneralOptionIncludeConnections => ResourceManager.GetString(nameof(Query_GeneralOptionIncludeConnections))!; + /// Gets the first line of the general --output option description. public static string Query_GeneralOptionOutput1 => ResourceManager.GetString(nameof(Query_GeneralOptionOutput1))!; @@ -161,6 +164,15 @@ internal static class QueryStrings /// Gets the --walk-depth option line shown for the 'impact' verb. public static string Query_OptionWalkDepthImpact => ResourceManager.GetString(nameof(Query_OptionWalkDepthImpact))!; + /// Gets the first line of the --include-connections option shown for the 'impact' verb. + public static string Query_OptionIncludeConnectionsImpact1 => ResourceManager.GetString(nameof(Query_OptionIncludeConnectionsImpact1))!; + + /// Gets the second line of the --include-connections option shown for the 'impact' verb. + public static string Query_OptionIncludeConnectionsImpact2 => ResourceManager.GetString(nameof(Query_OptionIncludeConnectionsImpact2))!; + + /// Gets the third line of the --include-connections option shown for the 'impact' verb. + public static string Query_OptionIncludeConnectionsImpact3 => ResourceManager.GetString(nameof(Query_OptionIncludeConnectionsImpact3))!; + /// Gets the --direction option line shown for the 'hierarchy' verb. public static string Query_OptionDirectionHierarchy => ResourceManager.GetString(nameof(Query_OptionDirectionHierarchy))!; diff --git a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx index 5beed964..cb8aac84 100644 --- a/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx +++ b/src/DemaConsulting.SysML2Tools.Tool/Query/QueryStrings.resx @@ -148,6 +148,9 @@ --include-stdlib Include OMG standard library elements in results + + --include-connections Also follow connect/bind edges ('impact' verb only) + --output <file> Output file path (default: stdout). Names a FILE, @@ -181,6 +184,15 @@ --walk-depth <#> Maximum impact-walk depth (default: unlimited) + + --include-connections Also follow connect/bind edges, undirected + + + (default: reference edges only). Connector + + + hops are bounded by --walk-depth, else 1. + --direction up|down|both Traversal direction (default: both) diff --git a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs new file mode 100644 index 00000000..e628bfea --- /dev/null +++ b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs @@ -0,0 +1,226 @@ +// +// Copyright (c) DemaConsulting. All rights reserved. +// + +using DemaConsulting.SysML2Tools.Query; +using DemaConsulting.SysML2Tools.Semantic; +using DemaConsulting.SysML2Tools.Semantic.Model; +using DemaConsulting.SysML2Tools.Stdlib; + +namespace DemaConsulting.SysML2Tools.Tests.Query; + +/// +/// API-only suite for connection-aware impact analysis and the structured traversal +/// metadata carried on each . Every test here follows the +/// non-CLI client pattern (as used by SysML2Workbench): load a workspace through +/// , construct a , call +/// , and read the structured entry properties — never the CLI, and +/// never the human-readable text. +/// +[Collection("Sequential")] +public class QueryEngineImpactTests +{ + /// + /// Minimal connector topology: two motor part usages each connect one of their nested + /// ports to a distinct port of a shared hub part usage, with a supertype reference + /// between two definitions so both the reference and connector branches can be observed. + /// + private const string ConnectedPartsFixture = """ + package Model { + part def Hub { + port J1; + port J2; + } + + part def Motor { + port power; + port encoder; + } + + part def ServoMotor specializes Motor; + + part def System { + part hub : Hub; + part motorA : Motor; + part motorB : Motor; + + connect motorA.power to hub.J1; + connect motorB.power to hub.J2; + } + } + """; + + /// + /// Connector topology whose endpoints are themselves declared part usages rather than + /// nested ports, so the far-endpoint attribution has no ancestor to roll up to and must + /// report the endpoint itself. + /// + private const string DeclaredEndpointsFixture = """ + package Model { + part def System { + part alpha; + part beta; + part gamma; + + connect alpha to beta; + bind beta = gamma; + } + } + """; + + /// + /// Writes the fixture to a temp file, loads it exactly as a non-CLI library caller + /// would, resolves the named element, and returns both for a direct + /// call. + /// + /// The inline SysML source to load. + /// The qualified name of the element to resolve. + /// The loaded workspace and the resolved element. + private static async Task<(SysmlWorkspace Workspace, SysmlNode Element)> LoadAsync( + string sysml, string qualifiedName) + { + var tempFile = Path.GetTempFileName() + ".sysml"; + try + { + await File.WriteAllTextAsync(tempFile, sysml, TestContext.Current.CancellationToken); + + var (stdlibTable, _) = StdlibProvider.GetSymbolTable(); + var loadResult = await WorkspaceLoader.LoadAsync([tempFile], stdlibTable); + + Assert.NotNull(loadResult.Workspace); + Assert.True( + loadResult.Workspace.Declarations.TryGetValue(qualifiedName, out var element), + $"'{qualifiedName}' was not found in the loaded workspace."); + + return (loadResult.Workspace, element); + } + finally + { + File.Delete(tempFile); + } + } + + /// + /// A non-CLI caller enables connection-aware impact purely through + /// and receives the connected part usage as + /// a result entry, with no CLI parsing or console output involved. + /// + [Fact] + public async Task Impact_IncludeConnections_ThroughPublicApi_ReturnsConnectedPartEntries() + { + var (workspace, element) = await LoadAsync(ConnectedPartsFixture, "Model::System::motorA"); + + var withoutConnections = QueryEngine.Impact( + workspace, element, new QueryOptions { Verb = QueryVerb.Impact, Element = "Model::System::motorA" }); + var withConnections = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::motorA", + IncludeConnections = true + }); + + Assert.Empty(withoutConnections.Entries); + Assert.Contains(withConnections.Entries, e => e.QualifiedName == "Model::System::hub"); + } + + /// + /// A connection entry carries machine-readable depth, relation, and far-endpoint + /// metadata, so a client never has to parse the free-form detail text. + /// + [Fact] + public async Task Impact_ConnectionEntry_RecordsDepthRelationAndViaQualifiedName() + { + var (workspace, element) = await LoadAsync(ConnectedPartsFixture, "Model::System::motorA"); + + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::motorA", + IncludeConnections = true + }); + + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::hub"); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.Equal("Model::System::hub::J1", entry.ViaQualifiedName); + } + + /// + /// A reference entry carries the same structured depth metadata and reports the resolved + /// reference edge kind that reached it, with no far-endpoint roll-up recorded. + /// + [Fact] + public async Task Impact_ReferenceEntry_RecordsDepthAndReferenceRelation() + { + var (workspace, element) = await LoadAsync(ConnectedPartsFixture, "Model::Motor"); + + var result = QueryEngine.Impact( + workspace, element, new QueryOptions { Verb = QueryVerb.Impact, Element = "Model::Motor" }); + + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::ServoMotor"); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Supertype, entry.Relation); + Assert.Null(entry.ViaQualifiedName); + } + + /// + /// When a connector's far endpoint is itself a declared element, the endpoint is reported + /// as the impacted item. Neither the enclosing definition that also owns the subject nor + /// the subject itself is ever reported, so impact agrees with the topology that the + /// connections verb reports for the same connector. + /// + [Fact] + public async Task Impact_IncludeConnections_DeclaredFarEndpoint_ReportsEndpointItself() + { + var (workspace, element) = await LoadAsync(DeclaredEndpointsFixture, "Model::System::alpha"); + + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::alpha", + IncludeConnections = true + }); + + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::beta"); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::System"); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::System::alpha"); + } + + /// + /// A connection entry whose far endpoint required no roll-up omits + /// , honoring the documented "null when no + /// roll-up occurred" contract, while the notes still name the connector's raw endpoints so + /// no information is lost. + /// + [Fact] + public async Task Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName() + { + var (workspace, element) = await LoadAsync(DeclaredEndpointsFixture, "Model::System::alpha"); + + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::alpha", + IncludeConnections = true + }); + + var entry = Assert.Single(result.Entries, e => e.QualifiedName == "Model::System::beta"); + Assert.Null(entry.ViaQualifiedName); + Assert.Contains(entry.Notes, n => n.Contains("Model::System::beta", StringComparison.Ordinal)); + Assert.Contains(entry.Notes, n => n.Contains("Model::System::alpha", StringComparison.Ordinal)); + } +} diff --git a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs index 2d617354..db9e26c7 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryRenderingTests.cs @@ -4,6 +4,7 @@ using System.Text.Json; using DemaConsulting.SysML2Tools.Query; +using DemaConsulting.SysML2Tools.Semantic.Model; namespace DemaConsulting.SysML2Tools.Tests.Query; @@ -429,4 +430,95 @@ public void RenderJson_NonDependenciesVerb_DirectionFieldOmittedFromOutput() Assert.DoesNotContain("Direction", json); } + + /// + /// An entry carrying traversal metadata serializes Depth, Relation, and + /// ViaQualifiedName, with Relation written as its enum member name so the + /// JSON contract is immune to SysmlEdgeKind member reordering, and round-trips + /// all three back through the source-generated serializer context. + /// + [Fact] + public void RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips() + { + var result = new QueryResult + { + Verb = "impact", + Element = "Model::System::motorA", + Entries = + [ + new QueryResultEntry + { + QualifiedName = "Model::System::hub", + Kind = "part", + Detail = "depth 1", + Depth = 1, + Relation = SysmlEdgeKind.Connect, + ViaQualifiedName = "Model::System::hub::J1" + } + ] + }; + + var json = QueryResultRenderer.RenderJson(result); + var deserialized = JsonSerializer.Deserialize(json, QueryResultSerializerContext.Default.QueryResult); + + Assert.Contains("\"Depth\": 1", json); + Assert.Contains("\"Relation\": \"Connect\"", json); + Assert.NotNull(deserialized); + Assert.Equal(1, deserialized.Entries[0].Depth); + Assert.Equal(SysmlEdgeKind.Connect, deserialized.Entries[0].Relation); + Assert.Equal("Model::System::hub::J1", deserialized.Entries[0].ViaQualifiedName); + } + + /// + /// Regression test: entries produced by non-traversing verbs omit all three traversal + /// metadata properties from JSON output entirely (rather than serializing them as + /// null), so adding them does not change any existing verb's JSON output shape. + /// + [Fact] + public void RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia() + { + var result = new QueryResult + { + Verb = "uses", + Element = "Model::Foo", + Entries = [new QueryResultEntry { QualifiedName = "Model::Bar", Kind = "supertype" }] + }; + + var json = QueryResultRenderer.RenderJson(result); + + Assert.DoesNotContain("Depth", json); + Assert.DoesNotContain("Relation", json); + Assert.DoesNotContain("ViaQualifiedName", json); + } + + /// + /// The structured metadata is additive only: an impact entry's Detail text stays + /// the same human-readable "depth N" string in Markdown output, so CLI output remains + /// friendly and existing Markdown consumers are unaffected. + /// + [Fact] + public void RenderMarkdown_ImpactEntry_DetailRemainsHumanReadableDepthText() + { + var result = new QueryResult + { + Verb = "impact", + Element = "Model::System::motorA", + Entries = + [ + new QueryResultEntry + { + QualifiedName = "Model::System::hub", + Kind = "part", + Detail = "depth 1", + Depth = 1, + Relation = SysmlEdgeKind.Connect + } + ] + }; + + var lines = QueryResultRenderer.RenderMarkdown(result); + + Assert.Contains(lines, l => l.Contains("| Model::System::hub | part | depth 1 |")); + Assert.DoesNotContain(lines, l => l.Contains("Connect")); + } } diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Cli/ContextTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Cli/ContextTests.cs index 4c3887bb..d6a543a8 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Cli/ContextTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Cli/ContextTests.cs @@ -879,6 +879,22 @@ public void Context_Create_QueryCommand_WithIncludeStdlibFlag_SetsIncludeStdlibT Assert.True(context.Query.IncludeStdlib); } + /// + /// Test creating a context with the query command and --include-connections sets + /// Query.IncludeConnections to true, proving the Tool CLI forwards the flag to Core's + /// parser without any Tool-side parsing of its own. + /// + [Fact] + public void Context_Create_QueryCommand_WithIncludeConnectionsFlag_SetsIncludeConnectionsTrue() + { + // Act: execute the operation being tested + using var context = Context.Create(["query", "impact", "--include-connections"]); + + // Assert: verify expected behavior + Assert.NotNull(context.Query); + Assert.True(context.Query.IncludeConnections); + } + /// /// Test creating a context with the query command and --format markdown sets /// Query.Format independently of the render command's --format (context.Render is null diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs index 7667b8bb..42be84f5 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs @@ -431,6 +431,34 @@ public async Task QuerySubsystem_QueryVerbHelp_WithVerb_PrintsVerbHelpWithoutThr } } + /// + /// 'query impact --help' documents the --include-connections option, keeping the Core + /// parser's accepted grammar and the Tool's resx-sourced help text in lockstep. + /// + [Fact] + public async Task QuerySubsystem_ImpactVerbHelp_MentionsIncludeConnectionsOption() + { + // Arrange + var originalOut = Console.Out; + try + { + using var outWriter = new StringWriter(); + Console.SetOut(outWriter); + + // Act + using var context = Context.Create(["query", "impact", "--help"]); + await Program.RunAsync(context); + + // Assert + Assert.Contains("--include-connections", outWriter.ToString()); + Assert.Equal(0, context.ExitCode); + } + finally + { + Console.SetOut(originalOut); + } + } + /// /// 'dependencies' end-to-end: '--depth'/'--heading' apply to its heading line exactly /// like every other verb, while the bullet-prose body below is unaffected. diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs index 27b2e3f2..fc794ea1 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs @@ -29,6 +29,58 @@ namespace DemaConsulting.SysML2Tools.Tests.Query; /// internal static class QueryTestFixtures { + /// + /// Shared inline SysML fixture for connection-aware impact scenarios: a minimal + /// reduction of the three-axis-gantry topology in which two motor part usages each + /// connect one of their nested ports to a distinct port of a shared hub part usage. + /// Neither motor references the other, and no motor has any incoming reference edge, so + /// the default (reference-only) impact result for a motor is empty and any + /// element reported by --include-connections was necessarily reached through a + /// connector. + /// + public const string GantryConnections = """ + package Model { + part def Hub { + port J1; + port J2; + } + + part def Motor { + port power; + port encoder; + } + + part def System { + part hub : Hub; + part motorA : Motor; + part motorB : Motor; + + connect motorA.power to hub.J1; + connect motorB.power to hub.J2; + } + } + """; + + /// + /// Shared inline SysML fixture for declared-endpoint connector scenarios. No endpoint is + /// a nested port: every connector and binding names a directly declared sibling part + /// usage, so the declared-endpoint branch of the far-endpoint roll-up is exercised and + /// the endpoint itself — never the enclosing definition that also owns the subject — must + /// be reported. + /// + public const string DeclaredEndpointConnections = """ + package Model { + part def System { + part alpha; + part beta; + part gamma; + + connect alpha to beta; + bind beta = gamma; + } + } + """; + /// /// Writes to a uniquely-named temp .sysml file, runs /// query with the given arguments (the temp file path is appended automatically), diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs index 924aa163..881bc3ac 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs @@ -213,6 +213,220 @@ package Model { Assert.Contains("Model::Leaf", output); } + /// + /// 'impact' without --include-connections keeps its existing reference-only semantics: + /// a part usage whose only relationship to the rest of the model is a connector reports + /// no impacted elements. + /// + [Fact] + public async Task Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, "impact", "--element", "Model::System::motorA"); + + Assert.Equal(0, exitCode); + Assert.Contains("0 element(s) transitively impacted", output); + Assert.DoesNotContain("Model::System::hub", output); + Assert.DoesNotContain("including connections", output); + } + + /// + /// 'impact --include-connections' reaches the part usage on the far side of a connector, + /// which the reference-only walk cannot see. + /// + [Fact] + public async Task Impact_IncludeConnections_ReachesConnectedSiblingPart() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, + "impact", + "--element", + "Model::System::motorA", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::hub | part |", output); + Assert.Contains("including connections (connection hops <= 1)", output); + } + + /// + /// Connector traversal is undirected: querying from the endpoint that the connector + /// declaration names second (the hub) reaches the originating parts just as querying + /// from the first-named endpoint reaches the hub. + /// + [Fact] + public async Task Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, + "impact", + "--element", + "Model::System::hub", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::motorA | part |", output); + Assert.Contains("| Model::System::motorB | part |", output); + } + + /// + /// Connector endpoints are nested ports, but impact entries are attributed to the + /// nearest owning part usage, with the raw port retained in the entry notes so no + /// information is lost. + /// + [Fact] + public async Task Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, + "impact", + "--element", + "Model::System::motorA", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::hub | part |", output); + Assert.Contains("Model::System::motorA::power -> Model::System::hub::J1", output); + } + + /// + /// With no --walk-depth supplied, reference-edge traversal stays unlimited but connector + /// hops are bounded to one, so the second motor (two connector hops away, through the + /// hub) is not reported. + /// + [Fact] + public async Task Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, + "impact", + "--element", + "Model::System::motorA", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::hub | part |", output); + Assert.DoesNotContain("Model::System::motorB", output); + } + + /// + /// Supplying --walk-depth raises the connector hop bound to that same value, so the + /// second motor is reached through the hub at the second hop. + /// + [Fact] + public async Task Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.GantryConnections, + "impact", + "--element", + "Model::System::motorA", + "--include-connections", + "--walk-depth", + "2"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::hub | part |", output); + Assert.Contains("| Model::System::motorB | part |", output); + Assert.Contains("including connections (connection hops <= 2)", output); + } + + /// + /// A cyclic connector topology (two parts joined by two connectors in opposite textual + /// order) terminates and reports each impacted element exactly once. + /// + [Fact] + public async Task Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates() + { + const string sysml = """ + package Model { + part def Hub { + port J1; + port J2; + } + + part def Motor { + port power; + port encoder; + } + + part def System { + part hub : Hub; + part motorA : Motor; + + connect motorA.power to hub.J1; + connect hub.J2 to motorA.encoder; + } + } + """; + + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + sysml, "impact", "--element", "Model::System::motorA", "--include-connections", "--walk-depth", "5"); + + Assert.Equal(0, exitCode); + Assert.Contains("1 element(s) transitively impacted", output); + Assert.Single( + output.Split('\n'), + line => line.StartsWith("| Model::System::hub |", StringComparison.Ordinal)); + } + + /// + /// Binding connectors ('bind A = B;') are traversed undirected exactly like connection + /// connectors, reachable from either bound side. + /// + [Fact] + public async Task Impact_IncludeConnections_BindingEdges_AreTraversedUndirected() + { + const string sysml = """ + package Model { + part def Controller { + attribute setPoint; + } + + part def Actuator { + attribute command; + } + + part def System { + part controller : Controller; + part actuator : Actuator; + + bind controller.setPoint = actuator.command; + } + } + """; + + var (fromA, exitCodeA) = await QueryTestFixtures.RunQueryAsync( + sysml, "impact", "--element", "Model::System::controller", "--include-connections"); + var (fromB, exitCodeB) = await QueryTestFixtures.RunQueryAsync( + sysml, "impact", "--element", "Model::System::actuator", "--include-connections"); + + Assert.Equal(0, exitCodeA); + Assert.Equal(0, exitCodeB); + Assert.Contains("| Model::System::actuator | part |", fromA); + Assert.Contains("| Model::System::controller | part |", fromB); + } + + /// + /// When a connector names directly declared sibling part usages as its endpoints, the far + /// endpoint requires no roll-up and is reported as the impacted element. The enclosing + /// 'part def' that owns both endpoints is never reported in its place. + /// + [Fact] + public async Task Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.DeclaredEndpointConnections, + "impact", + "--element", + "Model::System::alpha", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::beta | part |", output); + Assert.DoesNotContain("| Model::System | part def |", output); + } + /// /// 'describe' reports the element's kind, supertypes, annotation text, and child count. /// From d0971113cd5945eb859b053c2f5d7d260ca5735e Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Wed, 29 Jul 2026 15:31:54 -0400 Subject: [PATCH 2/4] Fix connector traversal defects in Query impact verb Connect/Binding edges are published into the semantic index alongside ordinary reference edges, so the impact verb's reference pass followed every connector a second time: directed instead of undirected, attributed to the raw nested-port endpoint instead of its owning declaration, and outside the connector hop bound. Exclude connector edge kinds from CollectImpactReferences so CollectImpactConnections is their single attribution path. This bounds connector chains to the hop limit the summary already claims, and stops one connector producing two entries when the port sits on the connector's source side. Replace the impact cycle guard's membership set with a minimum-hop dictionary and a shared TryReach helper. Connector-hop cost is not monotonic in breadth-first depth, so an element first reached over a connector may later be reached over a pure reference path with its hop budget restored. Re-expand on a strictly cheaper arrival without emitting a second entry, so nothing within the bound is dropped while each element keeps its first-arrival depth and relation attribution. The flag-absent default now reports fewer rows than released versions on models with declared-endpoint connectors. This is a defect correction, not a feature removal: the previous behavior contradicted the documented default and emitted qualified names that cannot be used as query subjects. It also makes --include-connections a strict superset of the default. The change is called out in the user guide, README, and design constraints. Rewrite two mis-named tests onto fixtures that can actually fail, since every existing connector fixture puts both endpoints on nested ports and therefore never reaches the defective path. Add fixtures and tests for both user-reported models and for the hop-minimality case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- README.md | 2 +- docs/design/sysml2-tools-core/query.md | 66 +++++-- docs/reqstream/sysml2-tools-core/query.yaml | 53 +++++- docs/user_guide/introduction.md | 21 +++ docs/verification/sysml2-tools-core/query.md | 118 ++++++++++-- .../Query/QueryEngine.cs | 128 +++++++++++-- .../Query/QueryEngineImpactTests.cs | 109 +++++++++++ .../Query/QueryTestFixtures.cs | 83 +++++++++ .../Query/QueryVerbsTests.cs | 176 ++++++++++++++++-- 9 files changed, 701 insertions(+), 55 deletions(-) diff --git a/README.md b/README.md index 5eef1ae9..5841de53 100644 --- a/README.md +++ b/README.md @@ -259,7 +259,7 @@ sysml2tools help [lint|render|query []|export] | `--format markdown\|json` | Output format (default: `markdown`); distinct from `render`'s `--format` (`svg`/`png`) | | `--output ` | Write query report to this **file** (default: stdout); differs from `render`'s `--output` dir | | `--walk-depth <#>` | Maximum impact-walk depth (`impact` verb only) | -| `--include-connections` | `impact` only: also follow `connect`/`bind` edges, undirected (1 hop) | +| `--include-connections` | `impact` only: follow `connect`/`bind` edges undirected (1 hop); off = not followed | | `--direction up\|down\|both` | Traversal direction (`hierarchy` verb only) | | `--kind ` | Element-kind filter (`list`/`find` verbs only) | | `--name ` | Name substring filter (`list`/`find` verbs only) | diff --git a/docs/design/sysml2-tools-core/query.md b/docs/design/sysml2-tools-core/query.md index ab3a3e0a..63100f32 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -112,12 +112,19 @@ flowchart TD 5. `Dependencies` combines `Uses` and `UsedBy` for the same subject, tagging entries with `QueryEntryDirection.Outgoing` or `Incoming` rather than performing a third traversal. 6. `Impact` performs a breadth-first transitive closure over incoming edges, optionally bounded - by `WalkDepth`, with a visited set preventing infinite loops on cyclic graphs. Each frontier - item is a `(qualified name, connector hops taken)` pair, and each item is expanded by two - collaborating helpers: - - `CollectImpactReferences` — always on, and unchanged from the original algorithm: it reads - `workspace.Index.GetIncomingEdges` and emits one entry per newly-reached source, carrying - the connector-hop count through unchanged (a reference hop never consumes a connector hop). + by `WalkDepth`, with a minimum-connector-hop guard preventing infinite loops on cyclic + graphs. Each frontier item is a `(qualified name, connector hops taken)` pair, and each item + is expanded by two collaborating helpers: + - `CollectImpactReferences` — always on: it reads `workspace.Index.GetIncomingEdges` and + emits one entry per newly-reached source, carrying the connector-hop count through + unchanged (a reference hop never consumes a connector hop). Edges whose kind is in + `ImpactConnectorEdgeKinds` are **filtered out** by `IsImpactConnectorKind`, because + `ReferenceResolver` publishes resolved connector endpoints into `SemanticIndex.AllEdges` + alongside ordinary reference edges. Without that filter every connector would be followed + a second time here — directed instead of undirected, attributed to the raw endpoint instead + of its owning declaration, without `ViaQualifiedName`, and outside the connector hop bound. + The filter makes `CollectImpactConnections` the single attribution path for connector + relationships, so a connector is reported exactly once and only under `IncludeConnections`. - `CollectImpactConnections` — only when `QueryOptions.IncludeConnections` is set. It walks the connector edges collected once per invocation by `CollectConnectorEdges` for `ImpactConnectorEdgeKinds` (`Connect` and `Binding`) and follows each one **undirected**: @@ -148,8 +155,23 @@ flowchart TD loop therefore cannot express, so a per-frontier-item hop counter carries it instead. The one-hop default exists because connector graphs in real models are dense meshes, in which an unbounded connector closure degenerates to "the whole assembly". A single shared ordinal - `visited` set spans both helpers, so cyclic connector topologies terminate and no element is - reported twice. + `bestHops` dictionary spans both helpers, mapping each qualified name to the **minimum** + connector-hop count at which it has been reached, and the shared `TryReach` helper resolves + every arrival three ways: a first arrival is added to the next frontier and emits an entry; + a re-arrival at a strictly cheaper hop count is added to the frontier but emits no second + entry; a re-arrival at an equal or costlier hop count is discarded. Minimum-hop tracking is + required because connector-hop cost is not monotonic in breadth-first depth — an element + first reached over a connector may later be reached over a pure reference path that consumed + none of the hop budget, and a membership-only guard would pin it to its costlier first + arrival and silently drop everything one connector hop beyond it. Suppressing the entry on + re-arrival keeps each element reported exactly once, and keeps `Depth`, `Relation`, `Detail`, + `Notes`, and `ViaQualifiedName` describing the shallowest path: first arrival in a + breadth-first walk is by construction the minimum depth, so an already-recorded entry is + never rewritten. Termination is guaranteed because a recorded hop count strictly decreases on + each re-admission and is bounded below by zero. A consequence worth stating explicitly: an + element unlocked only by a cheaper re-arrival is reported at the breadth-first level at which + it actually became reachable, which is deeper than the re-arrival level — this is correct, + not an off-by-one. 7. `Describe` reports the target element's own kind and qualified name in `Summary`, then adds resolved supertypes, typing, annotations, applied metadata annotations, and a `Children: N` count. `N` is the count of visible, named child entries actually placed in `Entries` (i.e. @@ -273,10 +295,27 @@ flowchart TD in traversal order without affecting external output stability. - `QualifiedNameShortener` affects only `dependencies` Markdown output. JSON output, and every other verb's Markdown output, remains fully qualified. -- The default `impact` semantics are frozen: `IncludeConnections` defaults to `false`, and when - it is unset no connector edges are collected and no connector branch runs, so results are - identical to those produced before connection-aware impact existed. Additions to - `QueryResultEntry` are nullable and null-omitted from JSON for the same reason. +- The default `impact` semantics are reference-only: `IncludeConnections` defaults to `false`, + and when it is unset no connector edges are collected, no connector branch runs, and + `Connect`/`Binding` edges are filtered out of the reference closure as well. Additions to + `QueryResultEntry` are nullable and null-omitted from JSON, so a default-path JSON payload + keeps its original shape. +- **The default `impact` result changed relative to releases before this correction.** Resolved + connector endpoints have always been present in `SemanticIndex.AllEdges`, so the default walk + previously followed them as ordinary incoming reference edges: unbounded by the connector hop + limit, attributed to the raw nested-port endpoint rather than its owning declaration, and + without any relation metadata. That contradicted the documented default contract and emitted + qualified names (such as `System::hub::J1`) that cannot be used as a `--element` subject. It + is now corrected, so a default `impact` query over a model with declared-endpoint connectors + reports fewer rows than before — frequently none. The correction also makes + `--include-connections` a strict superset of the default, which is the only defensible + meaning for an `--include-*` flag. +- `impact` is deliberately **not** exactly "transitive `used-by`" any more. `UsedBy` remains + unfiltered and still reports connector edges as incoming references, because `used-by` answers + "what edges point at this" while `impact` answers "what breaks if this changes". The two + legitimately diverge once connectors receive first-class, rolled-up, hop-bounded handling. + This divergence is recorded rather than removed; `Uses`, `UsedBy`, and `Dependencies` keep + their existing semantics. - Connector traversal is bounded by default (`DefaultConnectionHopLimit` = 1 hop) rather than unbounded, because real connector graphs are dense meshes in which an unbounded closure answers "the whole assembly". A caller widens the bound deliberately by supplying @@ -294,8 +333,9 @@ flowchart TD | SysML2Tools-Core-Query-UsedBy | `QueryEngine.UsedBy` | | SysML2Tools-Core-Query-Dependencies | `QueryEngine.Dependencies`; `QueryResultRenderer.RenderMarkdown`/`RenderJson` | | SysML2Tools-Core-Query-DependenciesNameShortening | `QueryResultRenderer.RenderMarkdown`; `QualifiedNameShortener` | -| SysML2Tools-Core-Query-Impact | `QueryEngine.Impact`; `QueryEngine.CollectImpactReferences` | +| SysML2Tools-Core-Query-Impact | `QueryEngine.Impact`; `CollectImpactReferences`; `IsImpactConnectorKind` | | SysML2Tools-Core-Query-ImpactConnections | `QueryEngine.CollectImpactConnections`; `RollUpToNearestDeclaration` | +| SysML2Tools-Core-Query-ImpactHopMinimality | `QueryEngine.TryReach` | | SysML2Tools-Core-Query-EntryTraversalMetadata | `QueryResultEntry.Depth`/`Relation`/`ViaQualifiedName` | | SysML2Tools-Core-Query-Describe | `QueryEngine.Describe` | | SysML2Tools-Core-Query-Hierarchy | `QueryEngine.Hierarchy` | diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index 7cbc1ff6..ca4f3967 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -94,7 +94,8 @@ sections: title: >- The Query subsystem shall compute the transitive closure of incoming references from a target element through the `impact` verb, optionally bounded by `WalkDepth` and guarded - against cycles. + against cycles, excluding `Connect` and `Binding` connector edges from that reference + closure. justification: | A single-hop reverse-reference view understates true blast radius for deeply-referenced elements. A bounded transitive closure gives callers a practical, terminating answer to @@ -102,19 +103,31 @@ sections: are unchanged by the opt-in connection traversal described by `SysML2Tools-Core-Query-ImpactConnections`; connector edges are followed only when `IncludeConnections` is explicitly supplied. + + Connector relationships are published into the semantic index alongside ordinary + reference edges, so the exclusion must be explicit. Without it the default walk followed + every connector as a plain incoming reference — directed rather than undirected, + attributed to the raw nested-port endpoint rather than its owning declaration, and + outside the connector hop bound. That was a defect: it contradicted the documented + default contract and emitted qualified names that are not resolvable as query subjects. + Correcting it makes the default result smaller than in releases before this change, and + makes `IncludeConnections` a strict superset of the default. tests: - Impact_DepthOne_OnlyReachesDirectReferences - Impact_Unbounded_ReachesTransitiveClosure - Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult + - Impact_ChainedDeclaredConnectors_FlagAbsent_ReportsNoImpactedElements + - Impact_SourceSidePortEndpoint_FlagAbsent_ReportsNoImpactedElements - id: SysML2Tools-Core-Query-ImpactConnections title: >- When `IncludeConnections` is supplied, the `impact` verb shall additionally traverse resolved `Connect` and `Binding` connector relationships without regard to their recorded endpoint order, matching a subject element against endpoints nested inside it - and attributing each far endpoint to its nearest owning declaration, bounding connector - hops along any single traversal path to `WalkDepth` when supplied or to one hop - otherwise, and terminating on cyclic connector topologies. + and attributing each far endpoint to its nearest owning declaration exactly once and + never additionally as a reference edge, bounding connector hops along any single + traversal path to `WalkDepth` when supplied or to one hop otherwise, and terminating on + cyclic connector topologies. justification: | A connector's two ends carry no semantic source-causes-target direction, so the reverse-only reference closure reports nothing for a part joined to the rest of the @@ -124,6 +137,12 @@ sections: answer to be meaningful. Because connector graphs are dense meshes, an unbounded connector closure degenerates to "the whole assembly", so a one-hop default keeps the opt-in answer useful while `WalkDepth` remains available to widen it deliberately. + + The single-attribution guarantee is required because the same connector is reachable + from both the connector pass and the reference-edge pass. Attributing it twice produced + two result rows for one connector whenever the nested-port endpoint sat on the + connector's source side, and let a connector chain be followed past the hop bound the + summary text claims. tests: - Impact_IncludeConnections_ReachesConnectedSiblingPart - Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart @@ -135,6 +154,30 @@ sections: - Impact_IncludeConnections_ThroughPublicApi_ReturnsConnectedPartEntries - Impact_IncludeConnections_DeclaredFarEndpoint_ReportsEndpointItself - Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition + - Impact_IncludeConnections_ChainedDeclaredConnectors_StopsAtOneConnectorHop + - Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthTwo_ReachesSecondHopOnly + - Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort + - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry + + - id: SysML2Tools-Core-Query-ImpactHopMinimality + title: >- + The `impact` verb's cycle guard shall record the minimum connector-hop count at which + each element was reached and shall re-expand an element whenever a strictly cheaper hop + count is found, while retaining each element's first-arrival depth and relation + attribution and reporting each element at most once. + justification: | + Connector-hop cost is not monotonic in breadth-first depth: an element first reached + over a connector can later be reached over a pure reference path that consumed none of + the hop budget. A guard that records only membership pins each element to its + first-arrival cost, so such an element is never expanded with its restored budget and + elements one connector hop beyond it are silently dropped — making the result depend on + frontier iteration order rather than on the model. Re-expanding on a strictly cheaper + arrival removes that dependence, while suppressing the duplicate entry preserves both + the "each element appears exactly once" invariant and the rule that `Depth` equals the + breadth-first level of first arrival. + tests: + - Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement + - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution - id: SysML2Tools-Core-Query-EntryTraversalMetadata title: >- @@ -153,6 +196,8 @@ sections: - Impact_ConnectionEntry_RecordsDepthRelationAndViaQualifiedName - Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName - Impact_ReferenceEntry_RecordsDepthAndReferenceRelation + - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry + - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution - RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips - RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia - RenderMarkdown_ImpactEntry_DetailRemainsHumanReadableDepthText diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 12c74182..4d59f5d4 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -471,6 +471,18 @@ imports, and similar resolved references — in the reverse direction. A part th the rest of an assembly purely by `connect` statements therefore reports no impacted elements, because a connector is not a reference. +> **Upgrade note — the default `impact` result changed.** In releases before this correction, +> `query impact` could follow `connect` and `bind` relationships **even without** +> `--include-connections`, whenever a connector named a directly declared element (for example +> `connect b to a;` or `connect hub.J1 to motorA;`). Those connectors were followed without any +> hop bound and were reported as their raw endpoint — including nested port names such as +> `Model::System::hub::J1`, which cannot themselves be used as an `--element` subject. That +> contradicted the documented default described above and has been fixed. As a result, a +> `query impact` command that you have not changed may now report **fewer** rows than it used to +> — often none — on models that rely on `connect` statements. To get those elements back, +> deliberately add `--include-connections`, which now reports them correctly rolled up to their +> owning part and within the documented hop bound. + Adding `--include-connections` makes `impact` follow `connect` and `bind` relationships as well. Three rules apply: @@ -498,6 +510,15 @@ additionally carry `Depth` (the traversal depth), `Relation` (`Connect`/`Binding connector, or the reference edge kind otherwise), and `ViaQualifiedName`, so scripts can distinguish "referenced by" from "connected to" without parsing the human-readable detail text. +One consequence of the hop bound is worth knowing when reading depths. Reference hops cost +nothing against the connector budget, so an element that was first reached over a connector may +later be reached again over a pure reference path with its full connector budget restored. When +that happens the tool re-explores the connectors around it, which can report an element at a +depth *greater* than the depth of the element it was connected to. That is deliberate: the +reported depth is the first traversal level at which the element genuinely became reachable +within the hop budget, and the alternative — refusing to re-explore — would silently omit +elements that are well inside the bound you asked for. + ## Query Output Formats | Format | Flag | Notes | diff --git a/docs/verification/sysml2-tools-core/query.md b/docs/verification/sysml2-tools-core/query.md index 8e17fe1b..eb430d13 100644 --- a/docs/verification/sysml2-tools-core/query.md +++ b/docs/verification/sysml2-tools-core/query.md @@ -64,9 +64,10 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne `describe`), falling back to a generic `**Entries**` / `_No entries._` label for any unrecognized verb. The label is always plain bold text, never an ATX heading, so the report never branches into a Markdown sub-section regardless of the caller's requested heading depth. -- `impact` without `IncludeConnections` reports exactly the reference-only result it reported - before connection-aware impact existed, including for a subject whose only relationship to - the rest of the model is a connector. +- `impact` without `IncludeConnections` reports a reference-only result that excludes `Connect` + and `Binding` edges entirely, so a subject whose only relationship to the rest of the model is + a connector reports no impacted elements — including when both connector endpoints are + declared part usages rather than nested ports. - `impact` with `IncludeConnections` reaches the part usage on the far side of a connector, from either end of that connector, proving the traversal is undirected rather than reverse-only. - Connector endpoints that are nested ports are attributed to their nearest owning part usage, @@ -76,7 +77,15 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne topology and the enclosing definition that also owns the subject is never reported in the endpoint's place. - Connector hops are bounded to one per traversal path when no `WalkDepth` is supplied, and to - `WalkDepth` when it is, while reference-edge depth semantics are unchanged. + `WalkDepth` when it is, while reference-edge depth semantics are unchanged. The bound holds + for connector chains whose endpoints are declared part usages, not only for port endpoints. +- A single connector produces exactly one impact entry — its far endpoint rolled up to the + nearest owning declaration — regardless of which side of the connector the nested port sits + on, and never an additional raw-endpoint entry. +- An element re-reached at a strictly lower connector-hop count is re-expanded, so elements + within the hop budget of the reference closure are never dropped because a costlier path + reached them first, while the re-reached element keeps its first-arrival depth and relation + attribution and is still reported exactly once. - A cyclic connector topology terminates and reports each impacted element exactly once. - `Binding` connectors (`bind A = B;`) are traversed undirected exactly like `Connect` connectors. @@ -220,6 +229,30 @@ declared-endpoints fixture and subject; verifies that the `Model::System::beta` `ViaQualifiedName` null because no roll-up occurred, while its `Notes` still name both raw connector endpoints so no information is lost. +**`Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry`**: Uses the +source-side-port fixture, in which the nested port is the connector's *source* +(`connect hub.J1 to motorA;`) rather than its target, and queries `Model::System::motorA` with +`IncludeConnections` set. That orientation makes the subject itself the incoming-edge key for +the connector, which is the precondition for duplicate attribution — an unfiltered reference +pass reports the raw port `Model::System::hub::J1` in addition to the correctly rolled-up +`Model::System::hub`. Verifies that the result contains **exactly one** entry in total, that it +is `Model::System::hub` at `Depth` 1 with `Relation` of `SysmlEdgeKind.Connect` and +`ViaQualifiedName` of `Model::System::hub::J1`, and that no entry names the raw port. The +whole-list `Assert.Single` overload is used deliberately: the predicate overload asserts only +that a *matching* entry is unique and is structurally blind to an extra non-matching entry, +which is how the duplicate previously escaped detection. + +**`Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution`**: +Uses the minimum-hop fixture, in which `b` is reached from `s` over `connect b to s` at one +connector hop and re-reached one breadth-first level deeper over the subsetting chain +`b :> s2 :> s` at zero hops. Verifies that `b` appears exactly once and retains its +first-arrival `Depth` of 1 and `Relation` of `SysmlEdgeKind.Connect` — the cheaper re-arrival +re-opens it for expansion but never rewrites its attribution — and that `z`, reachable only by +expanding the re-opened `b`, is reported with `Relation` of `SysmlEdgeKind.Connect` at `Depth` +**3**. Depth 3 is correct and is not an off-by-one: `b` is re-reached cheaply at level 2 and +therefore expands its connectors at level 3, which is genuinely the first breadth-first level +at which `z` becomes reachable within the hop budget. + **`WriteMarkdown_HappyPath_MatchesRendererOutput`**: Verifies that `WriteMarkdown` writes the same Markdown text produced by `QueryResultRenderer.RenderMarkdown`. @@ -316,16 +349,27 @@ references. **`Impact_Unbounded_ReachesTransitiveClosure`**: Verifies that unbounded `QueryEngine.Impact` reaches the full transitive closure of incoming references without looping on cycles. -The connection-aware `impact` scenarios below all use the shared `QueryTestFixtures. +The connection-aware `impact` scenarios below use the shared `QueryTestFixtures. GantryConnections` fixture (a minimal reduction of the three-axis-gantry topology: two motor part usages, each connecting one of its nested ports to a distinct port of a shared hub part -usage) except where a local variant is named. No motor references the other and no motor has an -incoming reference edge, so any element reported was necessarily reached through a connector. - -**`Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult`**: Regression lock for -unchanged default semantics — verifies that `impact` on a motor without the flag reports zero -impacted elements, does not name the connected hub, and does not mention connections in its -summary. +usage) except where another shared fixture or a local variant is named. No motor references the +other and no motor has an incoming reference edge, so any element reported was necessarily +reached through a connector. Because every `GantryConnections` connector endpoint is a nested +port, the queried part usage is never a key in the incoming-edge index; scenarios that must +detect connector edges leaking into the reference closure therefore use +`ChainedDeclaredConnectors` or `SourceSidePortConnector`, whose endpoints are declared part +usages or source-side ports, instead. + +**`Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult`**: Uses the +`QueryTestFixtures.ChainedDeclaredConnectors` fixture (four sibling part usages joined by +`connect b to a; connect c to b; connect d to c;`) and queries `Model::System::a` without the +flag; verifies zero impacted elements, that none of `b`, `c`, or `d` is named, and that the +summary does not mention connections. The fixture is deliberately *not* `GantryConnections`: +because every `GantryConnections` connector endpoint is a nested port, the queried part usage is +never a key in the incoming-edge index, so the defective path — connector edges leaking into the +reference closure — is never reached and no assertion over that fixture could ever fail. This +scenario is the lock on the corrected default: connector edge kinds are excluded from the +reference closure entirely. **`Impact_IncludeConnections_ReachesConnectedSiblingPart`**: Verifies that the same query with `--include-connections` reaches the connected hub part usage and reports the one-hop connection @@ -339,9 +383,15 @@ both motors, the mirror image of the previous scenario. reported entry is the owning part usage with kind `part` (not the nested port), and that the raw port-to-port endpoint pair is preserved in the entry's notes. -**`Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne`**: Verifies that with no -`--walk-depth` supplied the connected hub is reached but the second motor — two connector hops -away through the hub — is not. +**`Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne`**: Uses the +`QueryTestFixtures.ChainedDeclaredConnectors` fixture and queries `Model::System::a` with +`--include-connections`; verifies exactly `1 element(s) transitively impacted`, that the `b` row +is present, that neither `c` nor `d` is named, and that the summary reads +`including connections (connection hops <= 1)`. The fixture change from `GantryConnections` is +essential for the same reason given above — only declared (non-port) connector endpoints make +the queried element an incoming-edge key, and only then can a connector chain be followed past +the hop bound the summary claims. Asserting the bare element count, not just names, is what +makes the scenario sensitive to any extra row. **`Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop`**: Verifies that `--walk-depth 2` raises the connector hop bound to two, reaching the second motor, and reports @@ -362,6 +412,44 @@ binding names a directly declared sibling part usage rather than a nested port, `| Model::System::beta | part |` row and never the `| Model::System | part def |` row for the enclosing definition. +**`Impact_IncludeConnections_ChainedDeclaredConnectors_StopsAtOneConnectorHop`**: Uses the +`ChainedDeclaredConnectors` fixture and queries `Model::System::a` with +`--include-connections`; verifies exactly one impacted element, that it is `b`, and that neither +`c` (two connector hops) nor `d` (three) is reported. This is the direct regression lock for the +reported defect in which a declared-connector chain was traversed unbounded while the summary +still claimed a one-hop bound. + +**`Impact_ChainedDeclaredConnectors_FlagAbsent_ReportsNoImpactedElements`**: Same fixture and +subject without the flag; verifies zero impacted elements, that `b` is not named, and that the +summary omits the connections suffix — proving connector edges contribute nothing to the default +reference closure. + +**`Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthTwo_ReachesSecondHopOnly`**: +Same fixture and subject with `--walk-depth 2`; verifies that `b` and `c` are reported, `d` is +not, and the summary reads `including connections (connection hops <= 2)` — proving the bound +tracks `--walk-depth` rather than being absent. + +**`Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort`**: Uses the +`QueryTestFixtures.SourceSidePortConnector` fixture (`connect hub.J1 to motorA;`, the nested port +on the connector's source side) and queries `Model::System::motorA` with +`--include-connections`; verifies exactly one impacted element, that it is the +`| Model::System::hub | part |` row, and that no `| Model::System::hub::J1 | connect` row +appears. This is the regression lock for the reported defect in which a single connector +produced two rows — the correct rolled-up owner plus a raw port entry. + +**`Impact_SourceSidePortEndpoint_FlagAbsent_ReportsNoImpactedElements`**: Same fixture and +subject without the flag; verifies zero impacted elements and that the hub is not named at all. + +**`Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement`**: +Uses the `QueryTestFixtures.MinimumHopReExpansion` fixture and queries `Model::Assembly::s` with +`--include-connections`; verifies exactly three impacted elements — `b` (reached over a +connector), `s2` (reached over a reference edge), and `z`, which is reachable only because `b` +is re-expanded after being re-reached at a cheaper connector-hop count. `z` is reported at depth +3, since `b` is re-reached at breadth-first level 2 and expands its connectors at level 3; that +is the first level at which `z` is genuinely reachable within the hop budget, not an off-by-one. +Without minimum-hop tracking `z` is silently dropped and the result depends on frontier +iteration order. + **`Interface_ReportsPortsAndTypedFeatures`**: Verifies that `QueryEngine.Interface` reports a target definition's ports and typed features. diff --git a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs index 115339cc..3340d0bb 100644 --- a/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs +++ b/src/DemaConsulting.SysML2Tools.Core/Query/QueryEngine.cs @@ -39,6 +39,15 @@ public static class QueryEngine /// that carry no semantic source-causes-target direction, so both are traversed /// undirected. /// + /// + /// These kinds are also excluded from the impact verb's reference-edge + /// closure. They are present in alongside ordinary + /// reference edges, so without that exclusion every connector would be followed a second + /// time as a plain incoming reference — directed, unrolled, unattributed, and outside the + /// connector hop bound. Excluding them makes the + /// single attribution path for connector relationships, so each connector is reported + /// exactly once and only under . + /// private static readonly SysmlEdgeKind[] ImpactConnectorEdgeKinds = [ SysmlEdgeKind.Connect, @@ -242,11 +251,26 @@ public static QueryResult Dependencies(SysmlWorkspace workspace, SysmlNode eleme /// traversal path by or, when that is /// , by — so the outer /// loop cannot enforce it and a per-frontier-item hop counter is carried instead. + /// + /// Connector edge kinds () are excluded from the + /// reference closure, so a connector is attributed exactly once — by + /// , rolled up to its owning declaration and within + /// the hop bound — and never additionally as a raw incoming reference edge. + /// + /// + /// The cycle guard records the minimum connector-hop count at which each element + /// has been reached rather than mere membership, because connector-hop count is not + /// monotonic in breadth-first depth: an element first reached over a connector can later + /// be reached again over a pure reference path that consumes no hop budget. Such an + /// element is re-expanded so nothing within the budget is silently dropped, but its + /// already-recorded entry is never rewritten and no duplicate entry is emitted, so each + /// element is reported exactly once at its first-arrival depth and attribution. + /// /// public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, QueryOptions options) { var qualifiedName = QualifiedNameOf(element, options); - var visited = new HashSet(StringComparer.Ordinal) { qualifiedName }; + var bestHops = new Dictionary(StringComparer.Ordinal) { [qualifiedName] = 0 }; var entries = new List(); // Collected once per call, and only when requested, rather than once per frontier item. @@ -266,12 +290,12 @@ public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, Qu foreach (var (current, hops) in frontier) { - CollectImpactReferences(workspace, options, current, hops, depth, visited, entries, next); + CollectImpactReferences(workspace, options, current, hops, depth, bestHops, entries, next); if (options.IncludeConnections && hops < connectionHopLimit) { CollectImpactConnections( - workspace, options, current, hops, depth, connectorEdges, visited, entries, next); + workspace, options, current, hops, depth, connectorEdges, bestHops, entries, next); } } @@ -301,12 +325,21 @@ public static QueryResult Impact(SysmlWorkspace workspace, SysmlNode element, Qu /// with their connector-hop count carried through unchanged /// (a reference hop never consumes a connector hop). /// + /// + /// Edges whose kind is in are skipped. Connector + /// edges are published into alongside ordinary + /// reference edges, so following them here as well would report every connector a second + /// time — directed instead of undirected, as the raw endpoint instead of its owning + /// declaration, without attribution, and + /// outside the connector hop bound. is therefore + /// the single attribution path for connector relationships. + /// /// The loaded workspace. /// The parsed query options. /// The frontier item's qualified name. /// The number of connector hops already taken to reach . /// The 1-based traversal depth of the names being reached. - /// The shared cycle guard. + /// The shared minimum-connector-hop cycle guard. /// The result entries accumulated so far. /// The next frontier being built. private static void CollectImpactReferences( @@ -315,20 +348,24 @@ private static void CollectImpactReferences( string current, int hops, int depth, - HashSet visited, + Dictionary bestHops, List entries, List<(string Name, int ConnectionHops)> next) { foreach (var edge in workspace.Index.GetIncomingEdges(current)) { - if (edge.SourceQualifiedName is not { Length: > 0 } source || !visited.Add(source)) + if (IsImpactConnectorKind(edge.Kind) || + edge.SourceQualifiedName is not { Length: > 0 } source || + !TryReach(bestHops, source, hops, out var isFirstArrival)) { continue; } next.Add((source, hops)); - if (!IsVisible(source, workspace, options.IncludeStdlib)) + // A cheaper re-arrival only re-opens the element for expansion; its first-arrival + // entry (and therefore its depth and relation attribution) is never rewritten. + if (!isFirstArrival || !IsVisible(source, workspace, options.IncludeStdlib)) { continue; } @@ -357,7 +394,7 @@ private static void CollectImpactReferences( /// The number of connector hops already taken to reach . /// The 1-based traversal depth of the names being reached. /// The connector edges collected once for this invocation. - /// The shared cycle guard. + /// The shared minimum-connector-hop cycle guard. /// The result entries accumulated so far. /// The next frontier being built. private static void CollectImpactConnections( @@ -367,7 +404,7 @@ private static void CollectImpactConnections( int hops, int depth, List<(string Source, string Target, string Keyword, SysmlEdgeKind Kind)> connectorEdges, - HashSet visited, + Dictionary bestHops, List entries, List<(string Name, int ConnectionHops)> next) { @@ -386,14 +423,16 @@ private static void CollectImpactConnections( var near = nearIsSource ? source : target; var far = nearIsSource ? target : source; var owner = RollUpToNearestDeclaration(workspace, far); - if (!visited.Add(owner)) + if (!TryReach(bestHops, owner, hops + 1, out var isFirstArrival)) { continue; } next.Add((owner, hops + 1)); - if (!IsVisible(owner, workspace, options.IncludeStdlib)) + // A cheaper re-arrival only re-opens the element for expansion; its first-arrival + // entry (and therefore its depth and relation attribution) is never rewritten. + if (!isFirstArrival || !IsVisible(owner, workspace, options.IncludeStdlib)) { continue; } @@ -1005,6 +1044,73 @@ candidate is not null && (string.Equals(candidate, subjectQualifiedName, StringComparison.Ordinal) || candidate.StartsWith(subjectQualifiedName + "::", StringComparison.Ordinal)); + /// + /// Determines whether an edge kind is one of the connector kinds the impact verb + /// handles through its dedicated, hop-bounded, rolled-up connector pass. + /// + /// The edge kind to test. + /// + /// when is in + /// and must therefore be excluded from the + /// reference-edge closure. + /// + private static bool IsImpactConnectorKind(SysmlEdgeKind kind) => + Array.IndexOf(ImpactConnectorEdgeKinds, kind) >= 0; + + /// + /// Applies the impact verb's minimum-connector-hop cycle guard to a candidate + /// element, deciding whether it may be expanded and whether it is a first arrival. + /// + /// + /// A plain membership set is insufficient because connector-hop count is not monotonic in + /// breadth-first depth: an element first reached over a connector may later be reached + /// over a pure reference path that has consumed less of the hop budget, and would + /// otherwise never be expanded at the cheaper cost — silently dropping elements that are + /// genuinely within the bound. Termination is guaranteed because a recorded hop count + /// strictly decreases on each re-admission and is bounded below by zero. + /// + /// + /// The guard, mapping qualified name to the minimum connector-hop count at which it has + /// been reached so far. Updated in place. + /// + /// The candidate element's qualified name. + /// The connector-hop count at which the candidate is being reached. + /// + /// On return, when the candidate had never been reached before and + /// therefore requires a result entry; when it is a cheaper + /// re-arrival at an element that has already been reported. + /// + /// + /// when the candidate must be added to the next frontier — either + /// as a first arrival or as a strictly cheaper re-arrival; when it + /// has already been reached at an equal or lower hop count. + /// + private static bool TryReach( + Dictionary bestHops, + string name, + int hops, + out bool isFirstArrival) + { + // First arrival: record the cost, expand it, and emit its entry. + if (!bestHops.TryGetValue(name, out var existing)) + { + bestHops[name] = hops; + isFirstArrival = true; + return true; + } + + // Re-arrival at an equal or costlier hop count buys nothing already available. + isFirstArrival = false; + if (hops >= existing) + { + return false; + } + + // Strictly cheaper re-arrival: re-open for expansion without emitting a second entry. + bestHops[name] = hops; + return true; + } + /// /// Rolls a connector endpoint up to its nearest owning declaration. The endpoint itself /// is probed first and returned unchanged when it is itself present in diff --git a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs index e628bfea..c70a0414 100644 --- a/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tests/Query/QueryEngineImpactTests.cs @@ -68,6 +68,48 @@ part def System { } """; + /// + /// Connector topology placing the nested-port endpoint on the connector's source + /// side, so the subject part usage is itself the incoming-edge key for the connector. + /// That orientation is what exposes duplicate attribution: an unfiltered reference pass + /// reports the raw port in addition to the correctly rolled-up owning part usage. + /// + private const string SourceSidePortFixture = """ + package Model { + port def PowerPort; + + part def Hub { + port J1 : PowerPort; + } + + part def System { + part hub : Hub; + part motorA; + + connect hub.J1 to motorA; + } + } + """; + + /// + /// Topology in which b is first reached from s over a connector (one hop) + /// and later re-reached one level deeper over the subsetting chain b :> s2 :> s + /// at zero hops, so the minimum-hop cycle guard must re-expand it for z to be found. + /// + private const string MinimumHopFixture = """ + package Model { + part def Assembly { + part s; + part s2 :> s; + part b :> s2; + part z; + + connect b to s; + connect z to b; + } + } + """; + /// /// Writes the fixture to a temp file, loads it exactly as a non-CLI library caller /// would, resolves the named element, and returns both for a direct @@ -223,4 +265,71 @@ public async Task Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName() Assert.Contains(entry.Notes, n => n.Contains("Model::System::beta", StringComparison.Ordinal)); Assert.Contains(entry.Notes, n => n.Contains("Model::System::alpha", StringComparison.Ordinal)); } + + /// + /// A connector whose nested-port endpoint sits on the source side yields exactly one + /// entry — the port's owning part usage — and never an additional raw-port entry. + /// + /// + /// Assert.Single is applied to the whole entry list rather than through the + /// predicate overload on purpose: the predicate overload asserts only that a + /// matching entry is unique and is structurally blind to an extra non-matching + /// entry, which is exactly how the duplicate raw-port entry escaped detection. + /// + [Fact] + public async Task Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry() + { + var (workspace, element) = await LoadAsync(SourceSidePortFixture, "Model::System::motorA"); + + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::System::motorA", + IncludeConnections = true + }); + + var entry = Assert.Single(result.Entries); + Assert.Equal("Model::System::hub", entry.QualifiedName); + Assert.Equal(1, entry.Depth); + Assert.Equal(SysmlEdgeKind.Connect, entry.Relation); + Assert.Equal("Model::System::hub::J1", entry.ViaQualifiedName); + Assert.DoesNotContain(result.Entries, e => e.QualifiedName == "Model::System::hub::J1"); + } + + /// + /// An element re-reached at a strictly lower connector-hop count is re-expanded so + /// elements beyond it are not lost, while its already-recorded first-arrival depth and + /// relation attribution are retained and no duplicate entry is emitted. + /// + /// + /// z is expected at depth 3, not 2: b is re-reached cheaply at + /// breadth-first level 2 and therefore expands its connectors at level 3, which is + /// genuinely the first level at which z is reachable within the hop budget. + /// + [Fact] + public async Task Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution() + { + var (workspace, element) = await LoadAsync(MinimumHopFixture, "Model::Assembly::s"); + + var result = QueryEngine.Impact( + workspace, + element, + new QueryOptions + { + Verb = QueryVerb.Impact, + Element = "Model::Assembly::s", + IncludeConnections = true + }); + + var b = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::b"); + Assert.Equal(1, b.Depth); + Assert.Equal(SysmlEdgeKind.Connect, b.Relation); + + var z = Assert.Single(result.Entries, e => e.QualifiedName == "Model::Assembly::z"); + Assert.Equal(3, z.Depth); + Assert.Equal(SysmlEdgeKind.Connect, z.Relation); + } } diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs index fc794ea1..b9f7030f 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryTestFixtures.cs @@ -81,6 +81,89 @@ part def System { } """; + /// + /// Shared inline SysML fixture for connector hop-bound scenarios: a chain of three + /// connectors joining four directly declared sibling part usages, so a is one + /// connector hop from b, two from c, and three from d. + /// + /// Unlike , both endpoints of every connector are + /// declared part usages rather than nested ports, so each connector also appears as + /// an incoming edge keyed by the subject's own qualified name. That is what makes this + /// fixture — and not — able to detect a reference pass + /// that follows connector edges a second time: in the + /// incoming-edge key is always a nested port such as Model::System::hub::J1, never + /// the queried part usage, so the connector-leak path is never reached and no assertion + /// over it can fail. + /// + /// + public const string ChainedDeclaredConnectors = """ + package Model { + part def System { + part a; + part b; + part c; + part d; + + connect b to a; + connect c to b; + connect d to c; + } + } + """; + + /// + /// Shared inline SysML fixture placing a nested port on the connector's source + /// side (connect hub.J1 to motorA;) rather than its target side. + /// + /// This orientation makes Model::System::motorA the incoming-edge key for the + /// connector, so a reference pass that does not exclude connector kinds reports the raw + /// port Model::System::hub::J1 in addition to the correctly rolled-up + /// Model::System::hub — two entries for one connector. Every existing fixture puts + /// the port on the target side, where that duplication cannot arise. + /// + /// + public const string SourceSidePortConnector = """ + package Model { + port def PowerPort; + + part def Hub { + port J1 : PowerPort; + } + + part def System { + part hub : Hub; + part motorA; + + connect hub.J1 to motorA; + } + } + """; + + /// + /// Shared inline SysML fixture in which an element is first reached over a connector and + /// later re-reached over a cheaper pure-reference path. + /// + /// Querying impact from s: the connector connect b to s reaches b at + /// one connector hop, while the subsetting chain b :> s2 :> s reaches b + /// one level deeper at zero connector hops. Unless the cycle guard records the minimum + /// hop count and re-expands on the cheaper arrival, b is never expanded with hop + /// budget remaining and z — one connector hop beyond b — is silently lost. + /// + /// + public const string MinimumHopReExpansion = """ + package Model { + part def Assembly { + part s; + part s2 :> s; + part b :> s2; + part z; + + connect b to s; + connect z to b; + } + } + """; + /// /// Writes to a uniquely-named temp .sysml file, runs /// query with the given arguments (the temp file path is appended automatically), diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs index 881bc3ac..a3bb9c62 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QueryVerbsTests.cs @@ -214,19 +214,31 @@ package Model { } /// - /// 'impact' without --include-connections keeps its existing reference-only semantics: - /// a part usage whose only relationship to the rest of the model is a connector reports - /// no impacted elements. + /// 'impact' without --include-connections excludes connector edges from its reference + /// closure entirely, so a chain of connectors joining declared sibling part usages + /// reports no impacted elements at all. /// + /// + /// This corrects a pre-existing defect. Connector edges are published into the semantic + /// index alongside ordinary reference edges, so the default walk followed them as plain + /// incoming references — directed, attributed to the raw endpoint rather than its owning + /// declaration, and outside the connector hop bound. The fixture is deliberately + /// rather than + /// : only declared (non-port) endpoints + /// make the queried element an incoming-edge key, and therefore only they can exercise + /// the leaking path. + /// [Fact] public async Task Impact_IncludeConnectionsFlagAbsent_ReportsReferenceOnlyResult() { var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( - QueryTestFixtures.GantryConnections, "impact", "--element", "Model::System::motorA"); + QueryTestFixtures.ChainedDeclaredConnectors, "impact", "--element", "Model::System::a"); Assert.Equal(0, exitCode); Assert.Contains("0 element(s) transitively impacted", output); - Assert.DoesNotContain("Model::System::hub", output); + Assert.DoesNotContain("Model::System::b", output); + Assert.DoesNotContain("Model::System::c", output); + Assert.DoesNotContain("Model::System::d", output); Assert.DoesNotContain("including connections", output); } @@ -291,22 +303,33 @@ public async Task Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsag /// /// With no --walk-depth supplied, reference-edge traversal stays unlimited but connector - /// hops are bounded to one, so the second motor (two connector hops away, through the - /// hub) is not reported. + /// hops are bounded to one, so a chain of connectors is followed exactly one hop and the + /// reported count matches the "connection hops <= 1" claim in the summary. /// + /// + /// The fixture is rather than + /// because only declared (non-port) + /// connector endpoints make the queried element an incoming-edge key, which is the + /// precondition for an unbounded connector chain to leak through the reference pass. The + /// bare element count is asserted as well as the names, so the test is sensitive to any + /// extra row rather than only to specific ones. + /// [Fact] public async Task Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne() { var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( - QueryTestFixtures.GantryConnections, + QueryTestFixtures.ChainedDeclaredConnectors, "impact", "--element", - "Model::System::motorA", + "Model::System::a", "--include-connections"); Assert.Equal(0, exitCode); - Assert.Contains("| Model::System::hub | part |", output); - Assert.DoesNotContain("Model::System::motorB", output); + Assert.Contains("1 element(s) transitively impacted", output); + Assert.Contains("| Model::System::b | part |", output); + Assert.DoesNotContain("Model::System::c", output); + Assert.DoesNotContain("Model::System::d", output); + Assert.Contains("including connections (connection hops <= 1)", output); } /// @@ -427,6 +450,137 @@ public async Task Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSib Assert.DoesNotContain("| Model::System | part def |", output); } + /// + /// A chain of connectors between declared sibling part usages is followed exactly one + /// connector hop by default, so only the immediately connected part is reported and the + /// two- and three-hop parts are not. + /// + [Fact] + public async Task Impact_IncludeConnections_ChainedDeclaredConnectors_StopsAtOneConnectorHop() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.ChainedDeclaredConnectors, + "impact", + "--element", + "Model::System::a", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("1 element(s) transitively impacted", output); + Assert.Contains("| Model::System::b | part |", output); + Assert.DoesNotContain("Model::System::c", output); + Assert.DoesNotContain("Model::System::d", output); + } + + /// + /// Without --include-connections, a chain of connectors between declared sibling part + /// usages contributes nothing to the impact result: connector edge kinds are excluded + /// from the reference closure, so not even the directly connected part is reported. + /// + [Fact] + public async Task Impact_ChainedDeclaredConnectors_FlagAbsent_ReportsNoImpactedElements() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.ChainedDeclaredConnectors, + "impact", + "--element", + "Model::System::a"); + + Assert.Equal(0, exitCode); + Assert.Contains("0 element(s) transitively impacted", output); + Assert.DoesNotContain("Model::System::b", output); + Assert.DoesNotContain("including connections", output); + } + + /// + /// Raising --walk-depth to two raises the connector hop bound to two, so a connector + /// chain is followed exactly two hops — reaching the second part but never the third. + /// + [Fact] + public async Task Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthTwo_ReachesSecondHopOnly() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.ChainedDeclaredConnectors, + "impact", + "--element", + "Model::System::a", + "--include-connections", + "--walk-depth", + "2"); + + Assert.Equal(0, exitCode); + Assert.Contains("| Model::System::b | part |", output); + Assert.Contains("| Model::System::c | part |", output); + Assert.DoesNotContain("Model::System::d", output); + Assert.Contains("including connections (connection hops <= 2)", output); + } + + /// + /// A connector whose nested-port endpoint is on the source side is attributed exactly + /// once, to the port's owning part usage. The raw port name is never additionally + /// reported as an impacted element in its own right. + /// + [Fact] + public async Task Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.SourceSidePortConnector, + "impact", + "--element", + "Model::System::motorA", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("1 element(s) transitively impacted", output); + Assert.Contains("| Model::System::hub | part |", output); + Assert.DoesNotContain("| Model::System::hub::J1 | connect", output); + } + + /// + /// Without --include-connections, a source-side nested-port connector contributes nothing + /// to the impact result — neither the owning part usage nor the raw port. + /// + [Fact] + public async Task Impact_SourceSidePortEndpoint_FlagAbsent_ReportsNoImpactedElements() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.SourceSidePortConnector, + "impact", + "--element", + "Model::System::motorA"); + + Assert.Equal(0, exitCode); + Assert.Contains("0 element(s) transitively impacted", output); + Assert.DoesNotContain("Model::System::hub", output); + } + + /// + /// An element first reached over a connector and later re-reached over a cheaper pure + /// reference path is re-expanded with its restored hop budget, so elements one connector + /// hop beyond it are still reported rather than silently dropped. + /// + /// + /// 'z' is expected at depth 3, not 2: 'b' is re-reached cheaply at breadth-first level 2 + /// and therefore expands its connectors at level 3, which is genuinely the first level at + /// which 'z' is reachable within the hop budget. This is not an off-by-one. + /// + [Fact] + public async Task Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement() + { + var (output, exitCode) = await QueryTestFixtures.RunQueryAsync( + QueryTestFixtures.MinimumHopReExpansion, + "impact", + "--element", + "Model::Assembly::s", + "--include-connections"); + + Assert.Equal(0, exitCode); + Assert.Contains("3 element(s) transitively impacted", output); + Assert.Contains("| Model::Assembly::b | part |", output); + Assert.Contains("| Model::Assembly::s2 | part |", output); + Assert.Contains("| Model::Assembly::z | part |", output); + } + /// /// 'describe' reports the element's kind, supertypes, annotation text, and child count. /// From 59fc31c0f6ea5a097c0d326fdf5a01b317983acf Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Wed, 29 Jul 2026 16:06:58 -0400 Subject: [PATCH 3/4] Correct impact connector documentation accuracy Restate SysML2Tools-Core-Query-ImpactHopMinimality as observable behavior rather than cycle-guard mechanism, and move the guard, minimum-hop record, and re-expansion detail out of its justification while keeping the connector-hop non-monotonicity rationale. Scope the "not resolvable as query subjects" claim to nested-port endpoints, since declared endpoint names such as Model::System::b do resolve, and replace the appeal to a "documented default contract" with an intrinsic characterization of the defect, because that contract text never shipped in a released user guide. Reframe the user guide upgrade note, which anachronistically described prior releases as following connectors "even without --include-connections" when that flag did not exist, while retaining the warning strength. Restate the verification acceptance criterion to match the requirement and lead the hop-minimality traceability row with its public entry point. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/design/sysml2-tools-core/query.md | 8 +++-- docs/reqstream/sysml2-tools-core/query.yaml | 33 +++++++++++--------- docs/user_guide/introduction.md | 11 ++++--- docs/verification/sysml2-tools-core/query.md | 8 ++--- 4 files changed, 33 insertions(+), 27 deletions(-) diff --git a/docs/design/sysml2-tools-core/query.md b/docs/design/sysml2-tools-core/query.md index 63100f32..a5e37713 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -304,8 +304,10 @@ flowchart TD connector endpoints have always been present in `SemanticIndex.AllEdges`, so the default walk previously followed them as ordinary incoming reference edges: unbounded by the connector hop limit, attributed to the raw nested-port endpoint rather than its owning declaration, and - without any relation metadata. That contradicted the documented default contract and emitted - qualified names (such as `System::hub::J1`) that cannot be used as a `--element` subject. It + without any relation metadata. That made connector traversal an unbounded, direction-sensitive + side effect of the reference walk rather than the opt-in behavior this design defines, and + where an endpoint was a nested port it emitted qualified names (such as `System::hub::J1`) + that cannot be used as a `--element` subject. It is now corrected, so a default `impact` query over a model with declared-endpoint connectors reports fewer rows than before — frequently none. The correction also makes `--include-connections` a strict superset of the default, which is the only defensible @@ -335,7 +337,7 @@ flowchart TD | SysML2Tools-Core-Query-DependenciesNameShortening | `QueryResultRenderer.RenderMarkdown`; `QualifiedNameShortener` | | SysML2Tools-Core-Query-Impact | `QueryEngine.Impact`; `CollectImpactReferences`; `IsImpactConnectorKind` | | SysML2Tools-Core-Query-ImpactConnections | `QueryEngine.CollectImpactConnections`; `RollUpToNearestDeclaration` | -| SysML2Tools-Core-Query-ImpactHopMinimality | `QueryEngine.TryReach` | +| SysML2Tools-Core-Query-ImpactHopMinimality | `QueryEngine.Impact`; `CollectImpactConnections`; `TryReach` | | SysML2Tools-Core-Query-EntryTraversalMetadata | `QueryResultEntry.Depth`/`Relation`/`ViaQualifiedName` | | SysML2Tools-Core-Query-Describe | `QueryEngine.Describe` | | SysML2Tools-Core-Query-Hierarchy | `QueryEngine.Hierarchy` | diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index ca4f3967..55afd2cf 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -108,8 +108,10 @@ sections: reference edges, so the exclusion must be explicit. Without it the default walk followed every connector as a plain incoming reference — directed rather than undirected, attributed to the raw nested-port endpoint rather than its owning declaration, and - outside the connector hop bound. That was a defect: it contradicted the documented - default contract and emitted qualified names that are not resolvable as query subjects. + outside the connector hop bound. That was a defect: connector traversal became an + unbounded, direction-sensitive side effect of the reference walk instead of the bounded, + undirected, opt-in behavior it is meant to be, and where a connector endpoint was a + nested port the walk emitted qualified names that are not resolvable as query subjects. Correcting it makes the default result smaller than in releases before this change, and makes `IncludeConnections` a strict superset of the default. tests: @@ -161,20 +163,21 @@ sections: - id: SysML2Tools-Core-Query-ImpactHopMinimality title: >- - The `impact` verb's cycle guard shall record the minimum connector-hop count at which - each element was reached and shall re-expand an element whenever a strictly cheaper hop - count is found, while retaining each element's first-arrival depth and relation - attribution and reporting each element at most once. + The `impact` verb shall report every element reachable by its traversal within the + applicable connector-hop bound, regardless of the order in which paths to that element + are discovered, and shall report each such element exactly once, at the depth and + relation of the first path by which it was reached. justification: | - Connector-hop cost is not monotonic in breadth-first depth: an element first reached - over a connector can later be reached over a pure reference path that consumed none of - the hop budget. A guard that records only membership pins each element to its - first-arrival cost, so such an element is never expanded with its restored budget and - elements one connector hop beyond it are silently dropped — making the result depend on - frontier iteration order rather than on the model. Re-expanding on a strictly cheaper - arrival removes that dependence, while suppressing the duplicate entry preserves both - the "each element appears exactly once" invariant and the rule that `Depth` equals the - breadth-first level of first arrival. + Connector-hop cost is not monotonic in traversal depth: an element first reached over a + connector can later be reached over a pure reference path that consumed none of the hop + budget, restoring the budget available beyond it. Unless that cheaper route is honored, + elements one connector hop past such an element are silently missing from the answer, + and which elements are missing depends on the order in which the walk happened to + discover paths rather than on the model itself. A change-impact answer that varies with + traversal order cannot be relied on or reproduced, so the reported set must be a + function of the model and the requested bound alone — while each impacted element must + still appear once, at the depth at which it first entered the answer, so callers can + read distance from the subject off the result. tests: - Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 4d59f5d4..968bad6a 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -471,13 +471,14 @@ imports, and similar resolved references — in the reverse direction. A part th the rest of an assembly purely by `connect` statements therefore reports no impacted elements, because a connector is not a reference. -> **Upgrade note — the default `impact` result changed.** In releases before this correction, -> `query impact` could follow `connect` and `bind` relationships **even without** -> `--include-connections`, whenever a connector named a directly declared element (for example +> **Upgrade note — the default `impact` result changed.** In releases before connection-aware +> impact analysis existed, `query impact` followed `connect` and `bind` relationships as if they +> were ordinary references, whenever a connector named a directly declared element (for example > `connect b to a;` or `connect hub.J1 to motorA;`). Those connectors were followed without any > hop bound and were reported as their raw endpoint — including nested port names such as -> `Model::System::hub::J1`, which cannot themselves be used as an `--element` subject. That -> contradicted the documented default described above and has been fixed. As a result, a +> `Model::System::hub::J1`, which cannot themselves be used as an `--element` subject. That was +> never intended, is inconsistent with the reference-only default described above, and has been +> corrected. As a result, a > `query impact` command that you have not changed may now report **fewer** rows than it used to > — often none — on models that rely on `connect` statements. To get those elements back, > deliberately add `--include-connections`, which now reports them correctly rolled up to their diff --git a/docs/verification/sysml2-tools-core/query.md b/docs/verification/sysml2-tools-core/query.md index eb430d13..57a6e560 100644 --- a/docs/verification/sysml2-tools-core/query.md +++ b/docs/verification/sysml2-tools-core/query.md @@ -82,10 +82,10 @@ scenarios listed under "Test Scenarios (Tool Test Project)" below run via `dotne - A single connector produces exactly one impact entry — its far endpoint rolled up to the nearest owning declaration — regardless of which side of the connector the nested port sits on, and never an additional raw-endpoint entry. -- An element re-reached at a strictly lower connector-hop count is re-expanded, so elements - within the hop budget of the reference closure are never dropped because a costlier path - reached them first, while the re-reached element keeps its first-arrival depth and relation - attribution and is still reported exactly once. +- Every element reachable within the applicable connector-hop bound is reported, including + elements that only become reachable through a cheaper path found after a costlier one, so the + reported set does not depend on the order in which paths are discovered. Each such element is + reported exactly once, at the depth and relation of the first path by which it was reached. - A cyclic connector topology terminates and reports each impacted element exactly once. - `Binding` connectors (`bind A = B;`) are traversed undirected exactly like `Connect` connectors. From 9664f78e0fd70ea794c8b74f0344a47060793fc2 Mon Sep 17 00:00:00 2001 From: Malcolm Nixon Date: Wed, 29 Jul 2026 16:49:29 -0400 Subject: [PATCH 4/4] Split compound connection-impact requirements into atomic ones Replace the compound Core -ImpactConnections requirement with five atomic requirements (-ImpactConnections, -ImpactConnectionEndpoints, -ImpactConnectionRollUp, -ImpactConnectionHopBound, -ImpactConnectionCycles) and the compound -EntryTraversalMetadata requirement with three (-EntryTraversalMetadata, -EntryMetadataJsonOmission, -EntryRelationSerialization), redistributing every test registration. The "never additionally as a reference edge" and "exactly once" clauses are dropped rather than re-homed because -Impact and -ImpactHopMinimality already own them; the exactly-once test moves to -ImpactHopMinimality. Split the Tool -IncludeConnections requirement into -IncludeConnections and -IncludeConnectionsHelp, expressed purely as observable CLI behavior with Core-internal detail removed from both title and justification. Close the general-help traceability gap with a new Tool test asserting that 'query --help' documents --include-connections. Correct the Tool design's help-block description, which named a resx key that does not exist, and correct the user guide's claim that the flag can never alter an existing row: a connector can re-attribute a row to a lower depth and a Connect relation. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/design/sysml2-tools-core/query.md | 8 +- docs/design/sysml2-tools-tool/query.md | 32 +++-- docs/reqstream/sysml2-tools-core/query.yaml | 135 +++++++++++++----- docs/reqstream/sysml2-tools-tool/query.yaml | 26 +++- docs/user_guide/introduction.md | 8 +- docs/verification/sysml2-tools-tool/query.md | 8 +- .../Query/QuerySubsystemTests.cs | 31 ++++ 7 files changed, 191 insertions(+), 57 deletions(-) diff --git a/docs/design/sysml2-tools-core/query.md b/docs/design/sysml2-tools-core/query.md index a5e37713..8d209c62 100644 --- a/docs/design/sysml2-tools-core/query.md +++ b/docs/design/sysml2-tools-core/query.md @@ -336,9 +336,15 @@ flowchart TD | SysML2Tools-Core-Query-Dependencies | `QueryEngine.Dependencies`; `QueryResultRenderer.RenderMarkdown`/`RenderJson` | | SysML2Tools-Core-Query-DependenciesNameShortening | `QueryResultRenderer.RenderMarkdown`; `QualifiedNameShortener` | | SysML2Tools-Core-Query-Impact | `QueryEngine.Impact`; `CollectImpactReferences`; `IsImpactConnectorKind` | -| SysML2Tools-Core-Query-ImpactConnections | `QueryEngine.CollectImpactConnections`; `RollUpToNearestDeclaration` | +| SysML2Tools-Core-Query-ImpactConnections | `QueryEngine.CollectImpactConnections` | +| SysML2Tools-Core-Query-ImpactConnectionEndpoints | `CollectImpactConnections`; `IsSelfOrNestedUnder` | +| SysML2Tools-Core-Query-ImpactConnectionRollUp | `QueryEngine.RollUpToNearestDeclaration` | +| SysML2Tools-Core-Query-ImpactConnectionHopBound | `QueryEngine.Impact` hop counter; `DefaultConnectionHopLimit` | +| SysML2Tools-Core-Query-ImpactConnectionCycles | `QueryEngine.Impact` `bestHops` guard; `TryReach` | | SysML2Tools-Core-Query-ImpactHopMinimality | `QueryEngine.Impact`; `CollectImpactConnections`; `TryReach` | | SysML2Tools-Core-Query-EntryTraversalMetadata | `QueryResultEntry.Depth`/`Relation`/`ViaQualifiedName` | +| SysML2Tools-Core-Query-EntryMetadataJsonOmission | `JsonIgnore(WhenWritingNull)`; `QueryResultRenderer.RenderJson` | +| SysML2Tools-Core-Query-EntryRelationSerialization | `JsonStringEnumConverter`; `QueryResultSerializerContext` | | SysML2Tools-Core-Query-Describe | `QueryEngine.Describe` | | SysML2Tools-Core-Query-Hierarchy | `QueryEngine.Hierarchy` | | SysML2Tools-Core-Query-Requirements | `QueryEngine.Requirements` | diff --git a/docs/design/sysml2-tools-tool/query.md b/docs/design/sysml2-tools-tool/query.md index 2d535c5b..1b76cbd9 100644 --- a/docs/design/sysml2-tools-tool/query.md +++ b/docs/design/sysml2-tools-tool/query.md @@ -85,14 +85,29 @@ flowchart TD Adding a new Core option therefore requires no parsing change in this subsystem. - `--include-connections` (connection-aware `impact` analysis, Core's `QueryOptions.IncludeConnections`) is exactly such an option: the only Tool-side work is help - text. `Query_GeneralOptionIncludeConnections` is printed by `PrintGeneralHelp` immediately - after the `--include-stdlib` line, and `Query_OptionIncludeConnectionsImpact` by - `PrintVerbHelp`'s `QueryVerb.Impact` arm immediately after `Query_OptionWalkDepthImpact`. Both - keys live in `QueryStrings.resx` with matching `QueryStrings` accessor properties, as the - resx-parity tests require. + text, printed from four resx keys. `PrintGeneralHelp` writes the single line + `Query_GeneralOptionIncludeConnections` immediately after the `--include-stdlib` line: + + ```text + --include-connections Also follow connect/bind edges ('impact' verb only) + ``` + + `PrintVerbHelp`'s `QueryVerb.Impact` arm writes the three-line block + `Query_OptionIncludeConnectionsImpact1`, `Query_OptionIncludeConnectionsImpact2`, and + `Query_OptionIncludeConnectionsImpact3` immediately after `Query_OptionWalkDepthImpact`: + + ```text + --include-connections Also follow connect/bind edges, undirected + (default: reference edges only). Connector + hops are bounded by --walk-depth, else 1. + ``` + + All four keys live in `QueryStrings.resx` with matching `QueryStrings` accessor properties, as + the resx-parity tests require. - Because the Tool does not parse the flag itself, keeping this help text in lockstep with - Core's accepted grammar is a Tool-subsystem responsibility verified by a dedicated help test; - a Core-only option with no Tool help line would be silently undiscoverable. + Core's accepted grammar is a Tool-subsystem responsibility, verified by one general-help test + and one `impact`-verb-help test; a Core-only option with no Tool help line would be silently + undiscoverable. #### Output File Option @@ -137,4 +152,5 @@ flowchart TD | SysML2Tools-Tool-Query-ReportHeading | `HeadingDepth`; `QueryOptions.Heading`; `QueryResultRenderer.RenderMarkdown` | | SysML2Tools-Tool-Query-HelpEnrichment | `QueryStrings.GetExample`/`Query_SchemaHint_*`; workflow-note lines | | SysML2Tools-Tool-Query-OutputFile | `QueryCliArgumentParser.Parse`; `QueryCommand.RunAsync`; `QueryResultExporter` | -| SysML2Tools-Tool-Query-IncludeConnections | Core `QueryArgumentParser.Parse` pass-through; `QueryStrings` help lines | +| SysML2Tools-Tool-Query-IncludeConnections | `QueryCliArgumentParser.Parse` pass-through to Core; `Context.Query` | +| SysML2Tools-Tool-Query-IncludeConnectionsHelp | `QueryStrings` keys via `PrintGeneralHelp` and `PrintVerbHelp` | diff --git a/docs/reqstream/sysml2-tools-core/query.yaml b/docs/reqstream/sysml2-tools-core/query.yaml index 55afd2cf..278ed846 100644 --- a/docs/reqstream/sysml2-tools-core/query.yaml +++ b/docs/reqstream/sysml2-tools-core/query.yaml @@ -125,41 +125,80 @@ sections: title: >- When `IncludeConnections` is supplied, the `impact` verb shall additionally traverse resolved `Connect` and `Binding` connector relationships without regard to their - recorded endpoint order, matching a subject element against endpoints nested inside it - and attributing each far endpoint to its nearest owning declaration exactly once and - never additionally as a reference edge, bounding connector hops along any single - traversal path to `WalkDepth` when supplied or to one hop otherwise, and terminating on - cyclic connector topologies. + recorded endpoint order, so that the element at one end of a connector is reported when + the query subject is at the other end, whichever end each occupies. justification: | - A connector's two ends carry no semantic source-causes-target direction, so the + A connector's two ends carry no semantic "source causes target" direction, so the reverse-only reference closure reports nothing for a part joined to the rest of the - assembly only by `connect` statements — the exact case a change-impact question is - asked about. Connector endpoints are also nested ports, while users reason about the - parts that own them, so containment roll-up in both directions is required for the - answer to be meaningful. Because connector graphs are dense meshes, an unbounded - connector closure degenerates to "the whole assembly", so a one-hop default keeps the - opt-in answer useful while `WalkDepth` remains available to widen it deliberately. - - The single-attribution guarantee is required because the same connector is reachable - from both the connector pass and the reference-edge pass. Attributing it twice produced - two result rows for one connector whenever the nested-port endpoint sat on the - connector's source side, and let a connector chain be followed past the hop bound the - summary text claims. + assembly only by `connect` or `bind` statements — the exact case a change-impact + question is asked about. Answering that question correctly therefore requires the + connector to be followed from either end, and requires the caller to opt in explicitly, + because it broadens the answer beyond the reference-only default. tests: - Impact_IncludeConnections_ReachesConnectedSiblingPart - Impact_IncludeConnections_QueriedFromFarEndpoint_ReachesOriginatingPart - - Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage - - Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne - - Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop - - Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates - Impact_IncludeConnections_BindingEdges_AreTraversedUndirected - Impact_IncludeConnections_ThroughPublicApi_ReturnsConnectedPartEntries + + - id: SysML2Tools-Core-Query-ImpactConnectionEndpoints + title: >- + The `impact` verb shall treat a connector as attached to the query subject when an + endpoint of that connector is the subject element itself or a feature nested within it, + and shall report the element reached at the connector's other end. + justification: | + Users ask about parts, but a connector's declared endpoints are normally the ports + nested inside those parts. Unless a subject matches connectors reaching its nested + features, asking about a part whose ports are wired to the rest of the assembly returns + nothing, which is the opposite of the true answer. + tests: + - Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage + - Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort + - Impact_IncludeConnections_ReachesConnectedSiblingPart + + - id: SysML2Tools-Core-Query-ImpactConnectionRollUp + title: >- + The `impact` verb shall report each connector endpoint it reaches as the nearest + declaration that owns that endpoint, and shall report the endpoint itself unchanged + when the endpoint is already a declared element. + justification: | + A nested port such as `System::hub::J1` is not itself a usable query subject, so + reporting it raw gives the caller a name they cannot act on and cannot feed back into + another query. Reporting the owning part instead makes every row actionable, while + leaving an already-declared endpoint alone prevents the answer from being widened to an + enclosing definition that the connector never named. + tests: + - Impact_IncludeConnections_PortEndpoints_RollUpToOwningPartUsage - Impact_IncludeConnections_DeclaredFarEndpoint_ReportsEndpointItself - Impact_IncludeConnections_DeclaredEndpointConnector_ReportsSiblingPartNotOwningDefinition + - Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort + + - id: SysML2Tools-Core-Query-ImpactConnectionHopBound + title: >- + The `impact` verb shall follow at most `WalkDepth` connector relationships along any + single traversal path when `WalkDepth` is supplied, and at most one connector + relationship along any single traversal path when it is not. + justification: | + Connector graphs in real models are dense meshes in which an unbounded connector closure + degenerates to "the whole assembly" and answers nothing. A one-connector default keeps + the opt-in answer useful out of the box, while an explicit bound lets a caller widen the + blast radius deliberately and know exactly how far the reported answer reaches. + tests: + - Impact_IncludeConnections_NoWalkDepth_BoundsConnectionHopsToOne + - Impact_IncludeConnections_WalkDepthTwo_ReachesSecondConnectionHop - Impact_IncludeConnections_ChainedDeclaredConnectors_StopsAtOneConnectorHop - Impact_IncludeConnections_ChainedDeclaredConnectors_WalkDepthTwo_ReachesSecondHopOnly - - Impact_IncludeConnections_SourceSidePortEndpoint_ReportsOwnerNotRawPort - - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry + + - id: SysML2Tools-Core-Query-ImpactConnectionCycles + title: >- + The `impact` verb shall terminate and return a result for models whose connector + relationships form cycles. + justification: | + Connectors are routinely declared in both textual orders between the same two parts, so + cyclic connector topologies are ordinary models rather than corner cases. A + change-impact query that hangs on an ordinary model is unusable, so termination is a + user-visible obligation in its own right and not merely an implementation nicety. + tests: + - Impact_IncludeConnections_CyclicConnections_TerminatesWithoutDuplicates - id: SysML2Tools-Core-Query-ImpactHopMinimality title: >- @@ -181,30 +220,50 @@ sections: tests: - Impact_IncludeConnections_CheaperReferencePath_ReExpandsConnectorsFromReReachedElement - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution + - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry - id: SysML2Tools-Core-Query-EntryTraversalMetadata title: >- - Every traversal-produced result entry shall carry machine-readable `Depth`, `Relation`, - and (where an endpoint was rolled up) `ViaQualifiedName` values alongside the existing - human-readable `Detail` text; each of the three shall be omitted from JSON output when - it is null, and `Relation` shall serialize as its edge-kind member name. + Every traversal-produced result entry shall carry machine-readable `Depth` and + `Relation` values, and a `ViaQualifiedName` value naming the endpoint an entry was + attributed from whenever the reported element differs from that endpoint, alongside the + existing human-readable `Detail` text. justification: | - Non-CLI API consumers previously had to parse the free-form `Detail` string to recover - a traversal depth, and had no way at all to tell which kind of relationship reached an - entry — a distinction that becomes essential once reference and connector traversal are - combined in one result. Exposing the values structurally makes the API contract - authoritative while retaining `Detail` so Markdown output stays human-readable, and - omitting nulls keeps every existing verb's JSON payload unchanged. + Non-CLI API consumers previously had to parse the free-form `Detail` string to recover a + traversal depth, and had no way at all to tell which kind of relationship reached an + entry — a distinction that becomes essential once reference and connector results are + combined in one answer. Exposing the values structurally makes the API contract + authoritative, while retaining `Detail` keeps Markdown output human-readable. tests: - Impact_ConnectionEntry_RecordsDepthRelationAndViaQualifiedName - Impact_ConnectionEntry_WithoutRollUp_OmitsViaQualifiedName - Impact_ReferenceEntry_RecordsDepthAndReferenceRelation - - Impact_IncludeConnections_SourceSidePortEndpoint_ProducesExactlyOneEntry - - Impact_IncludeConnections_ReReachedAtLowerHopCount_KeepsFirstArrivalDepthAndAttribution - - RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips - - RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia - RenderMarkdown_ImpactEntry_DetailRemainsHumanReadableDepthText + - id: SysML2Tools-Core-Query-EntryMetadataJsonOmission + title: >- + A result entry's `Depth`, `Relation`, and `ViaQualifiedName` values shall each be + omitted from JSON output when that value is null. + justification: | + Verbs that perform no traversal populate none of these values. Emitting them as explicit + `null`s would change the JSON payload shape of every pre-existing verb and break + consumers that key off property presence, so a purely additive contract requires null + values to be absent rather than present-and-null. + tests: + - RenderJson_EntryWithoutTraversalMetadata_OmitsDepthRelationAndVia + + - id: SysML2Tools-Core-Query-EntryRelationSerialization + title: >- + A result entry's `Relation` value shall be written to JSON as the name of the + relationship kind it records. + justification: | + A consumer reading `"Relation": "Connect"` can distinguish "connected to" from + "referenced by" without any lookup table, and the emitted contract stays stable if the + underlying set of relationship kinds is ever reordered — which an ordinal encoding would + silently break. + tests: + - RenderJson_EntryWithDepthAndRelation_IncludesBothFieldsAndRoundTrips + - id: SysML2Tools-Core-Query-Describe title: >- The Query subsystem shall report a target element's own kind, qualified name, resolved diff --git a/docs/reqstream/sysml2-tools-tool/query.yaml b/docs/reqstream/sysml2-tools-tool/query.yaml index b9d4ab40..fb93f2e8 100644 --- a/docs/reqstream/sysml2-tools-tool/query.yaml +++ b/docs/reqstream/sysml2-tools-tool/query.yaml @@ -169,17 +169,29 @@ sections: - id: SysML2Tools-Tool-Query-IncludeConnections title: >- - The query command shall accept the `--include-connections` flag, forwarding it to - Core's `QueryOptions.IncludeConnections`, and shall document it in both general help - and `impact` verb help through culture-aware resx text. + The query command shall accept the `--include-connections` option on its command line + and, when it is supplied with the `impact` verb, shall produce a report that includes + elements reached through connections. justification: | Connection-aware impact analysis changes the answer the `impact` verb gives, so it - must be an explicit, discoverable user choice rather than a silent default. Because - the Tool's CLI parser forwards every non-`--output` token to Core's parser unchanged, - the flag needs no Tool-side parsing — only help text, which must stay in lockstep with - the grammar Core actually accepts. + must be an explicit, discoverable user choice rather than a silent default. Users + invoke the analysis through this command, so the command is where accepting the choice + and acting on it must be guaranteed. tests: - Context_Create_QueryCommand_WithIncludeConnectionsFlag_SetsIncludeConnectionsTrue + - Impact_IncludeConnections_ReachesConnectedSiblingPart + + - id: SysML2Tools-Tool-Query-IncludeConnectionsHelp + title: >- + The query command's general help and its `impact` verb help shall each document the + `--include-connections` option, in localized text. + justification: | + An option that changes the meaning of a result is useless if a user cannot find it. + Users look in two places — the command's overall option list and the specific verb's + help — so the option must be discoverable from both, and in the user's own language + like every other help line. + tests: + - QuerySubsystem_QueryHelp_NoVerb_MentionsIncludeConnectionsOption - QuerySubsystem_ImpactVerbHelp_MentionsIncludeConnectionsOption - id: SysML2Tools-Tool-Query-OutputFile diff --git a/docs/user_guide/introduction.md b/docs/user_guide/introduction.md index 968bad6a..d6d4fe57 100644 --- a/docs/user_guide/introduction.md +++ b/docs/user_guide/introduction.md @@ -505,8 +505,12 @@ well. Three rules apply: `--walk-depth` continues to bound reference-edge depth exactly as before; omitting it still means *unlimited* reference depth, and only the connector limit defaults to one. -Reference-only results are completely unchanged when the flag is omitted, so adding it to an -existing command can only ever add rows, never remove or alter them. `--format json` entries +Omitting the flag leaves reference-only results completely unchanged, and adding it never +removes an element: every element reported without the flag is still reported with it. It can, +however, change how an already-reported element is described. An element that a reference path +reaches at depth 2 may be reached over a connector at depth 1, and is then reported at the +lower depth with `Relation` `Connect` and the connector named in its notes, instead of at the +higher depth with its reference relation. `--format json` entries additionally carry `Depth` (the traversal depth), `Relation` (`Connect`/`Binding` for a connector, or the reference edge kind otherwise), and `ViaQualifiedName`, so scripts can distinguish "referenced by" from "connected to" without parsing the human-readable detail text. diff --git a/docs/verification/sysml2-tools-tool/query.md b/docs/verification/sysml2-tools-tool/query.md index 34508968..69fb97f2 100644 --- a/docs/verification/sysml2-tools-tool/query.md +++ b/docs/verification/sysml2-tools-tool/query.md @@ -38,7 +38,7 @@ renderer behavior, and Core-side file-export helper behavior are verified separa invocations, schema hints, and the `--output` help text. - `--include-connections` is accepted by the CLI without any Tool-side parsing (it is forwarded verbatim to Core's parser) and sets `QueryOptions.IncludeConnections`, and it is documented in - `impact` verb help through `QueryStrings`. + both general help and `impact` verb help through `QueryStrings`. - Error paths are covered: no input files, patterns supplied but none matching on disk, target element not found, invalid `--walk-depth`, invalid `--format`, and parse-error-containing input files that still complete best-effort. @@ -96,6 +96,12 @@ Verifies that `query --help` prints general help and returns exit code 0. Verifies that the general-help path includes the workflow note recommending `list`/`find` before element-scoped verbs. +#### QuerySubsystem_QueryHelp_NoVerb_MentionsIncludeConnectionsOption + +Verifies that `query --help` prints the `--include-connections` line, including the +general-help-only qualifier `'impact' verb only`, so the option is discoverable from the +command's overall option list and not only from `impact` verb help. + #### QuerySubsystem_QueryVerbHelp_MentionsExampleInvocationAndSchemaHints (theory, 12 cases) Verifies that `query --help` prints the real example invocation and shared diff --git a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs index 42be84f5..2054040f 100644 --- a/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs +++ b/test/DemaConsulting.SysML2Tools.Tool.Tests/Query/QuerySubsystemTests.cs @@ -352,6 +352,37 @@ public async Task QuerySubsystem_QueryHelp_NoVerb_MentionsTypicalWorkflow() } } + /// + /// 'query --help' (no verb) documents the --include-connections option, so the option is + /// discoverable from the command's overall option list and not only from 'impact' verb + /// help. + /// + [Fact] + public async Task QuerySubsystem_QueryHelp_NoVerb_MentionsIncludeConnectionsOption() + { + // Arrange + var originalOut = Console.Out; + try + { + using var outWriter = new StringWriter(); + Console.SetOut(outWriter); + + // Act + using var context = Context.Create(["query", "--help"]); + await Program.RunAsync(context); + + // Assert + var output = outWriter.ToString(); + Assert.Contains("--include-connections", output); + Assert.Contains("'impact' verb only", output); + Assert.Equal(0, context.ExitCode); + } + finally + { + Console.SetOut(originalOut); + } + } + /// /// 'query <verb> --help' for every verb includes that verb's example invocation and /// the shared Markdown/JSON output-shape schema hints.